fix: do not exit non-zero on inconclusive bisect#409
Merged
kelhusseiny merged 1 commit intomainfrom May 5, 2026
Merged
Conversation
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.
zcei
approved these changes
May 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Treat inconclusive bisects as a successful diagnostic outcome instead of a build failure. Reserve non-zero exits for cases where bisect could not actually run.
Motivation
bisect_commandcurrentlyexit! 1in two cases that represent successful diagnostic runs:"The bisection was inconclusive, there might not be any leaky test here."— bisect ran, narrowed candidates, and on final validation the failure did not reproduce on the narrowed-down order. This is the expected outcome for genuinely flaky tests (timing, async, network) rather than order-dependent ones."The failing test was the first test in the test order so there is nothing to bisect."— bisect ran and reported that there are no preceding tests to suspect.Both are valid findings produced by a successful run, not failures.
In practice, callers wire
rake test:bisect(orminitest-queue ... bisectdirectly) into a pipeline that runs leakbot on each detected flaky test. Most flaky tests in a large Rails codebase are flaky for non-order-dependent reasons (timing, shared external state, async). Bisect correctly returns "inconclusive" for those, but the non-zero exit code makes the pipeline report the run as failed even though bisect did exactly what it was supposed to do.This also makes it impossible to distinguish these legitimate outcomes from genuine harness failures — for example, the lazy_load + non-streaming-queue regression fixed in #400, where bisect bailed immediately with
"The failing test does not exist."becauseMinitest.loaded_testswas empty. Both surfaced as exit 1, so callers had no way to tell "bisect could not run" from "bisect ran and found nothing to pin down."Changes
After this PR:
FAILING_TESTmissing / not present in the queueIn short: exit 0 when bisect ran to completion, regardless of outcome. Exit non-zero only when bisect could not run.
The integration tests in
test/integration/minitest_bisect_test.rbare updated to assert the new exit-code contract for each scenario.Notes
log/test_order.log,log/bisect_test_details.log) is unchanged, so callers can still distinguish outcomes by reading those if they care.cc @andheiberg — follow-up to #400.