Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decorate potentially flaky tests with @pytest.mark.flaky from pytest-rerunfailures #2483

Merged
merged 19 commits into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelog/2483.trivial.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Added ``pytest-rerunfailures`` to the ``tests`` set of dependencies,
and applied it to a flaky test.
17 changes: 17 additions & 0 deletions docs/contributing/testing_guide.rst
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,22 @@ balanced with each other rather than absolute principles.
failing. If a test depends on random numbers, use the same random
seed for each automated test run.

.. tip::

Tests that fail intermittently can be decorated with the
:py:`@pytest.mark.flaky` decorator from `pytest-rerunfailures`_
to indicate that the test should be rerun in case of failures:

.. code-block:: python

@pytest.mark.flaky(reruns=5) # see issue 1548
def test_optical_density_histogram(): ...

Each usage of this decorator should have a comment that either
indicates why the test occasionally fails (for example, if the
test must download data from an external source) or refers to an
issue describing the intermittent failures.

* **Avoid testing implementation details.** Fine-grained tests help us
find and fix bugs. However, tests that are too fine-grained become
brittle and lose resistance to refactoring. Avoid testing
Expand Down Expand Up @@ -833,6 +849,7 @@ popular IDEs:
.. _PyCharm: https://www.jetbrains.com/pycharm
.. _pytest: https://docs.pytest.org
.. _`pytest-cov`: https://pytest-cov.readthedocs.io
.. _`pytest-rerunfailures`: https://github.com/pytest-dev/pytest-rerunfailures
.. _`Python debugger`: https://docs.python.org/3/library/pdb.html
.. _refactoring: https://refactoring.guru/refactoring/techniques
.. _`test discovery conventions`: https://docs.pytest.org/en/latest/goodpractices.html#conventions-for-python-test-discovery
Expand Down
12 changes: 4 additions & 8 deletions plasmapy/utils/data/tests/test_downloader.py
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pheuer — One of the tests in this file failed intermittently in a different PR, so I figured it would be worth using the @pytest.mark.flaky decorator here so that the test will get repeated in case of a failure. It'd probably be worth adding this decorator to all unit tests that require an internet connection.

Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,9 @@


@pytest.mark.parametrize(("filename", "expected"), test_files)
@pytest.mark.flaky(reruns=5) # in case of intermittent connection to World Wide Web™
def test_get_file(filename, expected, tmp_path) -> None:
"""
Test the get_file function

"""
"""Test the get_file function."""
# Delete file if it already exists, so the test always downloads it
dl_path = tmp_path / filename
if dl_path.exists():
Expand All @@ -41,11 +39,9 @@ def test_get_file(filename, expected, tmp_path) -> None:
downloader.get_file(filename, directory=tmp_path)


@pytest.mark.flaky(reruns=5) # in case of intermittent connection to World Wide Web™
def test_get_file_NIST_PSTAR_datafile(tmp_path) -> None:
"""
Test the get_file function on a NIST PSTAR datafile

"""
"""Test the get_file function on a NIST PSTAR datafile."""
filename = "NIST_PSTAR_aluminum.txt"

# Delete file if it already exists, so the test always downloads it
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ tests = [
"pre-commit >= 3.6.0",
"pytest >= 8.0.2",
"pytest-regressions >= 2.5.0",
"pytest-rerunfailures >= 13.0",
"pytest-xdist >= 3.5.0",
"tomli >= 2.0.1",
"tox >= 4.12.1",
Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ pytest==8.0.2
pytest-datadir==1.5.0
# via pytest-regressions
pytest-regressions==2.5.0
pytest-rerunfailures==13.0
pytest-xdist==3.5.0
python-dateutil==2.9.0.post0
# via
Expand Down