HypothesisWorks / hypothesis

Hypothesis is a powerful, flexible, and easy to use library for property-based testing.
https://hypothesis.works
Other
7.53k stars 586 forks source link

Use tmp_path in ghostwriter tests #4050

Closed pganssle closed 2 months ago

pganssle commented 2 months ago

Before creating the temporary functions, moves the local directory to a clean temporary directory; this should guarantee that the files to be created don't exist, and handles cleanup automatically.

This also works when the local directory that the tests are executed from is not writable, but the temporary directory is.

I have tested that the way fixture resolution works, it seems that in_temp_file will only be executed once if you include both of the file-creating fixtures.

pganssle commented 2 months ago

Most of the failures seem to be random flakiness (which maybe is not great considering I keep telling people "Oh hypothesis doesn't make your test suite flaky!"), but there's this lint rule that seems... odd:

hypothesis-python/tests/ghostwriter/test_ghostwriter.py:454:5: PT004 Fixture `in_temp_path` does not return anything, add leading underscore
    |
453 | @pytest.fixture
454 | def in_temp_path(tmp_path):
    |     ^^^^^^^^^^^^ PT004
455 |     """Fixture to execute tests in a temporary path."""
456 |     old_path = Path.cwd()
    |

Is that a real thing that you actually want addressed? I have never heard of this convention before. It doesn't seem to be in evidence in the pytest documentation, and it seems misleading, since it seems to imply that the fixture is private or something.

Edit Seems like at least one pytest maintainer (and several other people chiming in) agrees with me that this rule is incorrect.

Zac-HD commented 2 months ago

Most of the failures seem to be random flakiness (which maybe is not great considering I keep telling people "Oh hypothesis doesn't make your test suite flaky!")

Well, most people aren't writing a large number of tests like "the default strategy will always generate $specific_edge_case", or "if you do $thing you get a health-check failure"; we're pushing hard enough to hit a lot of nondeterministic or timing-dependent edge cases, and with the backend evolving (#3921) we haven't finished shaking out all the flakes yet 😞