add --repeat-until-failure to mix test

[SteffenDE]

The world isn't perfect and so are our tests. When debugging flaky tests I found myself wanting to run mix test repeatedly until a test fails. Of course you can do a loop in bash, fish, etc., but a lot of time is spent on starting the BEAM and the application.

This is where this PR comes in. It proposes a new flag --repeat-until-failure that re-runs all tests until at least one fails.

[josevalim]

Shall we make it an integer, so we can put a maximum amount of repeats?

[SteffenDE]

Changed it to be an integer. Works fine, but I tried writing some unit tests and that did not go well...

[josevalim]

Thank you!

To be honest, I am not very happy that we need to copy of all test cases in the runner now. You can use ExUnit to run tests programmatically and that would become a memory leak.

I can think of two other options:

1. Re-run the whole module, if it succeeds, instead of the whole suite

2. Instead of having a CLI flag, have a @tag which you can annotate tests: @tag repeat_until_failure: 10 and read this tag in the runner

Thoughts?

[SteffenDE]

I don't think 1 is a good solution, because non-flaky modules could prevent others from running?

Concerning 2: maybe such a repeat tag would be nice as a complement to this in general?

I adapted the code to only store the modules when --repeat-until-failure is used. Wdyt?

[josevalim]

Sorry but I am worried about depending on global state here. For example, ExUnit.configure can be called at any time and change the meaning of this. Ideally we will keep most state management in the runner. I can think of two other options:

1. Pass a flag when we call take_modules in the runner to also make a copy of the modules we are taking
2. Have the runner collect all modules as it runs them and explicitly push them again on restore_modules

I think 2 is the cleanest and requires only a small change to async_loop. There is no need of additional state and we most rely on existing server APIs. Thoughts?

[SteffenDE]

Thank you for all the feedback, I appreciate that very much. I changed the code to use your suggestion 2.

[SteffenDE]

Maybe it would be nice to use a new seed for every repeat if it was not explicitly configured with --seed?

[josevalim]

Maybe it would be nice to use a new seed for every repeat if it was not explicitly configured with --seed?

Sounds like a solid idea to me. :) Maybe we move the loop a bit higher in the stack and run the whole configuration again?

[viniciusmuller]

This feature will be very useful, thanks for implementing it @SteffenDE 😃

[SteffenDE]

Maybe we move the loop a bit higher in the stack and run the whole configuration again?

Sounds good. I wonder how we should handle the seed, because we store the generated seed in the env and then we cannot easily say if the seed was explicitly set or generated:

elixir/lib/ex_unit/lib/ex_unit.ex

Lines 484 to 487 in 713bac0

	defp persist_defaults(config) do
	config |> Keyword.take([:max_cases, :seed, :trace]) |> configure()
	config
	end

What would happen if we don't persist the seed? (the tests still pass if I exclude the seed)

[SteffenDE]

Hmm yep. Not persisting the seed is not compatible with #12442...

[SteffenDE]

not pretty, but not sure what would be better

[SteffenDE]

So, one important difference with the seed when using --repeat-until-failure is that modules are not recompiled with the order of the new seed, because they are already compiled on the first run. The changed seed therefore only affects the order of the test runs themselves.

I think that's reasonable.

[josevalim]

I will go ahead and merge it and explore some ideas. :) Thank you!

[josevalim]

💚 💙 💜 💛 ❤️

[saveman71]

Landing on this PR as I am debugging a flaky test at the moment 🥲

20mn passes as I upgrade the codebase to 1.17 to test that new shiny tool...

20mn to debug the test, which ended up being an issue with multiple DateTime.now(), which was caught by this, (needed more than 100 runs though, but it's nice that you can focus a single test)

And done, thanks! That helped a lot! One thing that could help is to be able to, say, run tests with a concurrency of 10 or 100, as my issue would have been more prominent the slower things were, or to have another way of "slowing things down".