fix: count failed parent nodes with passing leaves in test stats#34
Merged
Conversation
A test node that fails after all its subtests passed (an error in the parent body or its cleanup) was invisible in SumTestStats - counts only walked leaves - so a failed suite could report 100% passed. Count the failed parent as one failing test, and return it from FindFailingTests so the failure is attributable. Co-Authored-By: Claude <noreply@anthropic.com>
![unblocked[bot]](https://avatars.githubusercontent.com/in/166219?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Feature/bug description:
Found via api-tests staging run
staging/8216516902_109_51_51_0_0_0: the results UI (and Slack summary) reported 51/51, 100% passed while the suiteResultwasfailed. The musicmary package's parent test node failed in cleanup (inventory listing-delete timeout) after both its subtests passed — andSumTestStatsonly counts leaves, so a failed parent with passing children contributes nothing tofailed.FindFailingTestshas the same recursion shape, so the failure was also unattributable in summaries (empty failure list on a failed run).This is how I decided to implement/fix it:
SumTestStats: after summing children, a node that isfailedwhile its leaves contributed zero failures counts itself as one failing test — totals can no longer say 100% for a failed tree.FindFailingTests: when recursion over a failed node's subtests yields nothing, return the node itself so the failure is attributable to the parent (body-after-subtests or cleanup).What can this break:
Consumers see
totalgrow by one for this shape (the failed parent), which is the point — counts become consistent withResult. api-tests will pick this up via a version bump (v0.3.2) after merge; its Slack failure list then names the parent test instead of rendering an empty list.How has this been tested:
go test ./...andgolangci-lint rungreen; new cases cover both functions.🤖 Generated with Claude Code