Skip to content

fix(rspec): quarantine fixed-pending examples instead of hard-failing#1141

Merged
trunk-io[bot] merged 2 commits into
mainfrom
claude/quarantined-test-failure-whpv4c
Jul 10, 2026
Merged

fix(rspec): quarantine fixed-pending examples instead of hard-failing#1141
trunk-io[bot] merged 2 commits into
mainfrom
claude/quarantined-test-failure-whpv4c

Conversation

@TylerJang27

@TylerJang27 TylerJang27 commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Problem

Was seeing this pending fixed not being quarantined properly: https://github.com/trunk-io/flake-farm/actions/runs/29036655722/job/86202060288#step:7:83

A pending example whose body now passes is turned by RSpec into a real failure (PendingExampleFixedError) that reds the build. But Example#set_exception short-circuited on metadata[:pending] before reaching handle_quarantine_check:

return set_exception_core(exception) if metadata[:pending]   # bailed here
...
handle_quarantine_check(exception)

So a fixed-pending example never consulted the quarantine list. A test you had quarantined in Trunk still failed the build, and it uploaded as a plain, un-quarantined failure (is_quarantined derives from metadata[:quarantined_exception], which only handle_quarantine_check sets). This is what surfaced in flake-farm: reports a fixed pending example as failure stayed red despite being quarantined.

Follow-up to #1136, which fixed how these examples are classified but left the quarantine path unreached.

Fix

  • set_exception: let a PendingExampleFixedError fall through to the quarantine check. Ordinary still-failing pending examples (routed by RSpec to pending_exception) are left untouched and stay green.
  • status_and_exception: a quarantined fixed-pending has its exception swallowed, so RSpec leaves it :pending. Key on execution_result.pending_fixed? (set by RSpec's mark_fixed! and preserved through the swallow) to still report it as a failure carrying the fixed error — so it serializes as failure + quarantined rather than success.
  • handle_quarantine_check_safely: guards the case where the quarantine lookup itself fails/raises. This was already handled for normal failures, but for a pending example any exception raised inside set_exception was re-fed to set_exception by RSpec, routed into pending_exception (which also resets pending_fixed), and the example was silently reported as a green pending. Wrapping the check so that on any error we fall back to set_exception_core(exception) makes the real failure stand (fail closed) and logs a warning instead of swallowing it.

Tests

Mirrors the variant_quarantine_spec.rb pattern in the ruby gem upload action:

  • Adds spec/pending_spec.rb — a fixed-pending example (pending_quarantine_test).
  • The with-variant step now runs variant_quarantine_spec.rb + pending_spec.rb and asserts the run passes (both quarantined under smoke-test-variant) — exercising the swallow path end-to-end.
  • A new no-variant step runs pending_spec.rb alone and asserts a non-zero exit, isolating the fixed-pending failure when it is not quarantined.

Dylan's test/pending_status_spec.rb (unit-level status mapping) is unchanged.

Note

For the with-variant step to pass, pending_quarantine_test must be quarantined under variant smoke-test-variant in staging — the same prerequisite variant_quarantine_test already has.

Verification

  • Built the native extension; full gem suite rake test: 18 examples, 0 failures.
  • Drove the real run_with_trunk path against spec/pending_spec.rb with a stubbed lookup:
    • quarantined → exit 0, reported failure + quarantined (with-variant "should pass").
    • not quarantined → exit 1 (no-variant "should fail").
    • lookup raises → warning logged, exit 1 (fail closed via handle_quarantine_check_safely, no silent green).
  • action.yaml validated as well-formed YAML.

🤖 Generated with Claude Code

A `pending` example whose body now passes is turned by RSpec into a real
failure (PendingExampleFixedError) that reds the build. Example#set_exception
short-circuited on metadata[:pending] before reaching handle_quarantine_check,
so such an example never consulted the quarantine list: a fixed-pending test
the user had quarantined still failed the build and uploaded as a plain,
un-quarantined failure (is_quarantined derives from metadata[:quarantined_exception],
which only handle_quarantine_check sets).

- set_exception: let a PendingExampleFixedError fall through to the quarantine
  check (ordinary still-failing pending examples stay green).
- status_and_exception: when the swallowed fixed-pending leaves RSpec at
  :pending, use execution_result.pending_fixed? to still report it as a failure
  carrying the fixed error, so it serializes as failure + quarantined.

Coverage (ruby gem upload action), mirroring variant_quarantine_spec.rb:
- spec/pending_spec.rb: a fixed-pending example.
- with-variant step now runs variant_quarantine_spec.rb + pending_spec.rb and
  asserts the run passes (both quarantined under smoke-test-variant).
- new no-variant step runs pending_spec.rb alone and asserts a non-zero exit,
  isolating the fixed-pending failure when it is not quarantined.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RHvMvkpGYtQYZ3FQ4j2w65
@trunk-io

trunk-io Bot commented Jul 9, 2026

Copy link
Copy Markdown

😎 Merged directly without going through the merge queue, as the queue was empty and the PR was up to date with the target branch - details.

@codecov-commenter

codecov-commenter commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.77%. Comparing base (4d5dd41) to head (ed94abe).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1141      +/-   ##
==========================================
+ Coverage   82.65%   82.77%   +0.12%     
==========================================
  Files          70       70              
  Lines       15588    15588              
==========================================
+ Hits        12884    12903      +19     
+ Misses       2704     2685      -19     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@trunk-staging-io

trunk-staging-io Bot commented Jul 9, 2026

Copy link
Copy Markdown

Static BadgeStatic BadgeStatic BadgeStatic Badge

Failed Test Failure Summary Logs
pending_quarantine_test should be quarantined when run with variant A test marked as pending was expected to fail but unexpectedly passed. Logs ↗︎
pending_quarantine_test should be quarantined when run with variant A test marked as pending was expected to fail but unexpectedly passed. Logs ↗︎
pending_quarantine_test should be quarantined when run with variant A test marked as pending was expected to fail but unexpectedly passed. Logs ↗︎
variant_quarantine_test should be quarantined when run with variant A test expected the sum of 2 + 2 to be 5, but it was actually 4, indicating a failing assertion. Logs ↗︎

... and 4 more

View Full Report ↗︎Docs

@trunk-io

trunk-io Bot commented Jul 9, 2026

Copy link
Copy Markdown

Static BadgeStatic BadgeStatic BadgeStatic Badge

Failed Test Failure Summary Logs
pending_quarantine_test should be quarantined when run with variant A test marked as pending was expected to fail but unexpectedly passed. Logs ↗︎
pending_quarantine_test should be quarantined when run with variant A test marked as pending was expected to fail but unexpectedly passed. Logs ↗︎
pending_quarantine_test should be quarantined when run with variant A test marked as pending was expected to fail but unexpectedly passed. Logs ↗︎
variant_quarantine_test should be quarantined when run with variant A test expected the sum of 2 + 2 to be 5, but it was actually 4, indicating a failing assertion. Logs ↗︎

... and 2 more

View Full Report ↗︎Docs

@trunk-io trunk-io Bot merged commit c052381 into main Jul 10, 2026
31 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants