fix: show spec reporter failures at the correct indent level#5888
fix: show spec reporter failures at the correct indent level#5888mostafaNazari702 wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5888 +/- ##
==========================================
+ Coverage 80.85% 80.90% +0.04%
==========================================
Files 64 64
Lines 4566 4576 +10
Branches 977 960 -17
==========================================
+ Hits 3692 3702 +10
Misses 874 874 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
e83c131 to
d5e8737
Compare
JoshuaKGoldberg
left a comment
There was a problem hiding this comment.
The test itself looks reasonable and good. Just requesting clarity touchups. 🚀
d5e8737 to
7d1de8d
Compare
7d1de8d to
912292e
Compare
|
@mark-wiemer @JoshuaKGoldberg It would be great if Codecov’s diff results were shown only after all other PR checks have completed, and ideally as the final check in the list. A moment ago, seeing it earlier ( with its preliminary results that almost always say "Your PR lacks latest pushes to the codebase" ) made me think i had missed something in my commits, so i ended up pushing unnecessary "fixes" before all the checks had finished. That's partly on me for not waiting, but....yeah, everything is correct now anyway, i re-corrected my pushed commit. TLDR: Make Codecov do code differences analysis as last "task" in a PR, last check in a PR, meaning. |
PR Checklist
status: accepting prsOverview
So there was this weird issue with how failures showed up in the spec reporter.
If an async error popped up after a test had already passed, the runner would go back and mark that test as failed. Totally fine, but the way it was displayed was off. The reporter was using this running counter to figure out indentation, and that counter just follows whatever suite is currently being entered or exited, not the suite the failed test actually belongs to.
You'd end up with failures showing at the wrong level, which made the whole output kind of confusing to read.
The fix walks up
test.parentto the root suite to figure out the real depth. If that info isn't available (like in parallel mode withoutlinkPartialObjects, or with mocked test objects), it falls back to the old counter so things don't break.Also added a fixture and integration test to recreate that async error situation across suites, just to make sure it's properly covered now.