-
Notifications
You must be signed in to change notification settings - Fork 356
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: update libraries #8463
chore: update libraries #8463
Conversation
✅ Deploy Preview for determined-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
5cb4827
to
1685b22
Compare
67a4279
to
7a84045
Compare
not sure why e2e keep failing only on webkit. Looks like playwright version doesnt matter and e2e works fine on local. probably this in the error?
maybe switch back to the playwright docker image might resolve the issue but not sure why it is changed to this determined/.circleci/real_config.yml Line 1515 in e06b472
|
24017be
to
16d2ae4
Compare
753d65e
to
2076fa0
Compare
2076fa0
to
f2879a1
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.
✅ thanks for working through the build issues
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.
Just a couple of questions from me, but not blockers. Approving the config change under the assumption that they weren't mistakes. :)
@@ -1513,14 +1513,15 @@ jobs: | |||
|
|||
test-e2e-react: | |||
docker: | |||
- image: cimg/node:20.9.0 | |||
- image: mcr.microsoft.com/playwright:v1.40.0-focal |
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.
It's probably quicker to run with the Circle convenience images. Does the Microsoft-hosted image have an advantage?
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.
Yes, for the playwright, cimg/node:20.9.0
is not straightforward to setup. We used to MS image, but at some point, it was replaced with cimg/node:20.9.0
with Playwright version downgrade. That's not what we want because that prevents us to use the latest playwright version.
- react-get-deps | ||
- run: npx playwright install --with-deps chrome firefox webkit | ||
- run: npx playwright install --with-deps && npx playwright install chrome |
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.
Did you mean to install Firefox and webkit in the first install command? Is that installing nothing, or is there a default file with stuff to install somewhere?
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.
This doc and this doc might be helpful.
We list browsers here
determined/webui/react/playwright.config.ts
Lines 26 to 40 in d8ed945
projects: [ | |
{ | |
name: 'chromium', | |
use: { ...devices['Desktop Chrome'], channel: 'chrome' }, | |
}, | |
{ | |
name: 'firefox', | |
use: { ...devices['Desktop Firefox'] }, | |
}, | |
{ | |
name: 'webkit', | |
use: { ...devices['Desktop Safari'] }, | |
}, |
and
npx playwright install --with-deps && npx playwright install chrome
install these dependencies somewhere. I also had this settings before, but its removed with cimg/node:20.9.0
replacement. That limits us to use the latest version of Playwright.
Description
WEB-1802
Test Plan
npm build/test/lint
should workCommentary (optional)
Checklist
docs/release-notes/
.See Release Note for details.
Ticket