-
Notifications
You must be signed in to change notification settings - Fork 8
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
Upgrade Playwright and Apache Arrow dependencies #2387
Closed
Closed
Changes from 4 commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
ba86576
Update Playwright to 1.46.0
jattasNI ba62623
Upgrade Apache Arrow to 17.0.0
jattasNI 7db5187
Re-enable wafer map tests on webkit
jattasNI 8006a17
Change files
jattasNI 781641b
Roll back playwright update
jattasNI a998625
Revert "Re-enable wafer map tests on webkit"
jattasNI b6db013
Merge branch 'main' into quarterly-deps-update
jattasNI 0991f56
Playwright 1.47.0
jattasNI cb72512
Change files
jattasNI 6a2db40
Reapply "Re-enable wafer map tests on webkit"
jattasNI 2ea5b61
Merge branch 'main' into quarterly-deps-update
jattasNI 262f9e8
Regenerate lock file after merge
jattasNI f3ba1d1
Merge branch 'main' into quarterly-deps-update
jattasNI ca80ceb
Regenerate package-lock after merge
jattasNI e082587
rimraf 🧟
jattasNI 03886af
ubuntu agent version
jattasNI 561bbf6
Merge branch 'main' into quarterly-deps-update
jattasNI a857e6d
Revert "ubuntu agent version"
jattasNI d7ae0d3
Skip wafermap tests that fail on webkit
jattasNI File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
7 changes: 7 additions & 0 deletions
7
change/@ni-jasmine-parameterized-6f3cf07b-2ad9-4dd6-ae3c-dcf4eb9af2d9.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "none", | ||
"comment": "Update Playwright to 1.46.0", | ||
"packageName": "@ni/jasmine-parameterized", | ||
"email": "[email protected]", | ||
"dependentChangeType": "none" | ||
} |
7 changes: 7 additions & 0 deletions
7
change/@ni-nimble-blazor-2efd2050-f0da-4897-a69a-773e1becdb3f.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "patch", | ||
"comment": "Upgrade Apache Arrow to 17.0.0", | ||
"packageName": "@ni/nimble-blazor", | ||
"email": "[email protected]", | ||
"dependentChangeType": "patch" | ||
} |
7 changes: 7 additions & 0 deletions
7
change/@ni-nimble-components-10d0f85e-6aac-4435-b810-2fe6f9b21008.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "patch", | ||
"comment": "Upgrade Apache Arrow to 17.0.0", | ||
"packageName": "@ni/nimble-components", | ||
"email": "[email protected]", | ||
"dependentChangeType": "patch" | ||
} |
7 changes: 7 additions & 0 deletions
7
change/@ni-spright-components-376932f7-5f42-46a0-ab03-dc96dac76ef4.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "none", | ||
"comment": "Update Playwright to 1.46.0", | ||
"packageName": "@ni/spright-components", | ||
"email": "[email protected]", | ||
"dependentChangeType": "none" | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Notes on test failures:
Playwright 1.46 contains Chrome 128, Playwright 1.47 contains Chrome 129 (but isn't available on Nuget yet)
Tests pass in headless Playwright Webkit and Firefox
Tests fail in headless Playwright Chromium 128
Tests pass in headed Playwright Chromium 128, headed macOS Chrome 128 or 129 if browser has focus
Tests fail in headed Playwright Chromium 128, headed macOS Chrome 129 (didn't try 128) if browser doesn't have focus
We saw some new test failures in SLE when we upgraded to Playwright 1.46. In that case it was that something changed behavior in Chrome when a test opened a new tab, and the fix was that tests shouldn't open new tabs. Bug 2823451: SystemLinkShared Renovate upgrade of Playwright is blocked due to file-lib tests which open new browser tabs
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.
Test failures are nearly all in table tests, mostly with keyboard nav
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.
Looks like at least some of these tests have always failed when running headed if the browser doesn't have focus. In that situation, calling
element.focus()
doesn't seem to have an effect. So the change with Chromium 128 seems to be that headless runs now behave like headed runs did.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.
🙈
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.
One other note I forgot to write down but probably should since I'm backburnering this for a while:
I need to re-check that the test failures on the CI build match the list above. When I first investigated this I thought that was the case but might have been mistaken. In at least one of the CI builds some of the failures were more like this:
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 testing, these failures are annoying!
I updated the PR to re-disable the wafer map tests related to #2169 that it had been enabling and updated the description to no longer resolve that issue. From a quick scan it looks like that covers most of the test failures you saw. @m-akinc @rajsite wanna try again and see if that covers all of them and also fixes the mysterious floating window?
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 a sync, full clean, build, and then tried
npm run validate
three times. During all the runs at some pointtest-webkit
just starts timing out and stops making any progress:I have to kill the WebKitWebProcess.exe, it restarts, and then tests start making progress again. But the test execution becomes extremely slow. Like 10 seconds per test even though the timing in jasmine says 0 ms per test.
The tests start going very slow timing out one after another every 5 seconds. Also mysterious window still shows up.
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.
I'm still seeing a lot of weird stuff from the WebKit tests:
Maybe running WebKit tests on Windows is more trouble than it is worth.
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.
I don't think we have a clear next step planned so I'll move back to draft for now
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.
Think we should split out the arrow upgrade. Will do that after #2474 is merged. I'll close this PR for now.