koordinates / kart

Distributed version-control for geospatial and tabular data
https://kartproject.org
Other
522 stars 41 forks source link

Testing: we use contextmanager fixtures everywhere #280

Open olsen232 opened 3 years ago

olsen232 commented 3 years ago

Fixtures are slightly more useful than functions if they have both a setup and a teardown method. If they are just a single function, we could just define a function in some test_helpers file and import it.

A lot of our fixtures are just functions that return a context manager. Context managers are like fixtures in that they have set up and teardown, but the fixture itself doesn't have set up and teardown - it just returns the context manager. We could just import the context manager directly instead of declaring a fixture whose only job is to expose a context manager. Right now, the only thing that is useful about them being fixtures, is that it is a good bet that you will find them in conftest.py

There may be a way to make the context managers into real fixtures, which might help in some way (for instance, perhaps test failures will be caught by the debugger before the teardown is run??), or maybe we should just stop using fixtures as a more complicated alternative to import ... from ...

rcoup commented 3 years ago

Fixtures get picked up from various levels of conftest.py and override each other sanely, but I'm not sure whether they're import-able in quite the same way?

olsen232 commented 3 years ago

This bug isn't about whether we can import test fixtures. This bug is: why do we have text fixtures?

Almost all of our test fixtures are essentially an unusual way of importing a function from a particular file: conftest.py. They're not doing anything particularly fixturey, which I think is why I object to them. They're just functions, which in python, are generally imported using "import".