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

ci: Fix Edge based tests #17347

Closed
wants to merge 1 commit into from

Conversation

danielhjacobs
Copy link
Contributor

Supersedes #17255

@torokati44
Copy link
Member

torokati44 commented Aug 2, 2024

Yay! But, just curious - where did this come from? Was it recommended by some wdio/GHA/Edge folks? Or have you just found it on your own? (Not that it's really important...)
Also, less directly related: I was wondering whether it would be beneficial to split the 3 browsers into 3 separate "steps", so it's easier to tell exactly which one has a headache in any given failed run (to be honest, I have never even attempted to parse anything useful out of the massive flood of log that these tests spew out - maybe that could be improved even?).

@danielhjacobs
Copy link
Contributor Author

danielhjacobs commented Aug 2, 2024

where did this come from? Was it recommended by some wdio/GHA/Edge folks? Or have you just found it on your own? (Not that it's really important...)

As I noticed and noted in actions/runner-images#10380, https://github.com/webdriverio-community/node-edgedriver/blob/7b2b44946f422b86e4c937d277f412a380394a1c/src/install.ts#L24 relies on the EDGEDRIVER_VERSION enviroment variable.

https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2204-Readme.md#environment-variables-1 says EDGEWEBDRIVER is defined as /usr/local/share/edge_driver. I ran echo "$($EDGEWEBDRIVER --version)" in https://github.com/danielhjacobs/ruffle/actions/runs/10220607522/job/28281327149 and it said "/usr/local/share/edge_driver: Is a directory".

So I then ran ls $EDGEWEBDRIVER in https://github.com/danielhjacobs/ruffle/actions/runs/10220644237/job/28281446113 and it said "Driver_Notes" and "msedgedriver"

So I then ran $EDGEWEBDRIVER/msedgedriver --version in https://github.com/danielhjacobs/ruffle/actions/runs/10220669515/job/28281539537 and it said "Microsoft Edge WebDriver 127.0.2651.72 (efc0f6216bd953a61b23596768d5f3dfbafa0b53)". The 4th word there, splitting on spaces, is "127.0.2651.72". echo "EDGEDRIVER_VERSION=$( $EDGEWEBDRIVER/msedgedriver --version | cut -f4 -d ' ' )" >> $GITHUB_ENV sets that to the GitHub environment variable EDGEDRIVER_VERSION.

Also, less directly related: I was wondering whether it would be beneficial to split the 3 browsers into 3 separate "steps", so it's easier to tell exactly which one has a headache in any given failed run

Maybe, but I don't know if wdio runs on the browsers in parallel. If so, that may be a lot slower.

@@ -92,11 +92,16 @@ jobs:
working-directory: web
run: npm test

- name: Set EDGEDRIVER_VERSION environment variable
Copy link
Member

Choose a reason for hiding this comment

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

I think a comment explaining what this is and why it's there and what it's ultimately achieving, would be appreciated - just for future maintainability and bus factor and similar reasons...

@torokati44
Copy link
Member

@danielhjacobs danielhjacobs deleted the fix-edge-ci branch August 5, 2024 21:16
@torokati44
Copy link
Member

Superseded by #17378.

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.

3 participants