gh-137242: Change CI arguments: remove --randomize, add --fail-rerun
#138303
Conversation
mhsmith
added
skip news
needs backport to 3.13
mhsmith
changed the title
--randomize, add --fail-rerun--randomize, add --fail-rerun
It looks like some tests are already doing that on some runners. I'll switch this PR back to draft until this has been resolved. |
| if ns.use_mp is None: | ||
| ns.use_mp = 0 | ||
| ns.randomize = True |
There was a problem hiding this comment.
I would prefer to keep randomization for --fast-ci and --slow-ci options.
There was a problem hiding this comment.
I would prefer adding an option to disable randomization, option which could be used with --fast-ci / --slow-ci.
| ns.fail_env_changed = True | ||
| if ns.python is None: | ||
| ns.rerun = True | ||
| ns.fail_rerun = True |
There was a problem hiding this comment.
You can already pass --fail-rerun. Making it the default would make all CIs way more strict, I dislike this idea. When I tried a few years ago, I discovered tons of flaky tests and it was a pain to fix all of them.
--randomizecan discover real problems, but they're often ordering dependencies between tests, which are difficult to diagnose, and usually have nothing to do with the PR on which they occur.I assume this was the reason for removing
--fail-rerunfrom the--fast-ciand--slow-ciarguments in #110849. However, this renders--randomizealmost useless, because the failing test will usually pass on the rerun in a fresh process, and nobody will ever know that there was a failure.Also,
--rerunwithout--fail-rerunmeans that a test which ALWAYS fails the first time and passes the second time will still be treated as a pass. This seems unsafe.So I propose removing
--randomizeand restoring--fail-rerun.This will also allow iOS and Android to switch to
--fast-cion GitHub Actions and--slow-cion the buildbots. They were previously unable to do this because of the frequent failures caused by--randomize, which were not hidden on the rerun because these platforms use--single-processmode.