search

sbi-dev / sbi

Simulation-based inference toolkit
https://sbi-dev.github.io/sbi/
Apache License 2.0
578 stars 145 forks source link

test: refactor tests, make xfail strict. #1177

Closed janfb closed 3 months ago

janfb commented 3 months ago

I changed the pytest settings to xfail=strict. This means that when a test that is supposed to fail passes, this will results in a failure (was ignored before). This has the advantage that notice when a bug or missing feature was fixed "by accident" (see SNRE example below).

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 72.92%. Comparing base (8efb8e4) to head (99c8778). Report is 4 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1177 +/- ## =========================================== - Coverage 83.69% 72.92% -10.77% =========================================== Files 93 94 +1 Lines 7397 7451 +54 =========================================== - Hits 6191 5434 -757 - Misses 1206 2017 +811 ``` | [Flag](https://app.codecov.io/gh/sbi-dev/sbi/pull/1177/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sbi-dev) | Coverage Ξ” | | |---|---|---| | [unittests](https://app.codecov.io/gh/sbi-dev/sbi/pull/1177/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sbi-dev) | `72.92% <ΓΈ> (-10.77%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sbi-dev#carryforward-flags-in-the-pull-request-comment) to find out more. [see 26 files with indirect coverage changes](https://app.codecov.io/gh/sbi-dev/sbi/pull/1177/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sbi-dev)
janfb commented 3 months ago

Great work πŸ‘ πŸš€. I tested it locally it works like a charm.

Just one point, which is more one the pytest output. It will return (once intentionally triggered)

_________________________________________________________________________________________________________________________________ test_picklability[SNRE_B-vi] __________________________________________________________________________________________________________________________________
[XPASS(strict)] 
....
FAILED tests/save_and_load_test.py::test_picklability[SNRE_B-vi]
1 failed, 4 passed, 7 warnings in 1.83s

which does not give you much information why the test failed (just [XPASS(strict)] ).

Would be great if one could print out there something like "Expected failure but test passed". (Not sure if this is supported in pytest)

I agree that the output here is not so informative. It would be good to connect the FAILED with the corresponding XPASS. Not sure whether that's possible in the settings.