fix(rspec): report correct status for pending/skipped examples

.github/actions/test_ruby_gem_uploads/action.yaml

@@ -30,6 +30,14 @@ inputs:
   variant:
     description: Optional variant label to distinguish uploads within a matrix
     required: false
+  run-gem-unit-suite:
+    description: >-
+      When 'true', also run the gem's portable unit suite
+      (rspec-trunk-flaky-tests/test) against the built gem. Off by default so PR
+      runs do not duplicate the source-level `rake test`; enabled by the periodic
+      main-branch smoke workflow to exercise the suite on the built artifact.
+    required: false
+    default: "false"
   knapsack-pro-test-suite-token-rspec:
     description: Optional Knapsack Pro test suite token for RSpec
     required: true
@@ -92,6 +100,21 @@ runs:
         TRUNK_TEST_COLLECTION_ID: ${{ inputs.test-collection-id }}
         TRUNK_VARIANT: ${{ inputs.variant }}
 
+    - name: Run gem unit suite against the built gem (rspec-trunk-flaky-tests/test)
+      id: run-gem-unit-suite
+      if: ${{ inputs.run-gem-unit-suite == 'true' }}
+      shell: bash
+      working-directory: ${{ github.action_path }}
+      # Runs the gem's portable unit suite (rspec-trunk-flaky-tests/test) against
+      # the built, packaged gem -- making the periodic main-branch smoke run a
+      # superset of the source-level `rake test` suite. Gated (off by default) so
+      # PR runs of this action do not re-run those specs; enabled only where the
+      # checkout matches the built gem (see smoke_test_main.yml). Backend-free, so
+      # auto-upload is disabled; the backend scenario specs below cover the rest.
+      run: bundle exec rspec "${GITHUB_WORKSPACE}/rspec-trunk-flaky-tests/test" --format documentation
+      env:
+        DISABLE_RSPEC_TRUNK_FLAKY_TESTS: "true"
+
     - name: Run variant quarantine test without variant (should fail - not quarantined)
       id: run-variant-test-no-variant
       shell: bash

.github/workflows/smoke_test_main.yml

@@ -230,6 +230,9 @@ jobs:
           platform: x86_64-linux
           artifact-pattern: ""
           knapsack-pro-test-suite-token-rspec: ${{ secrets.KNAPSACK_PRO_TEST_SUITE_TOKEN_RSPEC }}
+          # main is built from this checkout, so the source test/ suite matches the
+          # built gem -- run it here to smoke-test the suite on the built artifact.
+          run-gem-unit-suite: "true"
 
   test_ruby_gem_production:
     name: Test Ruby gem on production
@@ -256,6 +259,9 @@ jobs:
           platform: x86_64-linux
           artifact-pattern: ""
           knapsack-pro-test-suite-token-rspec: ${{ secrets.KNAPSACK_PRO_TEST_SUITE_TOKEN_RSPEC }}
+          # main is built from this checkout, so the source test/ suite matches the
+          # built gem -- run it here to smoke-test the suite on the built artifact.
+          run-gem-unit-suite: "true"
 
   production-slack-workflow-status:
     if: always() && github.ref == 'refs/heads/main' && (needs.test_cli.result != 'success' || needs.test_ruby_gem_production.result != 'success')

rspec-trunk-flaky-tests/Rakefile

@@ -54,6 +54,10 @@ begin
   RSpec::Core::RakeTask.new(:test) do |t|
     ENV["RB_SYS_CARGO_PROFILE"] = 'dev'
     Rake::Task['compile'].invoke
+    # Put lib on the load path so specs can `require 'rspec_trunk_flaky_tests'`
+    # (rather than require_relative the source lib). The same specs then load the
+    # installed/built gem when run from the release smoke tests.
+    t.ruby_opts = '-Ilib'
     t.pattern = FileList['test/*_spec.rb']
   end
   task default: :test

rspec-trunk-flaky-tests/lib/trunk_spec_helper.rb

@@ -336,7 +336,7 @@ def close(_notification)
 
   # trunk-ignore(rubocop/Metrics/CyclomaticComplexity,rubocop/Metrics/AbcSize,rubocop/Metrics/MethodLength)
   def add_test_case(example)
-    exception = example.exception || example.metadata[:quarantined_exception]
+    status, exception = status_and_exception(example)
     failure_message = ''
     backtrace = ''
     if exception
@@ -355,22 +355,60 @@ def add_test_case(example)
     id = example.generate_trunk_id
 
     attempt_number = example.metadata[:retry_attempts] || example.metadata[:attempt_number] || 0
-    status = example.execution_result.status.to_s
     # set the status to failure, but mark it as quarantined
     is_quarantined = example.metadata[:quarantined_exception] ? true : false
-    case example.execution_result.status
-    when :passed
-      status = is_quarantined ? Status.new('failure') : Status.new('success')
-    when :failed
-      status = Status.new('failure')
-    when :pending
-      status = Status.new('skipped')
-    end
     parent_name = example.example_group.metadata[:description]
     parent_name = parent_name.empty? ? 'rspec' : parent_name
     @testreport.add_test(id, name, classname, file, parent_name, line, status, attempt_number,
                          started_at, finished_at, failure_message || '', backtrace || '', is_quarantined)
   end
+
+  # Determine the Trunk status to report for an example, plus the exception (if
+  # any) whose message/backtrace should be recorded for it.
+  #
+  # `pending` examples need care because RSpec expresses their outcome as the
+  # inverse of the body's pass/fail. A pending example's contract is "this is
+  # expected to fail", so:
+  #   - body still fails  -> expectation met     -> RSpec status :pending, build green
+  #   - body now passes   -> expectation violated -> RSpec status :failed
+  #                          (PendingExampleFixedError), build red
+  # We report the outcome RSpec actually decided, which also matches the build
+  # result:
+  #   - skip / xit (never ran)     -> skipped
+  #   - pending, body still failing -> success (expectation met)
+  #   - pending, body now passing   -> failure (PendingExampleFixedError)
+  # RSpec's own build pass/fail behavior is left untouched (see #set_exception).
+  #
+  # trunk-ignore(rubocop/Metrics/AbcSize,rubocop/Metrics/CyclomaticComplexity,rubocop/Metrics/MethodLength)
+  def status_and_exception(example)
+    result = example.execution_result
+    quarantined_exception = example.metadata[:quarantined_exception]
+
+    # A genuinely skipped example (`skip`/`xit`, or `skip` called from a hook or
+    # body) never runs, so it has no real pass/fail outcome. RSpec still reports
+    # it with status :pending, so detect skips explicitly -- and up front -- via
+    # metadata[:skip] before interpreting any pending pass/fail semantics below.
+    return [Status.new('skipped'), nil] if example.metadata[:skip]
+
+    case result.status
+    when :passed
+      # A quarantined failure is recorded as :passed by RSpec (see #set_exception),
+      # so report it as a failure but carry the original quarantined exception.
+      quarantined_exception ? [Status.new('failure'), quarantined_exception] : [Status.new('success'), nil]
+    when :failed
+      # Includes a pending example whose body unexpectedly passed: RSpec reports it
+      # as :failed with a PendingExampleFixedError (on example.exception) and breaks
+      # the build (the pending expectation was violated), so it is a failure here too.
+      [Status.new('failure'), example.exception || quarantined_exception]
+    when :pending
+      # Not a skip (handled above), so this is a pending example whose body ran and
+      # failed as expected: the pending expectation ("this should fail") was met, so
+      # RSpec keeps the build green -- report it as a success.
+      [Status.new('success'), nil]
+    else
+      [Status.new(result.status.to_s), example.exception || quarantined_exception]
+    end
+  end
 end
 
 RSpec.configure do |c|

rspec-trunk-flaky-tests/test/pending_status_spec.rb

@@ -0,0 +1,81 @@
+# frozen_string_literal: true
+
+require 'rspec_trunk_flaky_tests'
+require_relative '../spec/spec_helper'
+require 'rspec/core/sandbox'
+
+# RSpec inverts pass/fail for `pending` examples, so trunk_spec_helper's
+# TrunkAnalyticsListener#status_and_exception reverses them when deciding what to
+# report to Trunk. These tests run real pending/skip examples in an RSpec sandbox
+# (so the "fixed pending" failure does not fail this suite) and assert the mapping:
+#
+#   skip / xit (never ran)      -> skipped
+#   pending, body still failing -> success (the "should fail" expectation was met)
+#   pending, body now passing   -> failure (PendingExampleFixedError; breaks build)
+#
+# trunk-ignore(rubocop/Metrics/BlockLength)
+RSpec.describe 'pending/skip status mapping' do
+  let(:listener) { TrunkAnalyticsListener.new }
+
+  # Define and run examples in an isolated RSpec world, returning the executed
+  # Example objects (with their execution_result populated) for inspection.
+  def run_examples(&block)
+    examples = []
+    RSpec::Core::Sandbox.sandboxed do |_config|
+      group = RSpec.describe('sandboxed', &block)
+      group.run(RSpec::Core::NullReporter)
+      examples = group.examples
+    end
+    examples
+  end
+
+  def status_of(example)
+    listener.status_and_exception(example).first.to_s
+  end
+
+  it 'reports skip as skipped' do
+    example = run_examples do
+      it('s') do
+        skip('not ready')
+        expect(1).to eq(2)
+      end
+    end.first
+    expect(status_of(example)).to eq('skipped')
+  end
+
+  it 'reports xit as skipped' do
+    example = run_examples { xit('x') { expect(1).to eq(2) } }.first
+    expect(status_of(example)).to eq('skipped')
+  end
+
+  it 'reports a still-failing pending example as success' do
+    example = run_examples do
+      it('p') do
+        pending('known broken')
+        expect(1).to eq(2)
+      end
+    end.first
+    expect(status_of(example)).to eq('success')
+  end
+
+  it 'reports a fixed (now passing) pending example as failure, carrying the fixed error' do
+    example = run_examples do
+      it('f') do
+        pending('should still fail')
+        expect(1).to eq(1)
+      end
+    end.first
+    status, exception = listener.status_and_exception(example)
+    expect(status.to_s).to eq('failure')
+    expect(exception).to be_a(RSpec::Core::Pending::PendingExampleFixedError)
+  end
+
+  it 'leaves ordinary pass/fail unchanged' do
+    examples = run_examples do
+      it('ok') { expect(1).to eq(1) }
+      it('bad') { expect(1).to eq(2) }
+    end
+    expect(status_of(examples[0])).to eq('success')
+    expect(status_of(examples[1])).to eq('failure')
+  end
+end

rspec-trunk-flaky-tests/test/test_parse_and_validate_spec.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-require_relative '../lib/rspec_trunk_flaky_tests'
+require 'rspec_trunk_flaky_tests'
 require_relative '../spec/spec_helper'
 
 # trunk-ignore(rubocop/Metrics/BlockLength)

rspec-trunk-flaky-tests/test/test_upload_file_spec.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-require_relative '../lib/rspec_trunk_flaky_tests'
+require 'rspec_trunk_flaky_tests'
 require_relative '../spec/spec_helper'
 
 # trunk-ignore(rubocop/Metrics/BlockLength)