Change test runner on --fail-fast to report first error deterministic…

…ally
mbj · May 11, 2024 · 1f45d81 · 1f45d81
1 parent 363ccf2
commit 1f45d81
Show file tree

Hide file tree

Showing 26 changed files with 249 additions and 111 deletions.
diff --git a/Changelog.md b/Changelog.md
@@ -1,3 +1,16 @@
+# v0.12.1 2024-05-11
+
+* [#1443](https://github.com/mbj/mutant/pull/1443)
+
+  Change mutants test runner to always report in test definition order deterministically.
+  Before results where recorded in order of arrival, which could result a fast test that started late to
+  be reported before a slow test that started early. Basically it makes parallel test execution reporting
+  deterministic.
+
+  Change the behavior of reporting test errors on fail fast to only display the fist test that failed.
+  Parallel test execution can result in one or more tests failing before the execution engine has time to
+  adhere to the fail fast setting.
+
 # v0.12.0 2024-04-22
 
   Drop the license gem, entirely. This reduces DRM in mutants code base to the absolute minimum.

diff --git a/Gemfile.lock b/Gemfile.lock
@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    mutant (0.12.0)
+    mutant (0.12.1)
       diff-lcs (~> 1.3)
       parser (~> 3.3.0)
       regexp_parser (~> 2.9.0)

diff --git a/lib/mutant/integration/minitest.rb b/lib/mutant/integration/minitest.rb
@@ -109,9 +109,10 @@ def call(tests)
         reporter.report
 
         Result::Test.new(
-          output:  '',
-          passed:  reporter.passed?,
-          runtime: timer.now - start
+          job_index: nil,
+          output:    '',
+          passed:    reporter.passed?,
+          runtime:   timer.now - start
         )
       end
 

diff --git a/lib/mutant/integration/null.rb b/lib/mutant/integration/null.rb
@@ -18,9 +18,10 @@ def all_tests
       # @return [Result::Test]
       def call(_tests)
         Result::Test.new(
-          output:  '',
-          passed:  true,
-          runtime: 0.0
+          job_index: nil,
+          output:    '',
+          passed:    true,
+          runtime:   0.0
         )
       end
 

diff --git a/lib/mutant/integration/rspec.rb b/lib/mutant/integration/rspec.rb
@@ -75,10 +75,12 @@ def call(tests)
         start = timer.now
         passed = @runner.run_specs(@rspec_world.ordered_example_groups).equal?(EXIT_SUCCESS)
         @runner.configuration.reset_reporter
+
         Result::Test.new(
-          output:  '',
-          passed:  passed,
-          runtime: timer.now - start
+          job_index: nil,
+          output:    '',
+          passed:    passed,
+          runtime:   timer.now - start
         )
       end
       # rubocop:enable Metrics/AbcSize

diff --git a/lib/mutant/reporter/cli/printer/test.rb b/lib/mutant/reporter/cli/printer/test.rb
@@ -68,7 +68,7 @@ class EnvResult < self
             #
             # @return [undefined]
             def run
-              visit_collection(Result, object.failed_test_results)
+              visit_failed
               visit(Env, object.env)
               FORMATS.each do |report, format, value|
                 __send__(report, format, __send__(value))
@@ -77,6 +77,27 @@ def run
 
           private
 
+            def visit_failed
+              failed = object.failed_test_results
+
+              if object.env.config.fail_fast
+                visit_failed_tests(failed.take(1))
+                visit_other_failed(failed.drop(1))
+              else
+                visit_failed_tests(failed)
+              end
+            end
+
+            def visit_other_failed(other)
+              return if other.empty?
+
+              puts('Other failed tests (report suppressed from fail fast): %d' % other.length)
+            end
+
+            def visit_failed_tests(failed)
+              visit_collection(Result, failed)
+            end
+
             def efficiency_percent
               (testtime / runtime) * 100
             end

diff --git a/lib/mutant/result.rb b/lib/mutant/result.rb
@@ -158,7 +158,7 @@ def amount_tests_success
 
     # Test result
     class Test
-      include Anima.new(:passed, :runtime, :output)
+      include Anima.new(:job_index, :passed, :runtime, :output)
 
       alias_method :success?, :passed
 
@@ -170,9 +170,10 @@ class VoidValue < self
         # @return [undefined]
         def initialize
           super(
-            output:  '',
-            passed:  false,
-            runtime: 0.0
+            job_index: nil,
+            output:    '',
+            passed:    false,
+            runtime:   0.0
           )
         end
       end # VoidValue

diff --git a/lib/mutant/test/runner/sink.rb b/lib/mutant/test/runner/sink.rb
@@ -22,7 +22,7 @@ def status
           Result::TestEnv.new(
             env:          env,
             runtime:      env.world.timer.now - @start,
-            test_results: @test_results
+            test_results: @test_results.sort_by!(&:job_index)
           )
         end
 
@@ -42,7 +42,11 @@ def response(response)
             fail response.error
           end
 
-          @test_results << response.result.with(output: response.log)
+          @test_results << response.result.with(
+            job_index: response.job.index,
+            output:    response.log
+          )
+
           self
         end
       end # Sink

diff --git a/lib/mutant/version.rb b/lib/mutant/version.rb
@@ -2,5 +2,5 @@
 
 module Mutant
   # Current mutant version
-  VERSION = '0.12.0'
+  VERSION = '0.12.1'
 end # Mutant
diff --git a/spec/support/shared_context.rb b/spec/support/shared_context.rb
@@ -230,17 +230,19 @@ def setup_shared_context
 
     let(:mutation_a_test_result) do
       Mutant::Result::Test.new(
-        output:  '',
-        passed:  false,
-        runtime: 1.0
+        job_index: 0,
+        output:    '',
+        passed:    false,
+        runtime:   1.0
       )
     end
 
     let(:mutation_b_test_result) do
       Mutant::Result::Test.new(
-        output:  '',
-        passed:  false,
-        runtime: 1.0
+        job_index: 1,
+        output:    '',
+        passed:    false,
+        runtime:   1.0
       )
     end
 

diff --git a/spec/unit/mutant/env_spec.rb b/spec/unit/mutant/env_spec.rb
@@ -86,9 +86,10 @@ def apply
 
     let(:test_result) do
       Mutant::Result::Test.new(
-        passed:  true,
-        runtime: 0.1,
-        output:  ''
+        job_index: nil,
+        output:    '',
+        passed:    true,
+        runtime:   0.1
       )
     end
 

diff --git a/spec/unit/mutant/integration/rspec_spec.rb b/spec/unit/mutant/integration/rspec_spec.rb
@@ -336,9 +336,10 @@ def apply
       it 'should return failed result' do
         expect(subject).to eql(
           Mutant::Result::Test.new(
-            output:  '',
-            passed:  false,
-            runtime: 0.0
+            job_index: nil,
+            output:    '',
+            passed:    false,
+            runtime:   0.0
           )
         )
       end
@@ -352,9 +353,10 @@ def apply
       it 'should return passed result' do
         expect(subject).to eql(
           Mutant::Result::Test.new(
-            output:  '',
-            passed:  true,
-            runtime: 0.0
+            job_index: nil,
+            output:    '',
+            passed:    true,
+            runtime:   0.0
           )
         )
       end

diff --git a/spec/unit/mutant/integration_spec.rb b/spec/unit/mutant/integration_spec.rb
@@ -157,9 +157,10 @@ def apply
     it 'returns test result' do
       should eql(
         Mutant::Result::Test.new(
-          output:  '',
-          passed:  true,
-          runtime: 0.0
+          job_index: nil,
+          output:    '',
+          passed:    true,
+          runtime:   0.0
         )
       )
     end

diff --git a/spec/unit/mutant/reporter/cli/printer/test/env_result_spec.rb b/spec/unit/mutant/reporter/cli/printer/test/env_result_spec.rb
@@ -13,34 +13,101 @@
 
   let(:test_result_a) do
     Mutant::Result::Test.new(
-      output:  '<test-output>',
-      passed:  false,
-      runtime: 0.1
+      job_index: 0,
+      output:    '<test-output-a>',
+      passed:    false,
+      runtime:   0.1
     )
   end
 
   let(:test_result_b) do
     Mutant::Result::Test.new(
-      output:  '<test-output>',
-      passed:  true,
-      runtime: 0.2
+      job_index: 1,
+      output:    '<test-output-b>',
+      passed:    true,
+      runtime:   0.2
     )
   end
 
   describe '.call' do
-    it_reports <<~'STR'
-      <test-output>
-      Test environment:
-      Fail-Fast:    false
-      Integration:  null
-      Jobs:         auto
-      Tests:        2
-      Test-Results: 2
-      Test-Failed:  1
-      Test-Success: 1
-      Runtime:      0.80s
-      Testtime:     0.30s
-      Efficiency:   37.50%
-    STR
+    context 'single test failure' do
+      context 'without fail fast' do
+        it_reports <<~'STR'
+          <test-output-a>
+          Test environment:
+          Fail-Fast:    false
+          Integration:  null
+          Jobs:         auto
+          Tests:        2
+          Test-Results: 2
+          Test-Failed:  1
+          Test-Success: 1
+          Runtime:      0.80s
+          Testtime:     0.30s
+          Efficiency:   37.50%
+        STR
+      end
+
+      context 'with fail fast' do
+        let(:config) { super().with(fail_fast: true) }
+
+        it_reports <<~'STR'
+          <test-output-a>
+          Test environment:
+          Fail-Fast:    true
+          Integration:  null
+          Jobs:         auto
+          Tests:        2
+          Test-Results: 2
+          Test-Failed:  1
+          Test-Success: 1
+          Runtime:      0.80s
+          Testtime:     0.30s
+          Efficiency:   37.50%
+        STR
+      end
+    end
+
+    context 'with multiple test failures' do
+      let(:test_result_b) { super().with(passed: false) }
+
+      context 'without fail fast' do
+        it_reports <<~'STR'
+          <test-output-a>
+          <test-output-b>
+          Test environment:
+          Fail-Fast:    false
+          Integration:  null
+          Jobs:         auto
+          Tests:        2
+          Test-Results: 2
+          Test-Failed:  2
+          Test-Success: 0
+          Runtime:      0.80s
+          Testtime:     0.30s
+          Efficiency:   37.50%
+        STR
+      end
+
+      context 'with fail fast' do
+        let(:config) { super().with(fail_fast: true) }
+
+        it_reports <<~'STR'
+          <test-output-a>
+          Other failed tests (report suppressed from fail fast): 1
+          Test environment:
+          Fail-Fast:    true
+          Integration:  null
+          Jobs:         auto
+          Tests:        2
+          Test-Results: 2
+          Test-Failed:  2
+          Test-Success: 0
+          Runtime:      0.80s
+          Testtime:     0.30s
+          Efficiency:   37.50%
+        STR
+      end
+    end
   end
 end
diff --git a/spec/unit/mutant/reporter/cli/printer/test/result_spec.rb b/spec/unit/mutant/reporter/cli/printer/test/result_spec.rb
@@ -5,9 +5,10 @@
 
   let(:reportable) do
     Mutant::Result::Test.new(
-      output:  '<test-output>',
-      passed:  false,
-      runtime: 0.1
+      job_index: 0,
+      output:    '<test-output>',
+      passed:    false,
+      runtime:   0.1
     )
   end
 

diff --git a/spec/unit/mutant/reporter/cli/printer/test/status_progressive_spec.rb b/spec/unit/mutant/reporter/cli/printer/test/status_progressive_spec.rb
@@ -33,9 +33,10 @@
       let(:test_results) do
         [
           Mutant::Result::Test.new(
-            output:  '',
-            passed:  false,
-            runtime: 0.5
+            job_index: 0,
+            output:    '',
+            passed:    false,
+            runtime:   0.5
           )
         ]
       end