Skip to content
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: issue with missing chrome browser instance during tests #26817

Closed
wants to merge 1 commit into from
Closed

fix: issue with missing chrome browser instance during tests #26817

wants to merge 1 commit into from

Conversation

jazpearson
Copy link

@jazpearson jazpearson commented May 20, 2023

Additional details

Attempts to resolve an issue with cypress that is crippling our tests runs where chrome seems to crash intermittently. This re-creates the chrome instance if it's unexpectedly null. It's a workaround, but nobody seems to know why the instance is null so trying to be pragmatic about it. Also wondering if it's worth trying an experimental feature? Either way - i wanted to see if there was a way we can get this issue fixed.

Steps to test

Really difficult to reproduce, but our cypress tests fail rouhgly 50% of the time now, even with parallelisation.

How has the user experience changed?

PR Tasks

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


pearsoj seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@cypress-app-bot
Copy link
Collaborator

@cypress
Copy link

cypress bot commented May 20, 2023

30 flaky tests on run #46619 ↗︎

0 27302 1308 0 Flakiness 30

Details:

fix: issue with missing chrome browser instance during tests
Project: cypress Commit: 4842593e3c
Status: Passed Duration: 18:39 💡
Started: May 20, 2023 7:32 PM Ended: May 20, 2023 7:51 PM
Flakiness  create-from-component.cy.ts • 2 flaky tests • app-e2e

View Output Video

Test Artifacts
... > runs generated spec Output Screenshots Video
... > runs generated spec Output Screenshots Video
Flakiness  specs_list_latest_runs.cy.ts • 1 flaky test • app-e2e

View Output Video

Test Artifacts
App/Cloud Integration - Latest runs and Average duration > when no runs are recorded > shows placeholders for all visible specs Output Screenshots Video
Flakiness  cypress-in-cypress-component.cy.ts • 1 flaky test • app-e2e

View Output Video

Test Artifacts
Cypress In Cypress CT > default config > redirects to the specs list with error if a spec is not found Output Screenshots Video
Flakiness  e2e/origin/cookie_login.cy.ts • 1 flaky test • 5x-driver-electron

View Output Video

Test Artifacts
cy.origin - cookie login > Max-Age > past max-age -> not logged in Output Video
Flakiness  commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-electron

View Output Video

Test Artifacts
network stubbing > intercepting request > can delay and throttle a StaticResponse Output Video

The first 5 flaky specs are shown, see all 18 specs in Cypress Cloud.

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@nagash77
Copy link
Contributor

@jazpearson please make sure to sign the CLA linked in the comment above. We cannot consider PRs that do not have that signed.

// create a new instance of the client in case something went wrong
// https://github.com/cypress-io/cypress/issues/24650
const port = await protocol.getRemoteDebuggingPort()
browserCriClient = await BrowserCriClient.create(['127.0.0.1'], port, browser.displayName, options.onError, onReconnect)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we use the same connection logic elsewhere I wonder if we can refactor this. @ryanthemanuel is this something we should be doing?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking the same. I kind of wanted to get some discussion on this so we could try and attempt to fix this issue which is causing us some major head-aches. I'm more than happy to contribute towards the fix if need be.

@jazpearson
Copy link
Author

@jazpearson please make sure to sign the CLA linked in the comment above. We cannot consider PRs that do not have that signed.

I might have to re-make the PR as i've pushed the change logged in as my work account, and pushed to my personal github.

@jazpearson jazpearson closed this by deleting the head repository May 30, 2023
@AtofStryker AtofStryker removed their assignment Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing browserCriClient in connectToNewSpec
6 participants