-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Enable E2ETest.ServerExecutionTests.CircuitTests
#63143
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.
Pull Request Overview
This PR enables previously quarantined E2E tests for circuit termination by adding error UI dismissal logic. The tests were quarantined because error dialogs were overlapping buttons needed for subsequent test steps.
Key changes:
- Removes
QuarantinedTest
attributes from two test methods - Adds error UI dismissal logic to ensure test buttons remain accessible
Pull request was closed
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.
Makes sense in the context of E2E testing 👍
@MackinnonBuck, I'm setting for automerge, feel free to let us know if the test's intention was to click the button with the popup opened and we'll revert. |
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.
Looks good! 🚀
The test checks if we correctly terminate the circuit after an exception was thrown. The check if the circuit was terminated involves clicking a button that got hidden by error window with the throw exception.
There was an attempt to fix it by moving the window position, so that the button becomes visible. Why don't we close the window to make sure button visibility is not a problem?
Fixes #57551