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: Google Chrome for testing - version not parsed correctly #28243

Closed
wants to merge 7 commits into from

Conversation

Crustum7
Copy link
Contributor

@Crustum7 Crustum7 commented Nov 5, 2023

Additional details

This change should ensure that Google Chrome for Testing is parsed so the version number can be used. The only issue is that it assumes that the version is stable which might lead to some issues. If there is a good way of checking what channel it is, using the version number, that could be used to make a more robust change.

Steps to test

Download a version of Google Chrome for Testing, for example: npx @puppeteer/browsers install chrome@canary

Add the path to Google Chrome for Testing: export PATH=$PATH:/full/path/to/chrome/directory/ (note: this is the path to the directory that the executable chrome lives in, not the path to the executable itself)

How has the user experience changed?

image
Before
image
After

PR Tasks

@cypress-app-bot
Copy link
Collaborator

@emilyrohrbough
Copy link
Member

emilyrohrbough commented Nov 6, 2023

@Crustum7 Can you share how you verified this change? I was following the How Can I Get Chrome For Testing Binaries install directions to validate this change.

I ran

~ npx @puppeteer/browsers install chrome@canary
Downloading chrome r121.0.6109.0 - 154.4 MB [====================] 100% 0.0s 
[email protected] /Users/emily/chrome/mac-121.0.6109.0/chrome-mac-x64/Google Chrome for Testing.app/Contents/MacOS/Google Chrome for Testing

and it appears the Test Chrome download landed in Users/emily/chrome instead of Users/emily/Applications like the Chrome detect logic expects for Mac machines. I started with Canary to see if we could get any other details from the download!

@Crustum7
Copy link
Contributor Author

Crustum7 commented Nov 6, 2023

Hi @emilyrohrbough! I added the directory of the chrome version I downloaded to my path.
export PATH=$PATH:/full/path/to/chrome/directory/
I'm on WSL which might work differently in that regard though.

@MikeMcC399
Copy link
Contributor

@emilyrohrbough

npx @puppeteer/browsers install --help

provides the following information:

--path Path to the root folder for the browser downloads and installation. The installation folder structure is
compatible with the cache structure used by Puppeteer. [string] [default: Current working directory]

Unlike other browser installations, Chrome for Testing does not attempt to install in a common location. It installs into the current working directory by default and uses a version-dependent path.

I was able to successfully test this PR using Ubuntu 22.04 with Node.js 18.15.0 and Chrome for Testing r120.0.6099.109.

npx @puppeteer/browsers install chrome@stable --path ~/chrome-for-testing
Downloading chrome r120.0.6099.109 - 150.2 MB [====================] 100% 0.0s 
[email protected] /home/mike/chrome-for-testing/chrome/linux-120.0.6099.109/chrome-linux64/chrome

It seems like it would be worthwhile to progress this PR.

@REPLicated
Copy link

REPLicated commented Feb 28, 2024

@Crustum7 : Regarding your question about the appropriate "channel" value: As far as I understand, Chrome for Testing is not released in "channels". Channels as a concept make sense to control how fast you get updates. As pointed out on https://developer.chrome.com/blog/chrome-for-testing, the idea is that you get versioned browser binaries and pin a particular version for reliable testing.

There seems to be an API endpoint returning information which Chrome for Testing version currently corresponds to each channel (see https://github.com/GoogleChromeLabs/chrome-for-testing#json-api-endpoints). However, given the currently installed Chrome for Testing version, it seems likely that it may not correspond to the current version in any channel.

@alexsch01
Copy link
Contributor

any update on this?

@MikeMcC399
Copy link
Contributor

@REPLicated

npx @puppeteer/browsers install chrome@stable

understands stable and beta. It also drops a file .metadata in the download directory, which currently would have the following contents:

{
  "aliases": {
    "stable": "127.0.6533.72",
    "beta": "127.0.6533.57"
  }
}

@Roemer
Copy link
Contributor

Roemer commented Jul 24, 2024

Any chance to get this merged somewhen?

@MikeMcC399
Copy link
Contributor

MikeMcC399 commented Nov 15, 2024

@mschile / @AtofStryker

The issue which this PR is supposed to resolve is causing problems in GitHub self-hosted runners.

After installing a selected Chrome browser version using the action browser-actions/setup-chrome, Cypress fails because "Chrome for Testing" is installed.

Can this PR be reviewed soon?

It will need rebasing.

@Crustum7

Are you still interested in this? It's been a year since you submitted your PR.

@alexsch01
Copy link
Contributor

Would be great to get this in the next release

Copy link
Contributor

@MikeMcC399 MikeMcC399 left a comment

Choose a reason for hiding this comment

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

I was able to test this PR successfully against Chrome for Testing stable on Ubuntu 24.04.1 LTS

mkdir ~/chrome-for-testing
cd ~/chrome-for-testing
npx @puppeteer/browsers install chrome@stable

with

--browser ~/chrome-for-testing/chrome/linux-131.0.6778.69/chrome-linux64/chrome

There was also no regression, running against Chrome.

For better maintainability, there should be a CircleCI test which runs against Chrome for Testing.

@alexsch01
Copy link
Contributor

What else needs to be done for this to be pushed to develop?

@AtofStryker
Copy link
Contributor

The team and I were talking about this yesterday, and we are thinking its best to recognize "Chrome For Testing" as it's own browser as we do this with different channels already. What I think remains here is that we would like the expression to not detect "Chrome for Testing" for Chrome release channel, and add a separate detection for "Chrome for Testing" .

This is a bit more involved than the current PR implements, but I am hoping to take a look at this soon as this PR as been open for almost a year and think we should considering getting this in sooner rather than later.

@mschile mschile assigned mschile and unassigned Crustum7 Dec 9, 2024
@mschile
Copy link
Contributor

mschile commented Dec 11, 2024

I'm closing this PR in favor of #30751.

@mschile mschile closed this Dec 11, 2024
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.

Google Chrome for testing - version not parsed correctly
10 participants