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 tests to fix flakiness #76

Merged
merged 1 commit into from
Oct 27, 2022
Merged

Conversation

blazyy
Copy link
Contributor

@blazyy blazyy commented Oct 26, 2022

Multiple tests inside test_mdutils.py were found to be flaky, i.e. tests that both pass and fail despite no changes to the code or the tests itself.

Using the pytest-flakefinder plugin, when the tests were re-run more than once, multiple tests inside test_mdutils.py were failing. Upon looking at the code, the reason was because the working directory was being changed due to the various calls to os.chdir(tmp) inside test_fileutils.py. This meant the tests could not locate Test_file.md in the directory they were meant to be in.

To fix this, I just removed the calls to os.chdir(tmp) in test_fileutils.py and modified MarkDownFile inside fileutils.py to include another parameter called tmp, which indicates whether or not the markdown file should be created in the temporary directory or not. If this is not empty, the code is modified in such a way that the temporary files are created in the temporary directory without calls to os.chdir(tmp). This is done by including the entire path that the file is supposed to be in.

To reproduce:

  1. Install pytest-flakefinder by following the instructions here.
  2. At root directory, run pytest --flake-finder tests

After implementing the fix, all tests pass successfully, both with and without the pytest-flakefinder plugin being used.

In general, flaky tests are a pain to fix with regards to locating the root of the actual problem, so it's a good idea to fix them when you detect them. I'm aware that the tests might not be re-run at all, but this change makes the entire module more robust and future-proof so it's just a small fix for you to consider.

@didix21 didix21 self-requested a review October 27, 2022 15:10
Copy link
Owner

@didix21 didix21 left a comment

Choose a reason for hiding this comment

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

@blazyy Thanks for your contribution!

@didix21 didix21 merged commit 5a70128 into didix21:master Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants