Skip to content

fix(parsers): Render warned nodes - #81

Open
moshloop wants to merge 1 commit into
mainfrom
fix/parsers-warning-status-dka8pemsjr68
Open

fix(parsers): Render warned nodes#81
moshloop wants to merge 1 commit into
mainfrom
fix/parsers-warning-status-dka8pemsjr68

Conversation

@moshloop

@moshloop moshloop commented Jul 28, 2026

Copy link
Copy Markdown
Member

What

  • Render warned test nodes with warning status.
  • Preserve failure precedence over warnings.

Summary by CodeRabbit

  • Bug Fixes
    • Corrected test result display so warned tests show an amber warning icon and message instead of a pass indicator.
    • Ensured tests marked both failed and warned continue to display the failed status prominently.
  • Tests
    • Added coverage for warning and combined failure/warning result rendering.

Keep warned nodes visually consistent with summary status while preserving failure precedence over warnings.
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Changes

Warned Test Rendering

Layer / File(s) Summary
Pretty status rendering and coverage
testrunner/parsers/types.go, testrunner/parsers/failure_format_test.go
Test.Pretty() renders warned nodes with an amber warning icon and styling. Tests verify warned nodes do not use the pass icon and failed nodes retain failed-state rendering when also warned.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: rendering warned parser nodes with warning status.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/parsers-warning-status-dka8pemsjr68
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch fix/parsers-warning-status-dka8pemsjr68

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
testrunner/parsers/failure_format_test.go (1)

234-255: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test the amber styling promised by this change.

This test checks the warning icon and message, but not the text-amber-500 styling. Add a style-aware assertion or Clicky snapshot so a regression in the node/message color is detected.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@testrunner/parsers/failure_format_test.go` around lines 234 - 255, The
TestPrettyRendersWarnedNodeAmber test currently verifies only the warning icon
and message; extend it to inspect the rendered Clicky output and assert that the
warned node/message uses the text-amber-500 styling. Keep the existing
assertions intact and make the check fail if the amber style is removed or
replaced.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@testrunner/parsers/failure_format_test.go`:
- Around line 234-255: The TestPrettyRendersWarnedNodeAmber test currently
verifies only the warning icon and message; extend it to inspect the rendered
Clicky output and assert that the warned node/message uses the text-amber-500
styling. Keep the existing assertions intact and make the check fail if the
amber style is removed or replaced.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 84e9a9be-04df-4126-8951-115e449c404b

📥 Commits

Reviewing files that changed from the base of the PR and between bec0766 and 696a563.

📒 Files selected for processing (2)
  • testrunner/parsers/failure_format_test.go
  • testrunner/parsers/types.go

@github-actions

Copy link
Copy Markdown
Contributor

Gavel summary

Source Pass Fail Skip Duration
./site 14 0 0 856ms
./testrunner/ui 64 0 0 164ms
baseline 18 0 0 556.161µs
changegraph 14 0 0 411ms
commit 115 0 0 1.4s
fixtures 22 0 5 2ms
git 155 0 4 6.8s
github.com/flanksource/gavel/ai 7 0 0 -
github.com/flanksource/gavel/ai/aifix 18 0 0 -
github.com/flanksource/gavel/claudehistory 16 0 0 -
github.com/flanksource/gavel/cmd/gavel 167 0 0 37.9s
github.com/flanksource/gavel/cmd/gavel/choose 18 0 0 -
github.com/flanksource/gavel/commit 196 0 0 11.2s
github.com/flanksource/gavel/fixtures 143 0 0 160ms
github.com/flanksource/gavel/fixtures/types 10 0 0 10ms
github.com/flanksource/gavel/git 38 0 0 10ms
github.com/flanksource/gavel/github 110 0 2 -
github.com/flanksource/gavel/github/activity 8 0 0 10ms
github.com/flanksource/gavel/github/cache 64 0 5 -
github.com/flanksource/gavel/internal/ttyrender 3 0 0 -
github.com/flanksource/gavel/linters 49 0 0 -
github.com/flanksource/gavel/linters/betterleaks 14 0 0 10ms
github.com/flanksource/gavel/linters/golangci 2 0 0 -
github.com/flanksource/gavel/linters/jscpd 23 0 0 -
github.com/flanksource/gavel/linters/jscpd/testdata/sample 0 0 1 -
github.com/flanksource/gavel/linters/tsc 11 0 0 10ms
github.com/flanksource/gavel/pr/ui 45 0 0 -
github.com/flanksource/gavel/prwatch 87 0 0 20ms
github.com/flanksource/gavel/service 46 0 0 4.5s
github.com/flanksource/gavel/snapshots 19 0 0 1.2s
github.com/flanksource/gavel/status 48 0 0 1.6s
github.com/flanksource/gavel/testrunner 147 0 0 5.2s
github.com/flanksource/gavel/testrunner/bench 12 0 0 -
github.com/flanksource/gavel/testrunner/history 6 0 0 -
github.com/flanksource/gavel/testrunner/parsers 92 0 0 120ms
github.com/flanksource/gavel/testrunner/runners 72 0 0 70ms
github.com/flanksource/gavel/testrunner/ui 53 0 0 110ms
github.com/flanksource/gavel/todos 36 0 0 -
github.com/flanksource/gavel/todos/claude 55 0 0 650ms
github.com/flanksource/gavel/todos/types 30 0 0 -
github.com/flanksource/gavel/utils 51 0 0 -
github.com/flanksource/gavel/verify 199 0 0 -
kubernetes 43 0 0 16ms
lint: betterleaks 0 0 1 -
lint: golangci-lint 1 0 0 3m1s
lint: tsc 3 0 0 24.8s
linters 8 0 0 20ms
parsers 22 0 0 6ms
runcache 11 0 0 186ms
serve 28 0 0 533ms
ui 20 0 0 1m9s
utils 75 0 0 23ms

Totals: 2508 passed · 0 failed · 18 skipped · 5m48s

View full results

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.

1 participant