Skip to content
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

playwright v1.28 breaks electron tests #2969

Closed
khassel opened this issue Nov 22, 2022 · 6 comments
Closed

playwright v1.28 breaks electron tests #2969

khassel opened this issue Nov 22, 2022 · 6 comments

Comments

@khassel
Copy link
Collaborator

khassel commented Nov 22, 2022

node@46e0815dd8b0:/opt/magic_mirror$ npx jest tests/electron/env_spec.js
 FAIL   electron  tests/electron/env_spec.js
  Electron app environment
     should open browserwindow (470 ms)
  Development console tests
     should open browserwindow and dev console (439 ms)

   Electron app environment  should open browserwindow

    TypeError: Cannot read properties of null (reading 'locator')

      37 | exports.getElement = async (selector) => {
      38 |      expect(global.page);
    > 39 |      let elem = global.page.locator(selector);
         |                             ^
      40 |      await elem.waitFor();
      41 |      expect(elem).not.toBe(null);
      42 |      return elem;

      at Object.locator [as getElement] (tests/electron/helpers/global-setup.js:39:25)
      at Object.getElement (tests/electron/env_spec.js:13:32)

so we should not update at the moment.

@khassel
Copy link
Collaborator Author

khassel commented Nov 29, 2022

see microsoft/playwright#18928

@mxschmitt
Copy link

That would be a fix for 1.28:

diff --git a/package.json b/package.json
index b805bb9f..6f3038c9 100644
--- a/package.json
+++ b/package.json
@@ -59,7 +59,7 @@
 		"jsdom": "^20.0.0",
 		"lodash": "^4.17.21",
 		"nyc": "^15.1.0",
-		"playwright": "^1.26.1",
+		"playwright": "^1.28.1",
 		"prettier": "^2.7.1",
 		"pretty-quick": "^3.1.3",
 		"sinon": "^14.0.0",
diff --git a/tests/electron/env_spec.js b/tests/electron/env_spec.js
index f962949c..2523e6f4 100644
--- a/tests/electron/env_spec.js
+++ b/tests/electron/env_spec.js
@@ -1,5 +1,6 @@
 // see https://playwright.dev/docs/api/class-electronapplication
 
+const events = require("events");
 const { _electron: electron } = require("playwright");
 
 let electronApp = null;
@@ -16,6 +17,7 @@ describe("Electron app environment", function () {
 	});
 
 	it("should open browserwindow", async function () {
+		await electronApp.firstWindow();
 		expect(await electronApp.windows().length).toBe(1);
 		const page = await electronApp.firstWindow();
 		expect(await page.title()).toBe("MagicMirror²");
@@ -36,6 +38,7 @@ describe("Development console tests", function () {
 	});
 
 	it("should open browserwindow and dev console", async function () {
+		while (electronApp.windows().length < 2) await events.once(electronApp, "window");
 		const pageArray = await electronApp.windows();
 		expect(pageArray.length).toBe(2);
 		for (const page of pageArray) {

@khassel
Copy link
Collaborator Author

khassel commented Dec 5, 2022

Thank you @mxschmitt for your hints but it still doesn't work.

await electronApp.firstWindow(); throws

electronApplication.firstWindow: Timeout 30000ms exceeded while waiting for event "window"`

So the problem seems to be that no electron window event is created. Same code works if I downgrade to playwright v1.27.1.

rejas pushed a commit that referenced this issue Dec 8, 2022
Changes:
- as discussed in #2903: update to electron v22 (we can revert it before
next release if we find any problems)
- update other dependencies
- set playwright to version v1.27.1 until #2969 is solved
@khassel
Copy link
Collaborator Author

khassel commented Dec 20, 2022

this will be (hopefully) fixed with next playwright version v1.30 (see microsoft/playwright#19412)

@mxschmitt
Copy link

We'll cherry-pick it to 1.29.1

@rejas
Copy link
Collaborator

rejas commented Dec 22, 2022

Fixed

@rejas rejas closed this as completed Dec 22, 2022
MichMich added a commit that referenced this issue Jan 1, 2023
## [2.22.0] - 2023-01-01

Thanks to: @angeldeejay, @buxxi, @dariom, @dWoolridge,
@KristjanESPERANTO, @MagMar94, @naveensrinivasan, @retroflex, @SkySails
and @tom.

Special thanks to @khassel, @rejas and @sdetweil for taking over most
(if not all) of the work on this release as project collaborators. This
version would not be there without their effort. Thank you!

### Added

- Added test for remoteFile option in compliments module
- Added hourlyWeather functionality to Weather.gov weather provider
- Removed weatherEndpoint definition from weathergov.js (not used)
- Added css class names "today" and "tomorrow" for default calendar
- Added Collaboration.md
- Added new github action for dependency review (#2862)
- Added a WeatherProvider for Open-Meteo
- Added Yr as a weather provider
- Added config options "ignoreXOriginHeader" and
"ignoreContentSecurityPolicy"

### Removed

- Removed usage of internal fetch function of node until it is more
stable

### Updated

- Cleaned up test directory (#2937) and jest config (#2959)
- Wait for all modules to start before declaring the system ready
(#2487)
- Updated e2e tests (moved `done()` in helper functions) and use es6
syntax in all tests
- Updated da translation
- Rework weather module
- Make sure smhi provider api only gets a maximum of 6 digits
coordinates (#2955)
  - Use fetch instead of XMLHttpRequest in weatherprovider (#2935)
  - Reworked how weatherproviders handle units (#2849)
  - Use unix() method for parsing times, fix suntimes on the way (#2950)
  - Refactor conversion functions into utils class (#2958)
- The `cors`-method in `server.js` now supports sending and recieving
HTTP headers
- Replace `&hellip;` by `…`
- Cleanup compliments module
- Updated dependencies including electron to v22 (#2903)

### Fixed

- Correctly show apparent temperature in SMHI weather provider
- Ensure updatenotification module isn't shown when local is _ahead_ of
remote
- Handle node_helper errors during startup (#2944)
- Possibility to change FontAwesome class in calendar, so icons like
`fab fa-facebook-square` works.
- Fix cors problems with newsfeed articles (as far as possible), allow
disabling cors per feed with option `useCorsProxy: false` (#2840)
- Tests not waiting for the application to start and stop before
starting the next test
- Fix electron tests failing sometimes in github workflow
- Fixed gap in clock module when displayed on the left side with
displayType=digital
- Fixed playwright issue by upgrading to v1.29.1 (#2969)

Signed-off-by: naveen <172697+naveensrinivasan@users.noreply.github.com>
Co-authored-by: Karsten Hassel <hassel@gmx.de>
Co-authored-by: Malte Hallström <46646495+SkySails@users.noreply.github.com>
Co-authored-by: Veeck <github@veeck.de>
Co-authored-by: veeck <michael@veeck.de>
Co-authored-by: dWoolridge <dwoolridge@charter.net>
Co-authored-by: Johan <jojjepersson@yahoo.se>
Co-authored-by: Dario Mratovich <dario_mratovich@hotmail.com>
Co-authored-by: Dario Mratovich <dario.mratovich@outlook.com>
Co-authored-by: Magnus <34011212+MagMar94@users.noreply.github.com>
Co-authored-by: Naveen <172697+naveensrinivasan@users.noreply.github.com>
Co-authored-by: buxxi <buxxi@omfilm.net>
Co-authored-by: Thomas Hirschberger <47733292+Tom-Hirschberger@users.noreply.github.com>
Co-authored-by: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com>
Co-authored-by: Andrés Vanegas Jiménez <142350+angeldeejay@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants