From f7a10c5074d32c5fffc9fbe49761795b09da0068 Mon Sep 17 00:00:00 2001 From: Karim El-Husseiny Date: Mon, 4 May 2026 21:32:46 -0400 Subject: [PATCH] fix: do not exit non-zero on inconclusive bisect Bisect currently exits 1 in two cases that represent successful diagnostic runs rather than failures: 1. "The bisection was inconclusive, there might not be any leaky test here." - the bisect ran, narrowed candidates, and the failure did not reproduce on the final narrowed order. This is the expected outcome for genuinely flaky tests (timing, async, network) rather than order-dependent ones. 2. "The failing test was the first test in the test order so there is nothing to bisect." - the bisect ran and reported that there are no preceding tests to suspect. Both are valid findings produced by a successful run. Treating them as build failures forces callers (e.g. CI pipelines that invoke bisect on flaky tests) to permanently sit at a low pass rate even though bisect is doing exactly what it is supposed to do, and makes it impossible to distinguish these outcomes from real harness failures like the one fixed in #400 (lazy_load leaving Minitest.loaded_tests empty so the failing test could not be found). After this change: exit 0 - bisect ran to completion (polluter found, inconclusive, failing test failed in isolation, or nothing to bisect) exit 1 - bisect could not run (FAILING_TEST not present in the queue, missing arguments, etc.) Tests are updated to assert the contract for each scenario. --- ruby/lib/minitest/queue/runner.rb | 9 +++++-- ruby/test/integration/minitest_bisect_test.rb | 24 ++++++++++++++----- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/ruby/lib/minitest/queue/runner.rb b/ruby/lib/minitest/queue/runner.rb index 66e49d60..ef1e0bb5 100644 --- a/ruby/lib/minitest/queue/runner.rb +++ b/ruby/lib/minitest/queue/runner.rb @@ -254,7 +254,9 @@ def bisect_command step(yellow("The failing test was the first test in the test order so there is nothing to bisect.")) File.write('log/test_order.log', "") File.write('log/bisect_test_details.log', "") - exit! 1 + # Bisect ran successfully; there is simply nothing to bisect against. + # Reserve non-zero exits for cases where the bisect could not run (see failing_test_present? above). + exit! 0 end failing_order = queue.candidates @@ -263,7 +265,10 @@ def bisect_command step(yellow("The bisection was inconclusive, there might not be any leaky test here.")) File.write('log/test_order.log', "") File.write('log/bisect_test_details.log', "") - exit! 1 + # Bisect ran successfully; the failure did not reproduce on the narrowed-down order. + # This is the expected outcome for genuinely flaky tests (timing/async) rather than order-dependent ones. + # Reserve non-zero exits for cases where the bisect could not run. + exit! 0 else step(green('The following command should reproduce the leak on your machine:'), collapsed: false) command = %w(bundle exec minitest-queue --queue - run) diff --git a/ruby/test/integration/minitest_bisect_test.rb b/ruby/test/integration/minitest_bisect_test.rb index f317f3e7..4dcc94aa 100644 --- a/ruby/test/integration/minitest_bisect_test.rb +++ b/ruby/test/integration/minitest_bisect_test.rb @@ -11,10 +11,12 @@ def setup end def test_bisect + result = nil out, err = capture_subprocess_io do - run_bisect('log/leaky_test_order.log', 'LeakyTest#test_sensible_to_leak') + result = run_bisect('log/leaky_test_order.log', 'LeakyTest#test_sensible_to_leak') end + assert_equal true, result, "bisect should exit 0 when a leaky test is identified" assert_empty filter_deprecation_warnings(err) expected_output = strip_heredoc <<-EOS --- Testing the failing test in isolation @@ -101,10 +103,12 @@ def test_bisect end def test_inconclusive + result = nil out, err = capture_subprocess_io do - run_bisect('log/unconclusive_test_order.log', 'LeakyTest#test_sensible_to_leak') + result = run_bisect('log/unconclusive_test_order.log', 'LeakyTest#test_sensible_to_leak') end + assert_equal true, result, "inconclusive bisect should exit 0; it ran successfully and reported a valid diagnostic outcome" assert_empty filter_deprecation_warnings(err) expected_output = strip_heredoc <<-EOS --- Testing the failing test in isolation @@ -178,10 +182,12 @@ def test_inconclusive end def test_failing_test_is_the_first_entry_in_the_test_order + result = nil out, err = capture_subprocess_io do - run_bisect('log/unconclusive_test_order.log', 'LeakyTest#test_useless_0') + result = run_bisect('log/unconclusive_test_order.log', 'LeakyTest#test_useless_0') end + assert_equal true, result, "should exit 0 when there is nothing to bisect; the run completed normally" assert_empty filter_deprecation_warnings(err) expected_output = strip_heredoc <<-EOS --- Testing the failing test in isolation @@ -193,10 +199,12 @@ def test_failing_test_is_the_first_entry_in_the_test_order end def test_broken_tests_which_are_not_evaluated_are_ignored + result = nil out, err = capture_subprocess_io do - run_bisect('log/leaky_with_broken_test_order.log', 'LeakyTest#test_sensible_to_leak') + result = run_bisect('log/leaky_with_broken_test_order.log', 'LeakyTest#test_sensible_to_leak') end + assert_equal true, result, "inconclusive bisect should exit 0 even when broken tests are skipped along the way" assert_empty filter_deprecation_warnings(err) expected_output = strip_heredoc <<-EOS --- Testing the failing test in isolation @@ -270,10 +278,12 @@ def test_broken_tests_which_are_not_evaluated_are_ignored end def test_broken + result = nil out, err = capture_subprocess_io do - run_bisect('log/broken_test_order.log', 'LeakyTest#test_broken_test') + result = run_bisect('log/broken_test_order.log', 'LeakyTest#test_broken_test') end + assert_equal true, result, "should exit 0 when the failing test fails in isolation; bisect ran and produced a useful diagnostic" assert_empty filter_deprecation_warnings(err) expected_output = strip_heredoc <<-EOS --- Testing the failing test in isolation @@ -287,10 +297,12 @@ def test_broken end def test_failing_test_is_not_present + result = nil out, err = capture_subprocess_io do - run_bisect('log/leaky_test_order.log', 'LeakyTestDoesNotExist#test_sensible_to_leak') + result = run_bisect('log/leaky_test_order.log', 'LeakyTestDoesNotExist#test_sensible_to_leak') end + assert_equal false, result, "should exit non-zero when the failing test cannot be found; the run could not actually execute" assert_empty filter_deprecation_warnings(err) expected_output = strip_heredoc <<-EOS --- Testing the failing test in isolation