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 GitHub action headless browser tests #120

Merged
merged 2 commits into from Jul 31, 2023
Merged

Conversation

bgins
Copy link
Contributor

@bgins bgins commented Jul 26, 2023

Description

This PR implements the following changes:

  • Switch to wasm-pack test for headless browser testing

The 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

  • CI fix (non-breaking change that fixes an issue)

Test plan (required)

The run-tests actions should succeed.

@bgins bgins force-pushed the bgins/fix-browser-tests branch 13 times, most recently from bb4d743 to d7bf3f1 Compare July 26, 2023 22:07
@bgins bgins changed the title Fix GitHub action Chrome and Chromedriver setup Fix GitHub action headless browser tests Jul 26, 2023
@bgins bgins marked this pull request as ready for review July 26, 2023 22:32
@bgins bgins requested review from cdata and a team as code owners July 26, 2023 22:32
@bgins bgins self-assigned this Jul 26, 2023
Copy link
Member

@matheus23 matheus23 left a 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 :)

@cdata
Copy link
Member

cdata commented Jul 27, 2023

I'm a little bit unsure about depending on wasm-pack to test this. If I don't use wasm-pack, how much certainty do I have about these tests continuing to pass for me over time?

Separately, but related, this is the change where we investigated and fixed this issue in Noosphere: subconsciousnetwork/noosphere#514

@cdata cdata mentioned this pull request Jul 27, 2023
2 tasks
@bgins
Copy link
Contributor Author

bgins commented Jul 27, 2023

I'm a little bit unsure about depending on wasm-pack to test this. If I don't use wasm-pack, how much certainty do I have about these tests continuing to pass for me over time?

That's a fair point. I can't say with certainty that it will continue to work in the future.

Separately, but related, this is the change where we investigated and fixed this issue in Noosphere: subconsciousnetwork/noosphere#514

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.

@cdata
Copy link
Member

cdata commented Jul 27, 2023

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.

@bgins
Copy link
Contributor Author

bgins commented Jul 28, 2023

What would we think about using the setup-chromedriver GitHub action? (https://github.com/marketplace/actions/setup-chromedriver)

It looks like it is actively maintained. They updated with fixes a couple of days ago: nanasess/setup-chromedriver#192

Copy link
Member

@cdata cdata left a 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!

@cdata cdata merged commit 83528cc into main Jul 31, 2023
6 checks passed
@cdata cdata deleted the bgins/fix-browser-tests branch July 31, 2023 16:02
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.

None yet

3 participants