test: update Electron to v30 #30334
Merged
test: update Electron to v30 #30334
Conversation
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 comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
mxschmitt
force-pushed
the
electron-30
branch
2 times, most recently
from
335d8b3 to
24a81e0
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
dgozman
reviewed
Jun 20, 2024
| test.skip(isAndroid); | ||
| test.skip(isElectron); |
There was a problem hiding this comment.
Can we keep these ones with the electron/browser version check instead? I find such historical annotations very useful later on.
| @@ -88,8 +88,7 @@ it('should return headers', async ({ page, server, browserName }) => { | |||
| expect(response.request().headers()['user-agent']).toContain('WebKit'); | |||
| }); | |||
|
|
|||
| it('should get the same headers as the server', async ({ page, server, browserName, platform, isElectron, browserMajorVersion }) => { | |||
| it.skip(isElectron && browserMajorVersion < 99, 'This needs Chromium >= 99'); | |||
There was a problem hiding this comment.
Is there a strong reason to remove these annotations?
| it.skip(isAndroid, 'No files on Android'); | ||
| it.skip(browserName === 'firefox', 'Firefox does return null for file:// URLs'); | ||
| it.skip(mode.startsWith('service')); | ||
|
|
||
| const fileurl = url.pathToFileURL(asset('frames/two-frames.html')).href; | ||
| const response = await page.goto(fileurl); | ||
| if (isElectron || (browserName === 'chromium' && browserMajorVersion >= 99) || (browserName === 'webkit' && isWindows)) | ||
| if ((browserName === 'chromium' && browserMajorVersion >= 99) || (browserName === 'webkit' && isWindows)) |
There was a problem hiding this comment.
This change is a bit surprising. I'd think that new electron behaves like new chromium and returns 200.
dgozman
approved these changes
Jun 20, 2024
This comment has been minimized.
This comment has been minimized.
Test results for "tests others"5 failed 17765 passed, 476 skipped Merge workflow run. |
Test results for "tests 1"28374 passed, 648 skipped Merge workflow run. |
Test results for "tests 2"6 fatal errors, not part of any test 50 flaky206200 passed, 9154 skipped, 1382 did not run Merge workflow run. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Electron v29 is the last supported version as of today. It only receives security updates, so it doesn't get our backport of electron/typescript-definitions#264.
Most of the fixme's which were removed were related to
'error: Browser context management is not supported.'which is fixed since electron/electron#41718.Fixes #12096
Blocked by electron/typescript-definitions#264