-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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: stop subscription race condition error on debug page #27134
Conversation
15 flaky tests on run #48325 ↗︎Details:
|
Test | Artifacts | |
---|---|---|
... > runs generated spec |
Output
Screenshots
Video
|
specs_list_latest_runs.cy.ts • 1 flaky test • app-e2e
Test | Artifacts | |
---|---|---|
App/Cloud Integration - Latest runs and Average duration > when no runs are recorded > shows placeholders for all visible specs |
Output
Screenshots
Video
|
cypress-origin-communicator.cy.ts • 1 flaky test • app-e2e
Test | Artifacts | |
---|---|---|
Cypress In Cypress Origin Communicator > cy.origin passivity with app interactions > passes upon test reload mid test execution |
Output
Screenshots
Video
|
subscriptions/configChange-subscription.cy.ts • 1 flaky test • app-e2e
Test | Artifacts | |
---|---|---|
configChange subscription > on config page > responds to configChange event when viewport is changed |
Output
Screenshots
Video
|
e2e/origin/cookie_login.cy.ts • 1 flaky test • 5x-driver-electron
Test | Artifacts | |
---|---|---|
... > past Max-Age, before Expires -> not logged in |
Output
Video
|
The first 5 flaky specs are shown, see all 10 specs in Cypress Cloud.
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.
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.
Great, looks good - thanks for adding a test. I think I'd seen this before, but thought it was due to my local repo getting messed up when I changed branches, I should have looked into it more at that point.
Are you just debugging this by kicking off runs and using debug
and trying different edge cases? I imagine this was a bit tricky to track down and debug.
Re: the code, I'm not sure the test description is ideal - there's a lot of complexity in the test, it might not be immediately obvious why (unless the user follows the git blame -- that might be enough, probably more informative than any test description can be).
Test description aside, this looks good.
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.
This worked for me 👍🏻 thanks for fixing this!
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Additional details
Summary
A race condition between two different subscriptions used on the Debug page was causing a GraphQL error to be shown to the user when viewing a running Cypress Cloud build. The error would have no effect on the results being shown on the page and resolve itself after the next polling cycle for the running build.
Issue
The error occurred because the
RevelentRunSpecsDataSource
allows the front end to pass different GraphQL fields to be returned for therelevantRunSpecChange
subscription on theCloudRun
type. These fields get combined into one query that is sent to Cypress Cloud.The race condition was occurring when the following steps occurred:
DebugTestingProgress
would run first and kick off the underlying poller in the data source and cause the first cloud query to start.useDebugRunSummary
would be mounted and register itself to watch for emitted results. The second subscription asked for thecreatedAt
field which is a non-nullable field as defined in the GraphQL type. The first subscription did not ask for this field.before
video below due to the missingcreatedAt
.GraphQLResolveInfo
objects, it would generate a GraphQL query that correctly satisfy both subscriptions.Solution
The fix was to add an additional check to the
filter
function to also validate that the object being emitted had the fields asked for that particular subscription. The trade off for this solution is that the second subscription will not have accurate results for one additional polling cycle (currently 15 seconds).Before
Screen.Recording.2023-06-26.at.9.47.26.AM.mov
After
after_trimmed.mov
Steps to test
How has the user experience changed?
No error toast is shown when viewing a running build on the Debug page.
PR Tasks
cypress-documentation
?type definitions
?