Skip to content

Conversation

@ReinderVosDeWael
Copy link
Contributor

This pull request refactors the test workflow to handle failures gracefully. Previously, the workflow would continue even if tests failed, without reporting what the failures are. This PR resolves that by making the action fail upon test failure with full logging.

As a side-effect, coverage is no longer reported when tests fail. Having both accurate failures and coverage is a bit of a mess; to me this is an acceptable trade-off.

@ReinderVosDeWael
Copy link
Contributor Author

@nx10 bit of a feature trade-off on this one (greatly simplifies the workflow, provides accurate logging, but no more coverage on failure). This seems acceptable to me as I rarely find myself caring about coverage when tests are failing, but what do you think?

@ReinderVosDeWael ReinderVosDeWael self-assigned this Dec 12, 2023
@codecov
Copy link

codecov bot commented Dec 12, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (457a4db) 0.00% compared to head (49e9f81) 100.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##           main       #82        +/-   ##
===========================================
+ Coverage      0   100.00%   +100.00%     
===========================================
  Files         0         1         +1     
  Lines         0        10        +10     
===========================================
+ Hits          0        10        +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nx10
Copy link
Contributor

nx10 commented Dec 12, 2023

All for it! I was thinking about collapsing these steps as well - does this by chance help with implementing #32 ?

@ReinderVosDeWael
Copy link
Contributor Author

Honestly, no idea, but I'd suggest leaving that for a separate PR.

@ReinderVosDeWael
Copy link
Contributor Author

Note: since the coverage step is removed by this action. We'll have to force override that to merge and disable that requirement after.

@nx10 nx10 merged commit 872694a into main Dec 12, 2023
@nx10 nx10 deleted the feature/fix-test-failure branch December 12, 2023 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Failure during pytest outside a test yields no logs

4 participants