Refactor dmrpp tests to expose data file path #323
Conversation
TomNicholas
temporarily deployed
to
test-release
GitHub Actions
Inactive
| def dmrparser(dmrpp_xml_str: str, tmp_path: Path, filename="test.nc") -> DMRParser: | ||
| # TODO we should actually create a dmrpp file in a temporary directory | ||
| # this would avoid the need to pass tmp_path separately | ||
|
|
||
| return DMRParser( | ||
| root=ET.fromstring(dmrpp_xml_str), data_filepath=tmp_path / filename | ||
| ) |
There was a problem hiding this comment.
Turns out right now there is no need for this to be a fixture - fixtures are basically callables that provide something pre-packaged whilst also managing setup and teardown. But with tmp_path explicitly passed in at test execution time there is nothing to setup and teardown in here, so it can just be a function instead of a fixture.
There was a problem hiding this comment.
Sounds good! I just needed them to be separate so I can use different dmrpp XMLs to test different cases. However this dictionary approach makes more sense!
| ), | ||
| ], | ||
| ) | ||
| def test_parse_chunks( | ||
| basic_dmrpp, | ||
| tmp_path, |
There was a problem hiding this comment.
This test has the same kind of structure as the open_virtual_dataset tests I want to write in #243 - because both input test data (the parser) and the expected results need to know about the tmp_path.
| </dmrpp:chunks> | ||
| </Int32> | ||
| <Group name="group1"> | ||
| DMRPP_XML_STRINGS = { |
There was a problem hiding this comment.
Again these don't have setup or teardown, so each string doesn't really need its own fixture.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #323 +/- ##
==========================================
+ Coverage 77.76% 77.78% +0.01%
==========================================
Files 48 48
Lines 3378 3398 +20
==========================================
+ Hits 2627 2643 +16
- Misses 751 755 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
TomNicholas
temporarily deployed
to
test-release
GitHub Actions
Inactive
|
Looks good to me! Nice refactor of the two pytest fixtures |
cc @ayushnag