3 ms·

I'd add one more: clearly document what determines the order in which tests are run. On the one hand, running tests in any order should produce the same result

I'd add one more: clearly document what determines the order in which tests are run. On the one hand, running tests in any order should produce the same result, and would in any decent test suite. On the other hand, if the order is random or nondeterministic, it's really annoying when 2% of PRs randomly fail CI, not because of any change in the code, but because CI happened to run unrelated tests in an unexpected order.

zmj 4y ago
I’ll disagree with this. Every time I’ve seen that, the interference between tests was also possible between requests in production. I’d rather my test framework give me a 2% chance of noticing the bug than 0%.
- regularfry 4y ago
  What's annoying is not being able to reproduce the 2% cases so you can't fix it even when you've noticed them. Sensible test tools give you the random seed they used to order the tests so you can reproduce the sequence.
thomaslangston 4y ago
Test order should be random, so that the ability to run them in parallel and distribute them across multiple hosts is not lost by missing enforcement of test isolation.
schubart 4y ago
> On the one hand, running tests in any order should produce the same result, and would in any decent test suite. Therefore the tool should run the tests in random order, to flush out the non-decent tests. IMHO.
- Jare 4y ago
  If you do this, then the tool should be running the tests all the time, not just on new commits.
saghm 4y ago
The Ruby `minitest` API used to have a way to disable non-deterministic test ordering, but it was intentionally named in a condescending way: https://www.rubydoc.info/gems/minitest/Minitest%2FTest.i_suck_and_my_tests_are_order_dependent https://www.rubydoc.info/gems/minitest/Minitest%2FTest.i_suc...! I sometimes run into issues not so much due to order dependencies specifically, but due to tests running in parallel sometimes causing failures due to races. It's almost always been way more work to convert a fully serial test suite into a parallel one than it is to just write it that way from the start, so I think there's some merit in having test frameworks default to non-deterministic ordering (or parallel execution if that's feasible) with the ability to disable that and run things serially. I'm not dogmatic enough to think that fully parallel/random order tests are the right choice for every possible use case, but I think there's value in having people first run into the ordering/race issues they're introducing before deciding to run things fully serially so that they hopefully will consider the potential future work needed if they ever decide to reverse that decision.
LawTalkingGuy 4y ago
The tests are fatally broken. It means you can't trust them to properly check new work even. The solution is to use random ordering and print the ordering seed with each run so it can be repeated when it triggers an error. Immediately halt all new work until randomly run tests don't have problems. This isn't as bad as it sounds, generally it's a few classes of things that cause the interference which each will fix many tests. It's unlikely that the code actually has a 2%+ density of global-variable use, for example.

zmj 4y ago

I’ll disagree with this. Every time I’ve seen that, the interference between tests was also possible between requests in production. I’d rather my test framework give me a 2% chance of noticing the bug than 0%.

regularfry 4y ago
What's annoying is not being able to reproduce the 2% cases so you can't fix it even when you've noticed them. Sensible test tools give you the random seed they used to order the tests so you can reproduce the sequence.

thomaslangston 4y ago

Test order should be random, so that the ability to run them in parallel and distribute them across multiple hosts is not lost by missing enforcement of test isolation.

schubart 4y ago

> On the one hand, running tests in any order should produce the same result, and would in any decent test suite. Therefore the tool should run the tests in random order, to flush out the non-decent tests. IMHO.

Jare 4y ago
If you do this, then the tool should be running the tests all the time, not just on new commits.

saghm 4y ago

The Ruby `minitest` API used to have a way to disable non-deterministic test ordering, but it was intentionally named in a condescending way: https://www.rubydoc.info/gems/minitest/Minitest%2FTest.i_suck_and_my_tests_are_order_dependent https://www.rubydoc.info/gems/minitest/Minitest%2FTest.i_suc...! I sometimes run into issues not so much due to order dependencies specifically, but due to tests running in parallel sometimes causing failures due to races. It's almost always been way more work to convert a fully serial test suite into a parallel one than it is to just write it that way from the start, so I think there's some merit in having test frameworks default to non-deterministic ordering (or parallel execution if that's feasible) with the ability to disable that and run things serially. I'm not dogmatic enough to think that fully parallel/random order tests are the right choice for every possible use case, but I think there's value in having people first run into the ordering/race issues they're introducing before deciding to run things fully serially so that they hopefully will consider the potential future work needed if they ever decide to reverse that decision.

LawTalkingGuy 4y ago

The tests are fatally broken. It means you can't trust them to properly check new work even. The solution is to use random ordering and print the ordering seed with each run so it can be repeated when it triggers an error. Immediately halt all new work until randomly run tests don't have problems. This isn't as bad as it sounds, generally it's a few classes of things that cause the interference which each will fix many tests. It's unlikely that the code actually has a 2%+ density of global-variable use, for example.