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

chore: install playwright deps before playwright #86

Merged
merged 5 commits into from
Jan 27, 2025
Merged

Conversation

43081j
Copy link
Contributor

@43081j 43081j commented Jan 26, 2025

i think this will sort what you're trying to do @keithamus

@43081j
Copy link
Contributor Author

43081j commented Jan 26, 2025

nevermind this actually just uncovered another error - that some of these dependencies don't exist in whatever distro we're using...

@keithamus
Copy link
Member

I think deps need to be updated also.

We currently install playwright before its dependencies, and this seems
to be causing our CI to fail since a new playwright release at some
point.

Switching them around should fix it.
@43081j 43081j force-pushed the writeplay branch 2 times, most recently from 2a8b9e7 to 964d028 Compare January 26, 2025 22:35
@43081j
Copy link
Contributor Author

43081j commented Jan 26, 2025

lets sort all the renovate PRs first and see if this runs once i update the branch

@keithamus keithamus closed this Jan 26, 2025
@keithamus keithamus reopened this Jan 26, 2025
@keithamus
Copy link
Member

@43081j doesn't look like updating all the deps fixed this issue either 😢

@43081j
Copy link
Contributor Author

43081j commented Jan 26, 2025

renovate only looks at our direct dependencies i think

so i did an npm up playwright, which updates the version of playwright underneath WTR. that then seems to have fixed it

but now we have that same old rollup lockfile problem.

Error: Cannot find module @rollup/rollup-win32-x64-msvc. npm has a bug related to optional dependencies (npm/cli#4828). Please try npm i again after removing both package-lock.json and node_modules directory.

@43081j
Copy link
Contributor Author

43081j commented Jan 26, 2025

reinstalling web test runner seems to have fixed it

big lockfile diff but looks ok

@keithamus
Copy link
Member

Nice work!

@keithamus keithamus merged commit 6531c62 into main Jan 27, 2025
6 of 7 checks passed
@keithamus keithamus deleted the writeplay branch January 27, 2025 20:59
@meduzen
Copy link

meduzen commented Jan 28, 2025

@43081j @keithamus Side note: I think you can merge the two commands into one using the --with-deps flag. I don’t know if it’ll work for your use case, but that's what I do in a GitHub action and it works, and it’s also what’s mentioned on Playwright CI doc.

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