-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Make E2E tests work on Linux, support retries, and have new Azure pipeline #36207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Retry changes look good! Should we try to get these tests running on helix at the same time since long term these probably shouldn't be running on azdo test jobs
I think it would be fine to try that as soon as we do get it running on a regular pipeline, but I wouldn’t want to block until then. It’s taken a long time to get this far, and we really need the tests running in CI one way or another. |
Co-authored-by: Martin Costello <[email protected]>
@@ -0,0 +1,45 @@ | |||
# This configuration builds and runs Components E2E tests only |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we going to have both components-e2e-tests.yml
and components-e2e-tests-new.yml
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I only create the new one so I could invoke it manually (the old one is currently disabled, which means you can't even run it manually).
Before merging this PR, I'll replace the contents of the old pipeline with the new one, and delete the new one. Then we can re-enable the old one as per the "rehab plan" I listed above.
Thanks for checking.
/backport to release/6.0 |
Started backporting to release/6.0: https://github.com/dotnet/aspnetcore/actions/runs/1210805286 |
@SteveSandersonMS backporting to release/6.0 failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Make E2E tests work on Linux, support retries, and have new Azure pipeline
Applying: Opt components E2E tests out of other CI pipelines (run only in the new one)
Applying: Update src/Components/test/E2ETest/Tests/InputFileTest.cs
Applying: Move new pipeline logic into old pipeline
Using index info to reconstruct a base tree...
M .azure/pipelines/components-e2e-tests.yml
Falling back to patching base and 3-way merge...
Auto-merging .azure/pipelines/components-e2e-tests.yml
CONFLICT (content): Merge conflict in .azure/pipelines/components-e2e-tests.yml
Removing .azure/pipelines/components-e2e-tests-new.yml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0004 Move new pipeline logic into old pipeline
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128 Please backport manually! |
…eline (#36207) * Make E2E tests work on Linux, support retries, and have new Azure pipeline * Opt components E2E tests out of other CI pipelines (run only in the new one) * Update src/Components/test/E2ETest/Tests/InputFileTest.cs Co-authored-by: Martin Costello <[email protected]> * Move new pipeline logic into old pipeline Co-authored-by: Your Name <[email protected]> Co-authored-by: Martin Costello <[email protected]>
…eline (#36207) (#36247) * Make E2E tests work on Linux, support retries, and have new Azure pipeline * Opt components E2E tests out of other CI pipelines (run only in the new one) * Update src/Components/test/E2ETest/Tests/InputFileTest.cs Co-authored-by: Martin Costello <[email protected]> * Move new pipeline logic into old pipeline Co-authored-by: Your Name <[email protected]> Co-authored-by: Martin Costello <[email protected]> Co-authored-by: Your Name <[email protected]> Co-authored-by: Martin Costello <[email protected]>
This is the final chunk of the work I've been doing on making the Blazor E2E tests ready to be good friends with our CI environment. This PR:
InputDate
ones, for which the browser seems to behave unpredictably with keyboard input). All the other tests do not appear to be flaky, and I've done 200+ runs to try to confirm that. More details below.selenium-config.json
file get ignored in CI now, so we no longer have to worry about version mismatches between the chromedriver binary and the version of Chrome installed on the runner machine. Now, it uses the chromedriver binary that was preinstalled on the runner machine regardless ofselenium-config.json
(which is now only used locally). So we should no longer have failures that start happening every time the AzDO images update their version of Chrome.Evidence that these should now run well in CI
After implementing all the changes except retries, I was getting around 99% pass rate for the entire suite of 800 tests, based on running it in a loop in an Azure VM around 200 times and getting exactly two failures (i.e., 2 out of 200*800=160000). That is, individual test cases are 99.999% on average, if that's a meaningful thing to average, but the suite as a whole multiplies out to about 99%. As much as we want all our tests to be 100% deterministic, that's not a realistic goal for browser automation - at least I've never heard of anyone claiming 100% determinism at scale.
99% is not good enough on its own, but it is good enough that retries can cover the gap. If the outcomes were independent and identically distributed (IID) then allowing 3 attempts per test case we'd never expect it to fail in our lifetimes (but I know IID would be a tenuous assumption - there probably are some test cases that have as-yet undetected real problems). Up to 3 attempts is what I've set.
Since adding retries, I ran for another 2.5 days totalling 132 test suite runs, and got a 100% pass rate. From logs I see there were 2 separate incidents when a test case had to be retried once. This suggests that retries are covering the gap.
As for any concerns about the build itself hanging or failing, the new pipeline only automates a minimal set of "restore and build" steps that would be correct and legal for any contributor to do in our repo. So that shouldn't fail, but if it did, we'd want to know about it in the same way as any other build failure. Also I haven't seen any cases of this new, more minimal set of build actions failing.
Rehab plan
I know people won't be comfortable just turning everything back on in a single step. I want to move more cautiously too. So I propose the following roll-out plan:
main
and PRs, but at this stage, keep it painted fake green so it can't block anyone even if something goes wrongAlso:
InputDate
tests if we can ensure a high level of reliability, and doing other cleanup (e.g., removing all the code related to Sauce Labs which we aren't using, and the test parallelism and browser-restarting optimizations which we're no longer using)