From eb0bc1af7dcb18903831423d4981ef27aebc4610 Mon Sep 17 00:00:00 2001 From: Markus Schirp Date: Sat, 11 May 2024 22:43:33 +0000 Subject: [PATCH 1/3] Fix visibility --- lib/mutant/matcher.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/mutant/matcher.rb b/lib/mutant/matcher.rb index 2ac61cc34..0419d09af 100644 --- a/lib/mutant/matcher.rb +++ b/lib/mutant/matcher.rb @@ -50,5 +50,6 @@ def self.ignore_subject?(config, subject) expression.prefix?(subject.expression) end end + private_class_method :ignore_subject? end # Matcher end # Mutant From 363ccf27b6083790f88793fe92f593aed6efeb29 Mon Sep 17 00:00:00 2001 From: Markus Schirp Date: Sat, 11 May 2024 22:43:34 +0000 Subject: [PATCH 2/3] Fix coverage --- spec/unit/mutant/repository/diff_spec.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spec/unit/mutant/repository/diff_spec.rb b/spec/unit/mutant/repository/diff_spec.rb index 192e0a26a..5f201b9cf 100644 --- a/spec/unit/mutant/repository/diff_spec.rb +++ b/spec/unit/mutant/repository/diff_spec.rb @@ -107,6 +107,9 @@ def apply @@ -4 +4 @@ header -a +b + @@ -8 +8 @@ header + -a + +b DIFF end From 1f45d813909ce40dac48f42d2ce84e9c04048734 Mon Sep 17 00:00:00 2001 From: Markus Schirp Date: Sat, 11 May 2024 22:43:35 +0000 Subject: [PATCH 3/3] Change test runner on --fail-fast to report first error deterministically --- Changelog.md | 13 +++ Gemfile.lock | 2 +- lib/mutant/integration/minitest.rb | 7 +- lib/mutant/integration/null.rb | 7 +- lib/mutant/integration/rspec.rb | 8 +- lib/mutant/reporter/cli/printer/test.rb | 23 +++- lib/mutant/result.rb | 9 +- lib/mutant/test/runner/sink.rb | 8 +- lib/mutant/version.rb | 2 +- spec/support/shared_context.rb | 14 ++- spec/unit/mutant/env_spec.rb | 7 +- spec/unit/mutant/integration/rspec_spec.rb | 14 ++- spec/unit/mutant/integration_spec.rb | 7 +- .../cli/printer/test/env_result_spec.rb | 107 ++++++++++++++---- .../reporter/cli/printer/test/result_spec.rb | 7 +- .../printer/test/status_progressive_spec.rb | 7 +- spec/unit/mutant/result/test_env_spec.rb | 7 +- spec/unit/mutant/result/test_spec.rb | 7 +- spec/unit/mutant/test/runner/sink_spec.rb | 62 ++++++---- test_app/Gemfile.minitest.lock | 6 +- test_app/Gemfile.rspec3.10.lock | 6 +- test_app/Gemfile.rspec3.11.lock | 6 +- test_app/Gemfile.rspec3.12.lock | 6 +- test_app/Gemfile.rspec3.13.lock | 6 +- test_app/Gemfile.rspec3.8.lock | 6 +- test_app/Gemfile.rspec3.9.lock | 6 +- 26 files changed, 249 insertions(+), 111 deletions(-) diff --git a/Changelog.md b/Changelog.md index 6203fac3b..e59c9a057 100644 --- 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 index be48e787a..926d8c5c3 100644 --- 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 index 7475160ab..e73aa2f90 100644 --- 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 index 0f3b5d26a..adae26a38 100644 --- 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 index 71d5fbc32..073a8f58f 100644 --- 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 index 321a6ab8a..93bcd9863 100644 --- 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 index 2965b1bbc..15b9df9ba 100644 --- 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 index 072afe945..5d4ed4410 100644 --- 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 index d3548b251..e63405288 100644 --- 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 index f1f9a0a55..019161161 100644 --- 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 index 271fe2a8b..e7c623c6a 100644 --- 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 index 3101e747e..8c632ca1d 100644 --- 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 index ee5521264..4954c8d08 100644 --- 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 index 2a9fdb010..7eb55cbd6 100644 --- 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: '', - passed: false, - runtime: 0.1 + job_index: 0, + output: '', + passed: false, + runtime: 0.1 ) end let(:test_result_b) do Mutant::Result::Test.new( - output: '', - passed: true, - runtime: 0.2 + job_index: 1, + output: '', + passed: true, + runtime: 0.2 ) end describe '.call' do - it_reports <<~'STR' - - 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 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 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 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' + + 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 index 4f29c67a8..a9b03ad46 100644 --- 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: '', - passed: false, - runtime: 0.1 + job_index: 0, + 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 index fad48ed54..99ae5a11d 100644 --- 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 diff --git a/spec/unit/mutant/result/test_env_spec.rb b/spec/unit/mutant/result/test_env_spec.rb index 9bd80ed70..aabbea3d8 100644 --- a/spec/unit/mutant/result/test_env_spec.rb +++ b/spec/unit/mutant/result/test_env_spec.rb @@ -26,9 +26,10 @@ let(:test_result) do Mutant::Result::Test.new( - output: '', - passed: true, - runtime: 1.0 + job_index: nil, + output: '', + passed: true, + runtime: 1.0 ) end diff --git a/spec/unit/mutant/result/test_spec.rb b/spec/unit/mutant/result/test_spec.rb index c79208929..a96027888 100644 --- a/spec/unit/mutant/result/test_spec.rb +++ b/spec/unit/mutant/result/test_spec.rb @@ -4,9 +4,10 @@ describe '.new' do it 'returns expected attributes' do expect(described_class.instance.to_h).to eql( - output: '', - passed: false, - runtime: 0.0 + job_index: nil, + output: '', + passed: false, + runtime: 0.0 ) end end diff --git a/spec/unit/mutant/test/runner/sink_spec.rb b/spec/unit/mutant/test/runner/sink_spec.rb index 63ebbe412..f12dcf7e0 100644 --- a/spec/unit/mutant/test/runner/sink_spec.rb +++ b/spec/unit/mutant/test/runner/sink_spec.rb @@ -8,12 +8,11 @@ instance_double( Mutant::Env, config: config, - world: - instance_double( - Mutant::World, - timer: timer, - stderr: stderr - ) + world: instance_double( + Mutant::World, + timer: timer, + stderr: stderr + ) ) end @@ -31,17 +30,19 @@ let(:test_result_a_raw) do Mutant::Result::Test.new( - output: '', - passed: true, - runtime: 0.1 + job_index: nil, + output: '', + passed: true, + runtime: 0.1 ) end let(:test_result_b_raw) do Mutant::Result::Test.new( - output: '', - passed: true, - runtime: 0.2 + job_index: nil, + output: '', + passed: true, + runtime: 0.2 ) end @@ -54,13 +55,13 @@ let(:job_b) do Mutant::Parallel::Source::Job.new( - index: 0, + index: 1, payload: nil ) end - let(:test_result_a) { test_result_a_raw.with(output: test_response_a.log) } - let(:test_result_b) { test_result_b_raw.with(output: test_response_b.log) } + let(:test_result_a) { test_result_a_raw.with(job_index: 0, output: test_response_a.log) } + let(:test_result_b) { test_result_b_raw.with(job_index: 1, output: test_response_b.log) } let(:test_response_a) do Mutant::Parallel::Response.new( @@ -90,7 +91,7 @@ end end - shared_context 'two results' do + shared_context 'two results, in job index order' do before do object.response(test_response_a) object.response(test_response_b) @@ -171,8 +172,25 @@ it { should eql(expected_status) } end - context 'two results' do - include_context 'two results' + context 'two results, in job index order' do + include_context 'two results, in job index order' + + let(:expected_status) do + Mutant::Result::TestEnv.new( + env: env, + runtime: 1.5, + test_results: [test_result_a, test_result_b] + ) + end + + it { should eql(expected_status) } + end + + context 'two results, not in job index order' do + before do + object.response(test_response_b) + object.response(test_response_a) + end let(:expected_status) do Mutant::Result::TestEnv.new( @@ -206,8 +224,8 @@ end end - context 'two results' do - include_context 'two results' + context 'two results, in job index order' do + include_context 'two results, in job index order' context 'when results are successful' do it { should be(false) } @@ -244,8 +262,8 @@ end end - context 'two results' do - include_context 'two results' + context 'two results, in job index order' do + include_context 'two results, in job index order' context 'when results are successful' do it { should be(false) } diff --git a/test_app/Gemfile.minitest.lock b/test_app/Gemfile.minitest.lock index c687c1814..211c0696d 100644 --- a/test_app/Gemfile.minitest.lock +++ b/test_app/Gemfile.minitest.lock @@ -1,15 +1,15 @@ PATH remote: .. specs: - mutant (0.12.0) + mutant (0.12.1) diff-lcs (~> 1.3) parser (~> 3.3.0) regexp_parser (~> 2.9.0) sorbet-runtime (~> 0.5.0) unparser (~> 0.6.9) - mutant-minitest (0.12.0) + mutant-minitest (0.12.1) minitest (~> 5.11) - mutant (= 0.12.0) + mutant (= 0.12.1) GEM remote: https://rubygems.org/ diff --git a/test_app/Gemfile.rspec3.10.lock b/test_app/Gemfile.rspec3.10.lock index 82654b075..60f48ebf1 100644 --- a/test_app/Gemfile.rspec3.10.lock +++ b/test_app/Gemfile.rspec3.10.lock @@ -1,14 +1,14 @@ PATH remote: .. specs: - mutant (0.12.0) + mutant (0.12.1) diff-lcs (~> 1.3) parser (~> 3.3.0) regexp_parser (~> 2.9.0) sorbet-runtime (~> 0.5.0) unparser (~> 0.6.9) - mutant-rspec (0.12.0) - mutant (= 0.12.0) + mutant-rspec (0.12.1) + mutant (= 0.12.1) rspec-core (>= 3.8.0, < 4.0.0) GEM diff --git a/test_app/Gemfile.rspec3.11.lock b/test_app/Gemfile.rspec3.11.lock index a8edd3186..8c1836d99 100644 --- a/test_app/Gemfile.rspec3.11.lock +++ b/test_app/Gemfile.rspec3.11.lock @@ -1,14 +1,14 @@ PATH remote: .. specs: - mutant (0.12.0) + mutant (0.12.1) diff-lcs (~> 1.3) parser (~> 3.3.0) regexp_parser (~> 2.9.0) sorbet-runtime (~> 0.5.0) unparser (~> 0.6.9) - mutant-rspec (0.12.0) - mutant (= 0.12.0) + mutant-rspec (0.12.1) + mutant (= 0.12.1) rspec-core (>= 3.8.0, < 4.0.0) GEM diff --git a/test_app/Gemfile.rspec3.12.lock b/test_app/Gemfile.rspec3.12.lock index 94551d794..21fe63f69 100644 --- a/test_app/Gemfile.rspec3.12.lock +++ b/test_app/Gemfile.rspec3.12.lock @@ -1,14 +1,14 @@ PATH remote: .. specs: - mutant (0.12.0) + mutant (0.12.1) diff-lcs (~> 1.3) parser (~> 3.3.0) regexp_parser (~> 2.9.0) sorbet-runtime (~> 0.5.0) unparser (~> 0.6.9) - mutant-rspec (0.12.0) - mutant (= 0.12.0) + mutant-rspec (0.12.1) + mutant (= 0.12.1) rspec-core (>= 3.8.0, < 4.0.0) GEM diff --git a/test_app/Gemfile.rspec3.13.lock b/test_app/Gemfile.rspec3.13.lock index d290e750c..91be4b719 100644 --- a/test_app/Gemfile.rspec3.13.lock +++ b/test_app/Gemfile.rspec3.13.lock @@ -1,14 +1,14 @@ PATH remote: .. specs: - mutant (0.12.0) + mutant (0.12.1) diff-lcs (~> 1.3) parser (~> 3.3.0) regexp_parser (~> 2.9.0) sorbet-runtime (~> 0.5.0) unparser (~> 0.6.9) - mutant-rspec (0.12.0) - mutant (= 0.12.0) + mutant-rspec (0.12.1) + mutant (= 0.12.1) rspec-core (>= 3.8.0, < 4.0.0) GEM diff --git a/test_app/Gemfile.rspec3.8.lock b/test_app/Gemfile.rspec3.8.lock index c6804d163..25b26ea72 100644 --- a/test_app/Gemfile.rspec3.8.lock +++ b/test_app/Gemfile.rspec3.8.lock @@ -1,14 +1,14 @@ PATH remote: .. specs: - mutant (0.12.0) + mutant (0.12.1) diff-lcs (~> 1.3) parser (~> 3.3.0) regexp_parser (~> 2.9.0) sorbet-runtime (~> 0.5.0) unparser (~> 0.6.9) - mutant-rspec (0.12.0) - mutant (= 0.12.0) + mutant-rspec (0.12.1) + mutant (= 0.12.1) rspec-core (>= 3.8.0, < 4.0.0) GEM diff --git a/test_app/Gemfile.rspec3.9.lock b/test_app/Gemfile.rspec3.9.lock index 94551d794..21fe63f69 100644 --- a/test_app/Gemfile.rspec3.9.lock +++ b/test_app/Gemfile.rspec3.9.lock @@ -1,14 +1,14 @@ PATH remote: .. specs: - mutant (0.12.0) + mutant (0.12.1) diff-lcs (~> 1.3) parser (~> 3.3.0) regexp_parser (~> 2.9.0) sorbet-runtime (~> 0.5.0) unparser (~> 0.6.9) - mutant-rspec (0.12.0) - mutant (= 0.12.0) + mutant-rspec (0.12.1) + mutant (= 0.12.1) rspec-core (>= 3.8.0, < 4.0.0) GEM