Repository navigation
[TRUNK-16209] Limit quarantine disabling to only fetching context - #764
trunk-io[bot] merged 3 commits into
Conversation
Moves the check for disabling quarantining to only apply when we're fetching quarantine state, otherwise building up context. This means that failing tests will be compared against a default quarantining state, ie no quarantined tests, allowing us to report on them and fail them.
|
😎 Merged successfully - details. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #764 +/- ##
==========================================
+ Coverage 72.75% 72.99% +0.24%
==========================================
Files 72 72
Lines 17169 17189 +20
==========================================
+ Hits 12491 12547 +56
+ Misses 4678 4642 -36 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
This is a pretty big change in terms of functionality. Correct me if I'm wrong, but anyone not using quarantining will suddenly see their uploads fail if we have failures in the bundle.
There was a problem hiding this comment.
Yes, I figured that fit more into the ethos of having the cli report the same thing your test runner reports. If you think that'd be more confusing for users, I can switch it to ignore the test failures when checking status if quarantining is disabled.
There was a problem hiding this comment.
Yeah we definitely don't want to break CI for customers. Right now the expectation for users is: "if quarantining is enabled in the repo then we should fail on un-quarantined test failures. If quarantining is disabled in the repo then we should only fail on upload failures"
There was a problem hiding this comment.
Gotcha, I'll change to that.
![trunk-staging-io[bot]](https://avatars.githubusercontent.com/in/125632?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔴 Test Results: 1 Failure
| Category | Count | Description |
|---|---|---|
| 🧪 Assertion Error | 1 | Test expects "Fail: 50" in stderr, but the actual output is "Fail: 49". |
🧪 Assertion Error
View affected tests and solution
Affected tests:
What broke
The test upload::reports_failing_tests_but_succeeds_when_quarantine_disabled expects "Fail: 50" in the stderr output, but the actual output shows "Fail: 49".
Fix
- .stderr(predicate::str::contains("Fail: 50"));
+ .stderr(predicate::str::contains("Fail: 49"));
3 participants
Moves the check for disabling quarantining to only apply when we're fetching quarantine state, otherwise building up context. This means that failing tests will be compared against a default quarantining state, ie no quarantined tests, allowing us to report on them and fail them.