Update Electron 35 & Node 22 - #1372
Conversation
gcsecsey
left a comment
gcsecsey
left a comment
There was a problem hiding this comment.
Thanks for making these changes @bcotrim for explaining the preparation for the workaround.
With the updated node version, I also needed to do an npm i for the project to start properly, I added this to the testing steps.
I couldn't find any regressions while testing the app using the updated versions. ✅
When running npm test, I'm getting these failing tests on this branch currently:
|
|
||
| const whatsNewModal = new WhatsNewModal( session.mainWindow ); | ||
| if ( await whatsNewModal.locator.isVisible( { timeout: 5000 } ) ) { | ||
| if ( await whatsNewModal.locator.isVisible( { timeout: 20000 } ) ) { |
There was a problem hiding this comment.
This is causing some tests to be flaky.
It took me a while to find the cause for it since it never happened locally.
One solution I can think of is skipping the modal in the app if we are running e2e.
If anyone has any better suggestions please let me know.
There was a problem hiding this comment.
I ended up implementing a way around it for E2E in 703613b
Let me know what you think.
7c5cddf to
6d29edd
Compare
|
Thanks for the review @gcsecsey |
gavande1
left a comment
gavande1
left a comment
There was a problem hiding this comment.
The app starts and functions normally for me. All tests pass. Looks good to me! 👍
4 participants
Related issues
Proposed Changes
Note: These changes are in preparation for the workaround mentioned here: electron/electron#41613 (comment)
Testing Instructions
nvm usenpm install && npm startPre-merge Checklist