Bisect should fail fast if ordering is not stable

[derekprior]

I encountered an oddly failing test suite this morning, so I fired up rspec bisect and went to get breakfast once it started running. When I came back, this was the output I encountered:

$ rspec --bisect
Bisect started using options: ""
Running suite to find failures... (4 minutes 32.5 seconds)
Starting bisect with 8 failing examples and 1585 non-failing examples.
Checking that failure(s) are order-dependent... failure appears to be order-dependent

Round 1: bisecting over non-failing examples 1-1585 .

Bisect failed!

The example ordering is inconsistent. `--bisect` relies upon consistent ordering (e.g. by passing `--seed` if you're using random ordering) to work properly.

Is there a way to determine that I forgot to stabilize the order before running the initial test suite pass and fail the operation quickly and before the first 5 minute test suite run?

[xaviershay]

In the general case no, since the ordering strategy is customizable and is opaque to bisect short of actually trying the tests.

That said, we could possibly inspect the configuration to look for some combination of seed and default random ordering and issue a warning. However I think this would be too complicated/prone to error to be worthwhile adding, given the number of ways an ordering can be specified (CLI, configuration, metadata).

Maybe though a heuristic of "warn if --seed argument is not present in CLI args" is good enough? Are there any cases where you specifically wouldn't want to set one (even if just to get rid of the warning)? I wouldn't want to expand the scope of the configuration API to cover it. The coordinator has access to the original CLI args already...

Possible next steps:

• Proof-of-concept PR.
• Additional opinions about whether effectively requiring seed for warning-less bisect is acceptable.

[myronmarston]

Maybe though a heuristic of "warn if --seed argument is not present in CLI args" is good enough?

I think it would be frustrating for the folks who don't use random ordering. --bisect can be used with the default defined ordering, in which case a seed should not be provided. IMO, we shouldn't issue a warning from the lack of a --seed under the (potentially mistaken) assumption that the user must be using random ordering (and therefore the lack of a seed is problematic). It's confusing for a user to get a warning when they are doing something completely OK.

[fables-tales]

follow on: we probably should detect if the entire example tree is going to be consistently ordered before proceeding with a bisect. That is:

1. if all example groups use default ordering, we are done
2. if any example groups or the entire suite use random ordering, check if a seed is provided.

[myronmarston]

@samphippen while that is a complete solution, I think it's more complex than we need. For the general case, we already detect inconsistent ordering, we just don't pre-detect it as @derekprior is requesting. IMO, it's sufficient to only detect the problem for the common case of someone using random ordering (set on RSpec.configure; ultimately, CLI options or options in .rspec get copied there) and not setting a seed. It's pretty easy to detect RSpec.configuration.order == :random && RSpec.configuration.seed.nil? and warn in that case, with no need to look at any example groups. There are some complex, non-standard cases where the user is applying random ordering to only some example groups that this won't detect, but we'll still catch the issue there later on, and I don't think such a complex case is sufficiently common to warrant anything more than that.