diff --git a/.github/actions/test_ruby_gem_uploads/action.yaml b/.github/actions/test_ruby_gem_uploads/action.yaml index 900c6b02..cdf20610 100644 --- a/.github/actions/test_ruby_gem_uploads/action.yaml +++ b/.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 diff --git a/.github/workflows/smoke_test_main.yml b/.github/workflows/smoke_test_main.yml index b0180845..88f12cc2 100644 --- a/.github/workflows/smoke_test_main.yml +++ b/.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') diff --git a/rspec-trunk-flaky-tests/Rakefile b/rspec-trunk-flaky-tests/Rakefile index 6102f638..ee7bc4ce 100644 --- a/rspec-trunk-flaky-tests/Rakefile +++ b/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 diff --git a/rspec-trunk-flaky-tests/lib/trunk_spec_helper.rb b/rspec-trunk-flaky-tests/lib/trunk_spec_helper.rb index a691f638..78e38104 100644 --- a/rspec-trunk-flaky-tests/lib/trunk_spec_helper.rb +++ b/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| diff --git a/rspec-trunk-flaky-tests/test/pending_status_spec.rb b/rspec-trunk-flaky-tests/test/pending_status_spec.rb new file mode 100644 index 00000000..3eca7c4e --- /dev/null +++ b/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 diff --git a/rspec-trunk-flaky-tests/test/test_parse_and_validate_spec.rb b/rspec-trunk-flaky-tests/test/test_parse_and_validate_spec.rb index 879d931d..8b036eae 100644 --- a/rspec-trunk-flaky-tests/test/test_parse_and_validate_spec.rb +++ b/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) diff --git a/rspec-trunk-flaky-tests/test/test_upload_file_spec.rb b/rspec-trunk-flaky-tests/test/test_upload_file_spec.rb index 26d8b8a7..50cc05c5 100644 --- a/rspec-trunk-flaky-tests/test/test_upload_file_spec.rb +++ b/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)