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: update chromedriver download URL, resolve test issues #1391

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

kade-robertson
Copy link

@kade-robertson kade-robertson commented May 14, 2024

This fixes a few things to get CI working for the repo again:

  • The chromedriver releases were previously 404ing, causing these tests to fail. The URL for where these are hosted was changed, so it's just a simple update to get this working again.
  • In later Cargo releases, the output format for the "Finished..." log line changed. I've fixed this by making matches_cargo accept both the old and new formats.
  • With MacOS 14 (and by extension, M1) runners becoming the macos-latest default, the current geckodriver setup fails for that platform. It's not entirely obvious to me why at this point. Downgrading to the macos-13 runner has these tests working again. This is probably the most.. questionable change as it'd be better to track down why the latest runners don't work. Edit: It seems like the only thing preventing macOS from working was that I needed to manually install Firefox. I'm guessing this was available in the previous macOS runners by default.

Make sure these boxes are checked! 📦✅

  • You have the latest version of rustfmt installed
$ rustup component add rustfmt
  • You ran cargo fmt on the code base before submitting
  • You reference which issue is being closed in the PR text

✨✨ 😄 Thanks so much for contributing to wasm-pack! 😄 ✨✨

Closes #1390

@kade-robertson kade-robertson changed the title fix: update chromedriver download URL, update fix: update chromedriver download URL, resolve test issues May 14, 2024
@kade-robertson
Copy link
Author

Seems like there's some unfortunate caching issues I'll have to dig into.

@drager
Copy link
Member

drager commented May 23, 2024

Seems like there's some unfortunate caching issues I'll have to dig into.

Thank you for looking into this! Appreciated!

@kade-robertson
Copy link
Author

Thank you for looking into this! Appreciated!

This issue seems to be resolved now for the sake of this PR. However, in general I've found some consistency issues with trying to determine what paths to cache when manually in many of my other projects. Are you open to switching to something like Swatinem/rust-cache or mozilla-actions/sccache-action? I've used both successfully quite a bit, though recently all of my projects have moved to the latter. If so I can open up another PR after this is cleared to merge to switch out the caching.

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.

CI is not working
2 participants