b0881ad·Author: Arthur Eigenbrot·Closed by: Arthur Eigenbrot·2023-11-28
Description
This is a monster PR with a huge diff, so apologies for that. Unfortunately that was kind of necessary to bring this ancient test suite into the modern age. The primary pattern change (which accounts for the bulk of the diff) is moving test data setup from the task fixtures into the actual tests themselves. In other words, take something that used to look like this:
@pytest.fixture
def some_task(tmp_path, ...):
...
with Task(...) as task:
task.scratch = WorkflowFileSystem(...)
# A WHOLE BUNCH OF CODE TO MAKE WHATEVER DATA WE NEED
yield task, other, stuff
...
def test_thing(some_task):
some_task()
assert stuff == True
and turn it into this:
@pytest.fixture
def some_task(tmp_path, ...):
...
with Task(...) as task:
task.scratch = WorkflowFileSystem(...)
yield task, other, stuff
...
def test_thing(some_task):
some_task()
# A WHOLE BUNCH OF CODE TO MAKE WHATEVER DATA WE NEED
assert stuff == True
We actually do slightly better in practice: we often use helper methods to simplify # A WHOLE BUNCH OF CODE TO MAKE WHATEVER DATA WE NEED into a few calls to these helpers.
The primary benefit of this paradigm is that different tests of the same task that need different data can use the same task fixture and just make whatever data they need. This saves us a lot of duplicated task-fixture boilerplate. A secondary benefit is that we don’t have to emit data-specific values from the task fixture because we have them directly in the actual test function.
Â
To simplify and normalize the creation of test data all of the VispHeader*FramesDataset classes in conftest have been updated to produce a complete and correct set of frames based on the init parameters. In other words, all loops over modstate, map scan, raster step, etc. etc. are taken care of by the internals of the Dataset subclasses. Nice! Additionally, the headers will have the correct values based on theses loop variables as well. Gone are the days of manually editing headers!
Â
There are a few other minor updates and some cleanup of cruft.
This is a monster PR with a huge diff, so apologies for that. Unfortunately that was kind of necessary to bring this ancient test suite into the modern age. The primary pattern change (which accounts for the bulk of the diff) is moving test data setup from the task fixtures into the actual tests themselves. In other words, take something that used to look like this:
@pytest.fixture def some_task(tmp_path, ...): ... with Task(...) as task: task.scratch = WorkflowFileSystem(...) # A WHOLE BUNCH OF CODE TO MAKE WHATEVER DATA WE NEED yield task, other, stuff ... def test_thing(some_task): some_task() assert stuff == Trueand turn it into this:
@pytest.fixture def some_task(tmp_path, ...): ... with Task(...) as task: task.scratch = WorkflowFileSystem(...) yield task, other, stuff ... def test_thing(some_task): some_task() # A WHOLE BUNCH OF CODE TO MAKE WHATEVER DATA WE NEED assert stuff == TrueWe actually do slightly better in practice: we often use helper methods to simplify
# A WHOLE BUNCH OF CODE TO MAKE WHATEVER DATA WE NEEDinto a few calls to these helpers.The primary benefit of this paradigm is that different tests of the same task that need different data can use the same task fixture and just make whatever data they need. This saves us a lot of duplicated task-fixture boilerplate. A secondary benefit is that we don’t have to emit data-specific values from the task fixture because we have them directly in the actual test function.
Â
To simplify and normalize the creation of test data all of the
VispHeader*FramesDatasetclasses in conftest have been updated to produce a complete and correct set of frames based on the init parameters. In other words, all loops over modstate, map scan, raster step, etc. etc. are taken care of by the internals of the Dataset subclasses. Nice! Additionally, the headers will have the correct values based on theses loop variables as well. Gone are the days of manually editing headers!Â
There are a few other minor updates and some cleanup of cruft.