-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Fix and un-quarantine CanUseAntiforgeryAfterInitialRender test #63640
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
Fix and un-quarantine CanUseAntiforgeryAfterInitialRender test #63640
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.
Pull Request Overview
Fixes a flaky E2E test by addressing a race condition that caused StaleElementReferenceException
when reading element text. The fix replaces a direct element access pattern with a retry mechanism to handle cases where the element gets re-rendered between acquiring the WebElement reference and reading its text value.
- Removes the
[QuarantinedTest]
attribute to re-enable the test - Wraps element access in a try-catch block within a
Browser.True
assertion for automatic retries - Addresses the
StaleElementReferenceException
race condition
src/Components/test/E2ETest/ServerRenderingTests/FormHandlingTests/AntiforgeryTests.cs
Outdated
Show resolved
Hide resolved
@ilonatommy I combined the interactive rendering condition in the component with a simplified retrying assertion in the test itself. If I get 3k successful runs (my arbitrary benchmark), I would merge it like this. |
/backport to release/10.0 |
Started backporting to release/10.0: https://github.com/dotnet/aspnetcore/actions/runs/17649932628 |
Un-quarantines the
ServerRenderingTests.FormHandlingTests.CanUseAntiforgeryAfterInitialRender
test and attempts to fix its flakiness.There seems to be a race condition due to which the
result
element can get re-rerendered between when we acquire aWebElement
reference to it (using theBrowser.Exists
method) and when we read the element's text value. When that happens,StaleElementReferenceException
is thrown. The PR tries to remove the flakiness by wrapping the access in a try/catch block in aBrowser.True
assertion, effectively adding a retry mechanism.The test currently has no failures in the last 30 days on the quarantined test CI report. However, I have observe
StaleElementReferenceException
related failures when running the test locally in a loop (roughly 1 failure per 500 runs). With the PR change, the test has not failed in 3000 runs.Fixes #57766