You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In hitting the "automated tests" section, I noticed a couple of things about the unit test framework that could be improved.
All tests seem to detect existence of the output file, but this does not confirm correct behavior. For many of the tests, it would be simple enough to detect, eg, changes in length or gain. For others (eg IR) you might need to do regression testing against known correct outputs.
Minor thing, but your tests don't clean up after themselves. If you're using something like travis with build caching enabled, outputs from old runs could linger, and lead to spurious passing tests (because the files exist from a previous run). Since you're using pytest already, it wouldn't be too much work to use a tmp_path fixture to prevent this kind of behavior: https://docs.pytest.org/en/latest/tmpdir.html , or otherwise clean up old outputs after tests execute (pass or fail).
Point (2) would be much easier if the API was extended to allow the user to specify a target output path (#11). I see that you use the output filename to encode the deformation parameters, and letting a user specify the path exactly might make that difficult. I have some thoughts about how you might be able to accomplish both things (ie via string interpolation), but that might be out of scope for the testing issue.
The text was updated successfully, but these errors were encountered:
reference review issue
In hitting the "automated tests" section, I noticed a couple of things about the unit test framework that could be improved.
Point (2) would be much easier if the API was extended to allow the user to specify a target output path (#11). I see that you use the output filename to encode the deformation parameters, and letting a user specify the path exactly might make that difficult. I have some thoughts about how you might be able to accomplish both things (ie via string interpolation), but that might be out of scope for the testing issue.
The text was updated successfully, but these errors were encountered: