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.

Sign up for GitHub

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

Jump to bottom

chore: update browser internal images to chrome 124 and firefox 125 #29391

Merged
merged 11 commits into from
Apr 30, 2024
Merged

chore: update browser internal images to chrome 124 and firefox 125 #29391

merged 11 commits into from
Apr 30, 2024

Conversation

AtofStryker
Copy link
Contributor

@AtofStryker AtofStryker commented Apr 23, 2024 •
edited
Loading

Additional details

Bumps chrome to version 124 from 118 and firefox from 115 to 125 for docker based tests inside the repo.

A few other things needed to happen in order to get chrome 124 working within the repo. This is outlined in this comment and thread below

Steps to test

How has the user experience changed?

PR Tasks

@AtofStryker AtofStryker marked this pull request as draft April 23, 2024 19:59
@AtofStryker AtofStryker force-pushed the chore/update_internal_browser_images branch from 63be1e3 to b9a95eb Compare April 23, 2024 20:38
@cypress Cypress
Copy link

cypress bot commented Apr 23, 2024 •
edited
Loading

7 failed and 4 flaky tests on run #55240 ↗︎

7 286 7 22 Flakiness 4

Details:

chore: update documentation in response middleware
Project: cypress Commit: 96ee05cfe5
Status: Failed Duration: 12:22 💡
Started: Apr 30, 2024 4:03 PM Ended: Apr 30, 2024 4:16 PM
Failed  cypress\e2e\top-nav-launchpad.cy.ts • 0 failed tests • launchpad-e2e

View Output

Test Artifacts
Failed  cypress\e2e\config-files-error-handling.cy.ts • 0 failed tests • launchpad-e2e

View Output

Test Artifacts
Failed  cypress\e2e\open-mode.cy.ts • 0 failed tests • launchpad-e2e

View Output

Test Artifacts
Failed  cypress\e2e\global-mode.cy.ts • 0 failed tests • launchpad-e2e

View Output

Test Artifacts
Failed  cypress\e2e\error-handling.cy.ts • 0 failed tests • launchpad-e2e

View Output

Test Artifacts

The first 5 failed specs are shown, see all 54 specs in Cypress Cloud.

Flakiness  cypress\e2e\project-setup.cy.ts • 2 flaky tests • launchpad-e2e

View Output

Test Artifacts
... > can skip setup CT testing for an E2E project Test Replay Screenshots
... > skips the setup steps when choosing component tests to run Test Replay Screenshots
Flakiness  cypress\e2e\scaffold-component-testing.cy.ts • 1 flaky test • launchpad-e2e

View Output

Test Artifacts
scaffolding component testing > vuecli4vue2 > scaffolds component testing for Vue CLI 4 w/ Vue 2 project Test Replay Screenshots
Flakiness  cypress\e2e\migration.cy.ts • 1 flaky test • launchpad-e2e

View Output

Test Artifacts
Full migration flow for each project > completes journey for migration-e2e-plugins-modify-config Test Replay Screenshots

Review all test suite changes for PR #29391 ↗︎

@AtofStryker AtofStryker force-pushed the chore/update_internal_browser_images branch from e44d048 to 9366673 Compare April 24, 2024 15:35
@jennifer-shehane jennifer-shehane mentioned this pull request Apr 25, 2024
Closed
@jennifer-shehane
Copy link
Member

@AtofStryker This PR is failing with Firefox 115 in CI, but passes locally in Firefox 125. Curious to get the Firefox update in to see if it passes the tests to unblock this: #29224

@AtofStryker
Copy link
Contributor Author

Currently having issues inside the run-app-integration-tests job where the job seems to continuously fail on the same spots. Investigating, I can reproduce this in docker when running on an Ubuntu 22 amd64 machine via

downloading and bashing into the docker image/container from the cypress directory (attaches via workspace)

docker run --rm -it -v $PWD:/e2e -w /e2e --entrypoint bash cypress/browsers-internal:node18.17.1-chrome124-ff125

Now that we are in the docker container, run the e2e jobs but just these two specs. This seems to reproduce every time.

cd packages/app
yarn cypress:run:e2e -b chrome --spec=./cypress/e2e/runner/reporter-ct-mount-hover.cy.ts,./cypress/e2e/runner/reporter-ct-vite.errors.cy.ts

@AtofStryker
Copy link
Contributor Author

The issue looks related to the chrome update from 118 to 124. I am trying to narrow down to when the issue started occurring.

  • 124 (124.0.6367.91-1) -> has problem
  • 123 (123.0.6312.122-1) -> has problem
  • 122 (122.0.6261.128-1) -> has problem
  • 121 (121.0.6167.184-1) -> has problem
  • 120 (120.0.6099.224-1) -> has problem
  • 119 (119.0.6045.199-1) -> has problem
  • 118 (118.0.5993.117-1) -> works
  • 119 (119.0.6045.105-1) (closest revision to 118.0.5993.117-1) -> has problem

The issue looks identical to #28942 which we could not exactly figure out the cause of, but the flake likely existed in windows due to our windows jobs always installing chrome latest stable. The flake for these jobs seemed to get bad right around late October of 2023, which coincides with the Chrome 119 release date.

@AtofStryker
Copy link
Contributor Author 

yarn cypress:run:e2e -b chrome --spec=./cypress/e2e/runner/reporter-ct-mount-hover.cy.ts,./cypress/e2e/runner/reporter-ct-vite.errors.cy.ts --headed

reproduces the error every time, regardless of environment. You can add a fey .onlys to speed up the feedback loop with the result being the same. However, running just the spec by itself is fine, like this:

yarn cypress:run:e2e -b chrome --spec=./cypress/e2e/runner/reporter-ct-vite.errors.cy.ts --headed

My guess is there is something going on in the cri-client tab creation code that has changed here.

@AtofStryker AtofStryker force-pushed the chore/update_internal_browser_images branch 2 times, most recently from 3df3b9f to 1bc9b3c Compare April 26, 2024 17:34
@AtofStryker
Copy link
Contributor Author

I think I figured out what is going on. So when rehydraying the page with our inner cypress, we request to to not bucket our injected code into a keyed origin agent cluster via the Origin-Agent-Cluster=?0 header on the response. However, traffic to that proxy URL has already gone through without that header being set, and Chrome 119 and above wants the header configured for all requests on that origin. Because we don't, the request gets bucketed anyway, when it shouldn't, and doesn't allow us to set document.domain, which causes all sorts of issues

To work around this, we can either set chromeWebSecurity to false or we can set the response header on the proxy target for cypress in cypress tests. I have elected to do the later which is implemented in 324475e.

This fix also looks to be fixing the windows job failures we saw in #28942, which I will re open and link here, which enables recording again for the windows cypress in cypress jobs 🎉

@AtofStryker AtofStryker force-pushed the chore/update_internal_browser_images branch from 5957a09 to 98cfed6 Compare April 26, 2024 21:45
@AtofStryker AtofStryker changed the title chore: update browser internal images to chrome 124 and firefox 125 [run ci] chore: update browser internal images to chrome 124 and firefox 125 Apr 29, 2024
@AtofStryker AtofStryker mentioned this pull request Apr 29, 2024
Merged
@AtofStryker AtofStryker marked this pull request as ready for review April 29, 2024 19:12
mschile
mschile reviewed Apr 30, 2024
View reviewed changes
packages/proxy/lib/http/response-middleware.ts Outdated
// behavior starting in Chrome 119. The new behavior works like the following:
// If a page did not request an origin-keyed agent cluster, chrome will place it in one
// anyway because a previous request went through without the Origin-Agent-Cluster=false header set.
// At time of writing, documentation detailing this behavior has not been found.
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the documentation is here:

The browser will ensure that all pages from a given origin are either origin-keyed or they are not. This means that:

  • If the first page from an origin does not set the header, then no other pages from that origin will be origin-keyed, even if those other pages do set the header.
  • If the first page from an origin sets the header and is made origin-keyed, then all other pages from that origin will be origin-keyed whether they ask for it or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

referenced the docs and replaced the text in 96ee05c

@AtofStryker AtofStryker requested a review from mschile April 30, 2024 15:34
@AtofStryker AtofStryker merged commit 6937190 into develop Apr 30, 2024
86 of 93 checks passed
@AtofStryker AtofStryker deleted the chore/update_internal_browser_images branch April 30, 2024 18:16
jj497 pushed a commit to jj497/cypress that referenced this pull request May 5, 2024
@AtofStryker @jj497
chore: update browser internal images to chrome 124 and firefox 125 (c…
7622b56 
…ypress-io#29391)

* chore: update browser internal images to chrome 124 and firefox 125 [run ci]

* update screenshot sizes for chrome as they have changed in the chrome 124 and is ultimately out of our control since the options passed into the CLI are STILL not respected [run ci]

* fix cypress-in-cypress tests for chrome 119 and up

* remove windows app integration code added in 29198

* run windows build [run ci]

* fix some failing system tests [run ci]

* update non proxied png [run ci]

* fix system tests failing [run ci]

* fix afterEach [run ci]

* chore: update documentation in response middleware
@cypress-bot
Copy link
Contributor

cypress-bot bot commented May 8, 2024

Released in 13.9.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v13.9.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators May 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@cacieprins cacieprins cacieprins approved these changes

@mschile mschile mschile approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Flake: errors ui assertion failures
4 participants
@AtofStryker @jennifer-shehane @cacieprins @mschile