-
Notifications
You must be signed in to change notification settings - Fork 15
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 GitHub action headless browser tests #120
Conversation
bb4d743
to
d7bf3f1
Compare
d7bf3f1
to
235f6c9
Compare
235f6c9
to
4c0994e
Compare
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.
I think this looks good!
I'll tentatively approve, but won't press the merge button in case noosphere folks have opinions on this :)
I'm a little bit unsure about depending on Separately, but related, this is the change where we investigated and fixed this issue in Noosphere: subconsciousnetwork/noosphere#514 |
That's a fair point. I can't say with certainty that it will continue to work in the future.
Regarding the Noosphere implementation, how can you be sure the retrieved Chromedriver version will match the Chrome version provided by apt-get? I was tinkering with this yesterday and had hoped this would work out: - name: Setup Chrome and Chromedriver
run: |
CHROME_INSTALL_OUTPUT=( $(npx @puppeteer/browsers install chrome@stable) )
CHROMEDRIVER_INSTALL_OUTPUT=( $(npx @puppeteer/browsers install chromedriver@stable) )
CHROME_PATH=${CHROME_INSTALL_OUTPUT[1]}
CHROMEDRIVER_PATH=${CHROMEDRIVER_INSTALL_OUTPUT[1]}
sudo mv $CHROME_PATH $CHROMEDRIVER_PATH /usr/local/bin This would install Chrome for Testing and Chromedriver provided by the Chrome team at guaranteed matching versions. But unfortunately it didn't, and I wasn't able to get to the bottom of why. |
I guess the only way to be sure is to also use the Chrome binary distribution linked from the same source we're pulling chromedriver from. It should be doable following the same pattern we use for chromedriver, although annoying to find a convenient install location for the binary I suppose. |
What would we think about using the It looks like it is actively maintained. They updated with fixes a couple of days ago: nanasess/setup-chromedriver#192 |
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.
LGTM, thanks for taking care of this!
Description
This PR implements the following changes:
wasm-pack test
for headless browser testingThe Chrome team recently released Chrome for Testing: https://developer.chrome.com/blog/chrome-for-testing/. As a part of the change, they have ended support for the endpoint we use to determine the required version of Chromedriver.
This PR simplifies our set up by switching to
wasm-pack test
, which automatically installs the correct version of Chromedriver.Type of change
Test plan (required)
The
run-tests
actions should succeed.