jupyterlab / lumino

Lumino is a library for building interactive web applications
https://lumino.readthedocs.io/
Other
598 stars 127 forks source link

Switch from karma to web-test-runner #704

Closed fcollonval closed 1 month ago

fcollonval commented 1 month ago

Fixes #610

Switch from karma to web-test-runner

I did not switch to jest to be able to test in browser directly and to not have to change the test (keep using mocha and chai).

Browsers are provided by playwright

fcollonval commented 1 month ago

The nice consequence is to get the macOS webkit tests green again

fcollonval commented 1 month ago

Thanks for the review @gabalafou

The fact that you need to change the tests locally is surprising as they work for me locally and on the CI. Does it happens with all browsers?

What is your environment? NodeJS, OS?

Note for yarn, the real version used should be independent of your installed version as we include it with the repo: https://github.com/jupyterlab/lumino/tree/main/.yarn/releases

Config is there: https://github.com/jupyterlab/lumino/blob/f26c85a9d5600ece3e2ad14b5e4505568bc025a9/.yarnrc.yml#L3

fcollonval commented 1 month ago

I was also wondering, would you mind leaving a note about how you created this pull request? Was it a lot of search/replace or were there some magic lerna commands that helped update the sub-package configs?

Yeah lots of smart search/replace...

fcollonval commented 1 month ago

Okay, this runs "out of the box" now, thank you! (By which I mean, I can do a fresh install of the repo and run the tests without getting stuck along the way.)

:tada:

fcollonval commented 1 month ago

I'm still getting the test failures I mentioned before. I checked that this doesn't happen before this PR so I think the failures are somehow related. I'm running Node 20.2.0 and macOS 14.4.1. Do you want to open an issue and punt on this for now, since the CI is fine?

If someone else could test it to see if you have an unlucky combination of software's or not. It would be nice.

Gentle ping to @JasonWeill, if you are on MacOS, would you have time to run the tests of this PR locally?

For reference, I'm on Debian 12 with Node.js 20.9.0.

fcollonval commented 1 month ago

Thanks a lot @gabalafou for all the tests.

JasonWeill commented 1 month ago

Running Lumino tests on tip-of-main pass, but on @fcollonval 's branch for this PR, they fail. I'm using a fresh Conda env on macOS Ventura 13.6.7.

Main branch ``` $ yarn test lerna notice cli v7.1.4 lerna info versioning independent lerna notice filter including "@lumino/!(example-)*" lerna info filter [ '@lumino/!(example-)*' ] ✔ @lumino/properties:test (5s) ✔ @lumino/domutils:test (5s) ✔ @lumino/keyboard:test (5s) ✔ @lumino/algorithm:test (5s) ✔ @lumino/collections:test (3s) ✔ @lumino/coreutils:test (3s) ✔ @lumino/virtualdom:test (3s) ✔ @lumino/signaling:test (3s) ✔ @lumino/messaging:test (3s) ✔ @lumino/disposable:test (3s) ✔ @lumino/dragdrop:test (3s) ✔ @lumino/polling:test (7s) ✔ @lumino/commands:test (8s) ✔ @lumino/widgets:test (6s) ✔ @lumino/application:test (3s) ✔ @lumino/datagrid:test (3s) ———————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————— > Lerna (powered by Nx) Successfully ran target test for 16 projects (30s) ```
fcollonval's branch ``` $ yarn test lerna notice cli v7.1.4 lerna info versioning independent lerna notice filter including "@lumino/!(example-)*" lerna info filter [ '@lumino/!(example-)*' ] > Lerna (powered by Nx) Running target test for 16 projects ✖ @lumino/algorithm:test > @lumino/algorithm@2.0.1 build:test > npm run clean:test && tsc --build tests && cd tests && rollup -c > @lumino/algorithm@2.0.1 clean:test > rimraf tests/lib tests/tsconfig.tsbuildinfo ./lib/index.spec.js → ./lib/bundle.test.js... (!) Circular dependencies ../../../node_modules/chai/lib/chai.js -> ../../../node_modules/chai/lib/chai/utils/index.js -> ../../../node_modules/chai/lib/chai/utils/addProperty.js -> ../../../node_modules/chai/lib/chai.js ../../../node_modules/chai/lib/chai.js -> ../../../node_modules/chai/lib/chai/utils/index.js -> ../../../node_modules/chai/lib/chai/utils/addMethod.js -> ../../../node_modules/chai/lib/chai.js ../../../node_modules/chai/lib/chai.js -> ../../../node_modules/chai/lib/chai/utils/index.js -> ../../../node_modules/chai/lib/chai/utils/overwriteProperty.js -> ../../../node_modules/chai/lib/chai.js ...and 3 more created ./lib/bundle.test.js in 369ms Chromium: |██████████████████████████████| 0/1 test files | 0 passed, 0 failed Running tests... Running 1 test files... tests/lib/bundle.test.js: ❌ browserType.launch: Executable doesn't exist at /Users/jweill/Library/Caches/ms-playwright/chromium-1117/chrome-mac/Chromium.app/Contents/MacOS/Chromium ╔═════════════════════════════════════════════════════════════════════════╗ ║ Looks like Playwright Test or Playwright was just installed or updated. ║ ║ Please run the following command to download new browsers: ║ ║ ║ ║ yarn playwright install ║ ║ ║ ║ <3 Playwright Team ║ ╚═════════════════════════════════════════════════════════════════════════╝ browserType.launch: Executable doesn't exist at /Users/jweill/Library/Caches/ms-playwright/chromium-1117/chrome-mac/Chromium.app/Contents/MacOS/Chromium ╔═════════════════════════════════════════════════════════════════════════╗ ║ Looks like Playwright Test or Playwright was just installed or updated. ║ ║ Please run the following command to download new browsers: ║ ║ ║ ║ yarn playwright install ║ ║ ║ ║ <3 Playwright Team ║ ╚═════════════════════════════════════════════════════════════════════════╝ at /Users/jweill/git/lumino/node_modules/@web/test-runner-playwright/dist/PlaywrightLauncher.js:94:74 at PlaywrightLauncher.getOrStartBrowser (/Users/jweill/git/lumino/node_modules/@web/test-runner-playwright/dist/PlaywrightLauncher.js:96:15) at PlaywrightLauncher.startSession (/Users/jweill/git/lumino/node_modules/@web/test-runner-playwright/dist/PlaywrightLauncher.js:41:36) at TestScheduler.startSession (/Users/jweill/git/lumino/node_modules/@web/test-runner-core/dist/runner/TestScheduler.js:103:70) at TestScheduler.runNextScheduled (/Users/jweill/git/lumino/node_modules/@web/test-runner-core/dist/runner/TestScheduler.js:83:30) at TestScheduler.schedule (/Users/jweill/git/lumino/node_modules/@web/test-runner-core/dist/runner/TestScheduler.js:47:14) at TestRunner.runTests (/Users/jweill/git/lumino/node_modules/@web/test-runner-core/dist/runner/TestRunner.js:92:28) at TestRunner.start (/Users/jweill/git/lumino/node_modules/@web/test-runner-core/dist/runner/TestRunner.js:65:22) at async startTestRunner (/Users/jweill/git/lumino/node_modules/@web/test-runner/dist/startTestRunner.js:46:9) Chromium: |██████████████████████████████| 1/1 test files | 0 passed, 0 failed Error while running tests. ———————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————— > Lerna (powered by Nx) Ran target test for 16 projects (4s) ✔ 0/1 succeeded [0 read from cache] ✖ 1/1 targets failed, including the following: - @lumino/algorithm:test ```
krassowski commented 1 month ago

@JasonWeill did you run yarn playwright install as instructed by the updated CONTRIBUTING.md?

JasonWeill commented 1 month ago

Sorry, I was looking at the old CONTRIBUTING.md file. After installing Playwright, then running the tests, I still see errors, but different errors.

yarn playwright install ``` $ yarn playwright install Removing unused browser at /Users/jweill/Library/Caches/ms-playwright/chromium-1067 Removing unused browser at /Users/jweill/Library/Caches/ms-playwright/firefox-1408 Removing unused browser at /Users/jweill/Library/Caches/ms-playwright/webkit-1860 Downloading Chromium 125.0.6422.26 (playwright build v1117) from https://playwright.azureedge.net/builds/chromium/1117/chromium-mac-arm64.zip 134.6 MiB [====================] 100% 0.0s Chromium 125.0.6422.26 (playwright build v1117) downloaded to /Users/jweill/Library/Caches/ms-playwright/chromium-1117 Downloading Firefox 125.0.1 (playwright build v1449) from https://playwright.azureedge.net/builds/firefox/1449/firefox-mac-13-arm64.zip 77.8 MiB [====================] 100% 0.0s Firefox 125.0.1 (playwright build v1449) downloaded to /Users/jweill/Library/Caches/ms-playwright/firefox-1449 Downloading Webkit 17.4 (playwright build v2003) from https://playwright.azureedge.net/builds/webkit/2003/webkit-mac-13-arm64.zip 66.6 MiB [====================] 100% 0.0s Webkit 17.4 (playwright build v2003) downloaded to /Users/jweill/Library/Caches/ms-playwright/webkit-2003 ```
yarn test ``` $ yarn test lerna notice cli v7.1.4 lerna info versioning independent lerna notice filter including "@lumino/!(example-)*" lerna info filter [ '@lumino/!(example-)*' ] ✔ @lumino/algorithm:test (8s) ✔ @lumino/coreutils:test (3s) ✔ @lumino/signaling:test (4s) ✔ @lumino/disposable:test (3s) ✔ @lumino/domutils:test (4s) ✔ @lumino/keyboard:test (3s) ✔ @lumino/virtualdom:test (4s) ✔ @lumino/dragdrop:test (4s) ✔ @lumino/commands:test (8s) ✔ @lumino/collections:test (3s) ✔ @lumino/messaging:test (4s) ✔ @lumino/properties:test (3s) ✖ @lumino/widgets:test > @lumino/widgets@2.3.2 build:test > npm run clean:test && tsc --build tests && cd tests && rollup -c > @lumino/widgets@2.3.2 clean:test > rimraf tests/lib tests/tsconfig.tsbuildinfo ./lib/index.spec.js → ./lib/bundle.test.js... (!) Circular dependencies ../../../node_modules/chai/lib/chai.js -> ../../../node_modules/chai/lib/chai/utils/index.js -> ../../../node_modules/chai/lib/chai/utils/addProperty.js -> ../../../node_modules/chai/lib/chai.js ../../../node_modules/chai/lib/chai.js -> ../../../node_modules/chai/lib/chai/utils/index.js -> ../../../node_modules/chai/lib/chai/utils/addMethod.js -> ../../../node_modules/chai/lib/chai.js ../../../node_modules/chai/lib/chai.js -> ../../../node_modules/chai/lib/chai/utils/index.js -> ../../../node_modules/chai/lib/chai/utils/overwriteProperty.js -> ../../../node_modules/chai/lib/chai.js ...and 3 more created ./lib/bundle.test.js in 817ms Chromium: |██████████████████████████████| 0/1 test files | 0 passed, 0 failed Running tests... Running 1 test files... tests/lib/bundle.test.js: ❌ @lumino/widgets > Menu > #handleEvent() > mousemove > should set the active index AssertionError: expected -1 to equal +0 + expected - actual --1 +0 at n. (tests/lib/bundle.test.js:39079:49) ❌ @lumino/widgets > Menu > #handleEvent() > mousemove > should open a child menu after a timeout AssertionError: expected -1 to equal +0 + expected - actual --1 +0 at n. (tests/lib/bundle.test.js:39094:49) ❌ @lumino/widgets > Menu > #handleEvent() > mouseleave > should reset the active index AssertionError: expected -1 to equal +0 + expected - actual --1 +0 at n. (tests/lib/bundle.test.js:39139:49) ❌ @lumino/widgets > Menu > #handleEvent() > mousedown > should not close the menu if on a child node AssertionError: expected false to equal true + expected - actual -false +true at n. (tests/lib/bundle.test.js:39160:48) ❌ @lumino/widgets > MenuBar > #handleEvent() > mousedown > should close an active menu AssertionError: expected -1 to equal +0 + expected - actual --1 +0 at n. (tests/lib/bundle.test.js:40419:48) ❌ @lumino/widgets > MenuBar > #handleEvent() > mousedown > should open an active menu AssertionError: expected false to equal true + expected - actual -false +true at n. (tests/lib/bundle.test.js:40437:48) ❌ @lumino/widgets > MenuBar > #handleEvent() > mousedown > should not close an active menu if not a left mouse press AssertionError: expected -1 to equal +0 + expected - actual --1 +0 at n. (tests/lib/bundle.test.js:40455:48) ❌ @lumino/widgets > MenuBar > #handleEvent() > mousedown > should not work on a menu bar item whose menu is empty AssertionError: expected +0 to equal -1 + expected - actual -0 +-1 at n. (tests/lib/bundle.test.js:40472:48) ❌ @lumino/widgets > MenuBar > #handleEvent() > mousemove > should open a new menu if a menu is already open AssertionError: expected +0 to equal 1 + expected - actual -0 +1 at n. (tests/lib/bundle.test.js:40487:48) Chromium: |██████████████████████████████| 1/1 test files | 926 passed, 9 failed, 11 skipped Finished running tests in 2.2s with 9 failed tests. ———————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————— > Lerna (powered by Nx) Ran target test for 16 projects (58s) ✔ 12/13 succeeded [0 read from cache] ✖ 1/13 targets failed, including the following: - @lumino/widgets:test ```
fcollonval commented 1 month ago

Thanks for testing the PR @JasonWeill

As the errors are reproducible, I applied the fix proposed by @gabalafou in e96e2e3. Tests are passing for me locally - so :crossed_fingers: for the CI.

fcollonval commented 1 month ago

Ok as the CI is green and the fix is working locally for @gabalafou

I'll merge this one.