Introduce fairseq2n pytest marker #83
Conversation
facebook-github-bot
added
the
CLA Signed
| python_devel_only(), | ||
| reason="New fairseq2n API in Python-only installation. Skipping till v0.2.", | ||
| ) | ||
| @pytest.mark.fairseq2n("0.2a0") |
There was a problem hiding this comment.
do we need to have a custom pytest marks?
why not using skip_if_fairseq2n_newer as an annotation ?
def skip_if_fairseq2n_newer(version):
...
if ...:
warnings.warn(...)
return pytest.skip(installed_version < annotated_version, reason=f"The test requires fairseq2n v{annotated_version} or greater.")
There was a problem hiding this comment.
TBH no particular reason. I just followed the instructions here: https://docs.pytest.org/en/7.1.x/example/markers.html. Looks like one advantage of using a pytest marker is that it integrates your marker with the pytest CLI (described here). It also makes it possible to use a marker in parametrized tests. Let me know if you have any objection on using a pytest marker though. I don't have a strong opinion on this.
| def pytest_runtest_setup(item: pytest.Function) -> None: | ||
| marker = item.get_closest_marker(name="fairseq2n") | ||
| if marker is not None: | ||
| skip_if_fairseq2n_newer(*marker.args, **marker.kwargs) |
There was a problem hiding this comment.
if we end up using custom marks can we have a link to documentation ? https://docs.pytest.org/en/stable/reference/reference.html#custom-marks
There was a problem hiding this comment.
Yep. I just realized that I should also update the contribution guidelines and mention this annotation there.
|
Not needed anymore after #124 |
Based on the discussion in #76, this PR introduces a pytest marker named
fairseq2nto indicate which versions of fairseq2n are compatible with a marked test.