gh-137242: Add a --no-randomize option, and use it in Android CI#138303
Uh oh!
There was an error while loading. Please reload this page.
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. |
vstinner left a comment
vstinner left a comment There was a problem hiding this comment.
I dislike changing CI defaults.
| # -j0 --fail-env-changed --rerun --fail-rerun --slowest --verbose3 | ||
| ifns.use_mpisNone: | ||
| 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.
Lib/test/libregrtest/cmdline.py Outdated
| ns.fail_env_changed=True | ||
| ifns.pythonisNone: | ||
| 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.
mhsmith changed the title --randomize, add --fail-rerun--no-randomize option, and use it in Android CI| self.list_tests=False | ||
| self.single=False | ||
| self.randomize=False | ||
| self.randomize=None |
There was a problem hiding this comment.
I would prefer avoiding None for randomize and keep a boolean instead. I wrote #138649 which is a clone of this PR with my additional change avoiding None. What do you think?
There was a problem hiding this comment.
That makes sense; I'll close this PR in favor of that one.
3 participants
Original PR title: "Change CI arguments: remove
--randomize, add--fail-rerun"--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.