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

Modify test to fix brittle flakiness #242

Closed
wants to merge 1 commit into from
Closed

Modify test to fix brittle flakiness #242

wants to merge 1 commit into from

Conversation

blazyy
Copy link

@blazyy blazyy commented Oct 8, 2022

One of the tests test_check_data_file inside the test_download.py module was found to be flaky (specifically, something called brittle flakiness), when using the pytest-flakefinder plugin. A flaky test is a test that both passes and fails despite no changes to the code or the test itself. Brittle flakiness (defined as per iFixFlakies) is when a test runs fine when run in conjunction with the other tests in the same module but fails when run on its own.

To reproduce:
pytest fooof/tests/utils/test_download.py::test_check_data_file

All other tests inside test_download.py ran fine when run on their own. To fix, I just added a single call to check_data_folder inside the check_data_file function which is called in the test_check_data_file test. This downloads the required folder before checking for the file.

I'm aware that the tests might not be run on their own, but this makes the entire module more robust so it's just a small fix for you to consider.

@blazyy blazyy closed this by deleting the head repository Oct 20, 2022
@blazyy blazyy reopened this Oct 20, 2022
@TomDonoghue
Copy link
Member

Hey @blazyy - I appreciate you looking through the module, but for this topic specifically I don't find it to be an issue that one of the tests is "flaky" - these tests are always run together, and the update to the function would make the underlying functions less modular. In general, if you have a suggested edit to a repo, it's often worth opening an issue to first check if the maintainers are interested in the update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants