Collection of test fixes #962
Conversation
Negative values for cursor position are not invalid
Because we need a visible window and the main window visibility must not be mutated by tests
There was a problem hiding this comment.
Pull request overview
This PR contains a collection of test fixes addressing flaky tests and improving test reliability. The changes include timeout adjustments, test refactoring to use isolated browser windows with proper cleanup, removal of platform-specific CI skips to re-enable tests, and simplification of some test assertions.
Key changes:
- Refactored several tests to create and destroy dedicated browser windows instead of reusing the main window, ensuring test isolation
- Increased printer enumeration timeout from 5 to 8 seconds to prevent timeout failures
- Removed coordinate assertions in cursor position tests that could fail in multi-monitor configurations
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| WebContentsTests.cs | Added ElectronNET.API using directive; refactored GetSetZoomLevel_check and Show_hide_visibility_roundtrip to use isolated windows with cleanup; commented out CI-specific test skips; simplified DevTools and UserAgent tests |
| ScreenTests.cs | Removed coordinate >= 0 assertions from GetCursorScreenPoint test to handle multi-monitor setups |
| BrowserWindowTests.cs | Removed unused System.Runtime.InteropServices import; refactored Show_hide_visibility_roundtrip and ReadyToShow tests to use isolated windows with proper cleanup |
| WebContents.cs | Increased GetPrintersAsync timeout from 5000ms to 8000ms to prevent timeout failures |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Re-enable some tests for CI as experiment
softworkz
force-pushed
the
submit_testupd
branch
from
b6ba78c to
88d2daa
Compare
|
I was able to remove the CI skip for all the tests. When there are inconsistent results, it has been almost always a timing issue. Yesterday, I've run the tests here 5 times in a row and all were always successful. For the future, there's a new |
|
Sounds great, the more tests run on CI the better. It is always annoying to remove tests when they start failing randomly on certain CI agents and I am glad you got to the bottom of it. |
|
The problem in this case was use of the MainWindow by multiple tests. There was one test which made it show and hide - and this changed the state and behavior of the window in subsequent tests, so other tests' results became dependent on the execution order (which is not guaranteed). Hence I changed a number of tests to create their own window to make them work independently. Like the ZoomLevel test for example is working with a hidden (never-been-shown) window on some platforms but on others it requires a window that has been shown at least once. |
3 participants
No description provided.