chore(integration-karma): migrate to webdriverio/vitest #4594

cardoso · 2024-10-01T12:38:45Z

Details

I'm pushing this PR as draft to better manage the diff and eventually clean it up for review.

It adds a yarn test:browser script to @lwc/integration-karma which runs a vitest config in browser mode.

The vitest config uses plugins and setup scripts that mimic the custom scripts and plugins used by karma, but are being written mostly from scratch.

The aim during migration is to ideally have minimal modification to tests, so the setup scripts polyfills jasmine and other karma globals. There's some exceptions: Some tests add custom matchers. I prefer to move those to the setup scripts, since polyfilling jasmine.addMatchers with expect.extend is more work than it's worth.

yarn test (karma) runs unmodified.
yarn test:browser (playwright/vitest) currently passes almost all tests but rewriting the plugins, scripts, and polyfills is still ongoing.

Once yarn test:browser is passing all tests, the plan is to remove karma completely, and transition this PR to ready for review. Most likely I'll leave the CI and coverage work for reviewers.

Same for yarn hydration:test (karma) and yarn hydration:browser (playwright/vitest)

If this works out, future tests can be written using facilities from vitest browser mode and playwright.

Old tests can be modernized as needed, and eventually the jasmine/karma polyfills can be removed.

Closes #3589

Current status

test-hydration (all)

 Test Files  136 passed (136)
      Tests  136 passed (136)
   Start at  21:54:55
   Duration  4.30s (transform 0ms, setup 3.76s, collect 5.58s, tests 263ms, environment 0ms, prepare 1.18s)

test (firefox)

 Test Files  314 passed (314)
      Tests  3213 passed | 12 skipped (3225)
   Start at  21:55:37
   Duration  31.14s (transform 0ms, setup 30.82s, collect 16.13s, tests 5.21s, environment 0ms, prepare 7.92s)

test (chromium)

 Test Files  1 failed | 313 passed (314)
      Tests  1 failed | 3212 passed | 12 skipped (3225)
   Start at  21:53:26
   Duration  11.09s (transform 0ms, setup 9.61s, collect 17.15s, tests 2.64s, environment 0ms, prepare 2.88s)

Does this pull request introduce a breaking change?

😮‍💨 No, it does not introduce a breaking change.

Does this pull request introduce an observable change?

🤞 No, it does not introduce an observable change.

GUS work item

…t-browser

cardoso

I'll do a clean up before continuing the migration work, but here's some notes on what's going on.

packages/@lwc/integration-karma/test/api/freezeTemplate/index.spec.js

packages/@lwc/integration-karma/test/polyfills/event-composed/index.spec.js

cardoso · 2024-10-01T12:53:09Z

packages/@lwc/integration-karma/vitest-helpers/test-hydrate.js

+window.HydrateTest = (function (lwc, testUtils) {
+    testUtils.setHooks({
+        sanitizeHtmlContent: (content) => content,
+    });


I haven't started migrating test-hydrate yet, although I'm keeping it in mind while rewriting the scripts and plugins.

cardoso · 2024-10-01T12:56:09Z

packages/@lwc/integration-karma/vitest-scripts/karma-configs/hydration/base.ts

+'use strict';
+
+import { resolve } from 'path';
+
+import options from '../../shared/options';
+const { ENABLE_SYNTHETIC_SHADOW_IN_HYDRATION, GREP, COVERAGE, COVERAGE_DIR_FOR_OPTIONS } = options;


Some scripts are just copied, ported to typescript, and kept for reference to rewrite / refactor. They aren't really used as it's too karma-specific.

cardoso · 2024-10-01T12:58:10Z

packages/@lwc/integration-karma/vitest-scripts/karma-plugins/Watcher.ts

+import path from 'node:path';
+import chokidar, { FSWatcher } from 'chokidar';
+
+export default class Watcher {
+    _suiteDependencyLookup: {
+        [x: string]: any;
+    };
+    _watcher: FSWatcher;
+    constructor(


I don't think this will be used as vitest handles it out of the box and has customization options.

packages/@lwc/integration-karma/vitest-scripts/karma-plugins/env.ts

cardoso · 2024-10-01T13:01:38Z

packages/@lwc/integration-karma/vitest-scripts/karma-plugins/hydration-tests.ts

+createHCONFIG2JSPreprocessor.$inject = ['config', 'logger', 'emitter'];
+
+export default {
+    'preprocessor:hydration-tests': ['factory', createHCONFIG2JSPreprocessor],
+};


hydration-tests plugin not yet rewritten.

cardoso

I made some changes the @lwc/rollup-plugin to simplify this migration. They could be removed before merge, or kept if there's some perf benefit in addition to simplifying the vitest plugin.

cardoso · 2024-10-01T13:37:28Z

packages/@lwc/rollup-plugin/src/index.ts

+    let {
+        rootDir,
+        modules = [], // TODO [#3370]: remove experimental template expression flag
+        experimentalComplexExpressions,
+        apiVersion,
+    } = pluginOptions;
+


Making some options configurable after the fact

cardoso · 2024-10-01T13:38:58Z

packages/@lwc/rollup-plugin/src/index.ts

+        api: {
+            updateOptions(options: {
+                rootDir: string;
+                modules?: ModuleRecord[];
+                experimentalComplexExpressions?: boolean;
+                apiVersion?: number;
+            }) {
+                rootDir = options.rootDir;
+
+                modules = [
+                    ...(options.modules ?? []),
+                    ...DEFAULT_MODULES,
+                    { dir: options.rootDir },
+                ];
+
+                if (options.experimentalComplexExpressions !== undefined) {
+                    experimentalComplexExpressions = options.experimentalComplexExpressions;
+                }
+
+                if (options.apiVersion !== undefined) {
+                    apiVersion = options.apiVersion;
+                }
+            },
+        },


Providing an api to change some options to avoid recreating the plugin for tests and fixtures which require different settings.

I've since found a cleaner approach which I might use as reference: https://github.com/vitejs/vite-plugin-vue/blob/main/packages/plugin-vue/src/index.ts#L192-L200

cardoso · 2024-10-01T14:10:34Z

packages/@lwc/integration-karma/test/api/getComponentDef/index.spec.js

-beforeAll(function () {
-    const getNormalizedFunctionAsString = (fn) => fn.toString().replace(/(\s|\n)/g, '');
-
-    jasmine.addMatchers({
-        toEqualWireSettings: function () {
-            return {
-                compare: function (actual, expected) {
-                    Object.keys(actual).forEach((currentKey) => {
-                        const normalizedActual = Object.assign({}, actual[currentKey], {
-                            config: getNormalizedFunctionAsString(actual[currentKey].config),
-                        });
-
-                        const normalizedExpected = Object.assign({}, expected[currentKey], {
-                            config: getNormalizedFunctionAsString(
-                                expected[currentKey].config || function () {}
-                            ),
-                        });
-
-                        expect(normalizedActual).toEqual(normalizedExpected);
-                    });
-
-                    return {
-                        pass: true,


This beforeAll() was just adding a custom matcher. I moved it to the test scripts in karma and implemented the equivalent in the vitest scripts.

…t-browser

nolanlawson · 2024-10-16T15:59:35Z

Thanks a lot for this PR! I haven't had a chance to review yet, but I have some quick thoughts:

We definitely need to get off of Karma, since it is deprecated
However, I'm concerned about using Vite, because one of the nice things about our current Karma+Jasmine setup is that all tests run in a single browser tab and single tab session. This means the tests run really fast, at the cost of forcing us to do manual cleanup between each test run. (Although this manual cleanup has other benefits – some tests actually confirm that the LWC engine is properly cleaning up state between tests, e.g. this test would fail if our cache keys were overbroad.)
I'm also concerned about using Playwright, since 1) AIUI it does not allow us to test multiple browser versions, and 2) it tests WebKit rather than "real" Safari. There are differences between WebKit and Safari and I'd rather we not delude ourselves by testing a browser that no real user is using.
Moving off of Jasmine also has some issues which you've highlighted, i.e. that you have to mock some Jasmine behavior. We have a lot of tests that hook into Jasmine's internals in subtle ways (e.g. here) and this could be tricky to mock.

Originally, I had planned on us migrating to something like Web Test Runner with Jasmine. That's not to say Vite isn't the right choice (we're already using it elsewhere), but that is what I had in mind.

cardoso · 2024-10-16T17:01:18Z

@nolanlawson thanks for looking into it!

re 2: Vitest also runs in a single tab and the test speed hasn't changed much (although hydration tests runs 2x faster with a tweak I made). I kept the clean up routine the same and mostly ported the ad-hoc plugins without much behavioral change.

re 3: we can definitely test different browser versions with playwright using providerOptions, but also we can use webdriverio and its own providerOptions. Admittedly, I haven't looked into any of this yet as my current focus is making all the different flags work (as well as the single failing test in chrome).

re 4: I'm not sure that case is currently causing any issues, at least not with the current flags, but I get your point.

There's a bunch of work I'm not sure how to handle yet, especially coverage and merging the results, because I want to first figure out all the flags and especially this failing test (only on chrome/webkit and maybe only on macOS. Firefox works fine.):

test/shadow-dom/ShadowRoot.elementsFromPoint

There's no rush on this PR (it can be left in draft and closed at some point), but I'd love your feedback especially regarding that failing test.

My current plan is to make some smaller PRs based on my learnings that would help transition into this setup or another setup.

nolanlawson · 2024-10-17T16:30:03Z

elementsFromPoint is a tricky one because the browsers disagree about how it should be implemented. (Browser folks have a lot of jokes about "the hit-testing spec" – there isn't one 🤣.) If it's failing, it could be due to some subtlety that isn't worth fixing in the test. The goal is just to make sure that our shadow DOM polyfill gives a result that is consistent with browsers.

also we can use webdriverio and its own providerOptions

I would much prefer this. WebDriver is the standard, we are already using webdriverio elsewhere, and I imagine this would play nicer with our current SauceLabs setup. The way that Playwright hacks around with CDP and a custom WebKit build makes me nervous.

Vitest also runs in a single tab and the test speed hasn't changed much (although hydration tests runs 2x faster with a tweak I made)

This is interesting, I wasn't aware of this... I assumed Vitest was doing some kind of test isolation, i.e. with browser tabs. If the tests take the same-ish amount of time to run, then my concern goes away. The current lack of test isolation isn't a hard requirement either.

Overall I really appreciate your work on this, but yeah this PR might end up being delayed as we work on other stuff. Right now it's kind of a crunch time for my team, so we may not be able to give this PR as much attention as it deserves. I really appreciate your help, though! Let me know if there's anything else I can answer to help unblock you. 🙂

…t-browser

cardoso · 2024-10-22T17:57:49Z

@nolanlawson your insight about elementsFromPoint made me laugh out loud 😂 . Truly, I could barely find any information on it.

I'm closing this PR for now as I found severe regression in test performance using webdriverio with vitest browser mode. It's not a first-class citizen there compared to playwright, maybe in a few months.

The performance impact is mostly due to the built-in test isolation, and the DOM clean up routines are not behaving the same way in order to run without isolation. Could they be improved to be more portable? Perhaps something similar to the hydration suite with a dedicated div per file/test?

Playwright runs the tests just as fast with the isolation, but I totally get why it's not a good option here.

I'll continue submitting improvements to the current karma tests. If you happen to have ideas, I'll be happy to take on any issues. #4600 is my current pet peeve, but it's so hard to reproduce.

Edit: About Web Test Runner, I like it, but I'm not confident it's a better option compared to keeping karma or rolling something custom when thinking about maintenance.

nolanlawson · 2024-10-22T18:21:41Z

@cardoso I really appreciate all your work on this. Uplifting our entire Karma test suite is a huge task. But even the small fixes you've found along the way have really helped improve our test quality! ❤️

I too was kind of disappointed when I searched the testing landscape for a good alternative to Karma. There just aren't many tools that avoid test isolation. I see the benefits of test isolation, but I really dislike it for our tests because 1) we are already diligent about cleaning things up, and 2) it's so fast!

Another option I (honestly) considered was to use raw WebDriverIO and just serve an HTML page with the Jasmine tests. We're mostly just using Rollup plugins, so this could even work in dev mode. But obviously we'd have to write a lot of glue code ourselves (e.g. for SauceLabs) which is not fun.

For the time being, I would not stress out too much on this – Karma is deprecated, but it's not going to stop working anytime soon. And maybe a good alternative will emerge as more teams try to move off of it.

cardoso added 18 commits September 26, 2024 09:13

chore(@lwc/integration-karma): migrate to vitest

ff6631c

chore(@lwc/integration-karma): tsify test-utils

4ea2f54

chore(@lwc/integration-karma): compatibility layer

86ed5f4

chore(@lwc/integration-karma): fix console

72f3ede

chore(@lwc/integration-karma): fix resolution

8474bec

chore(@lwc/integration-karma): resolve modules

a7c0e39

fix(@lwc/integration-karma): composedPath tests

8712d71

chore: add composedPath helpers

70bed5a

fix: event-methods

a0ed880

feat: use vitest mock

51cc9fa

fix: starting lwc vitest plugin from scratch

3ef0206

fix: simplified lwc vitest plugin 300+ tests pass

de2bb0a

fix: added jasmine.createSpy() compat

9fd8b3b

fix: implemented toEqualWireSettings

a3c3751

fix: template imports

9e7798f

fix: jasmine createSpy

d52038f

chore: update plugin

f5b4dc7

Merge branch 'master' of https://github.com/salesforce/lwc into vites…

3a32fcd

…t-browser

cardoso requested a review from a team as a code owner October 1, 2024 12:38

salesforce-cla bot added the cla:signed label Oct 1, 2024

cardoso changed the title ~~chore(integration-karma): migrate from karma/jasmine to playwright/vitest~~ chore(integration-karma): migrate to playwright/vitest Oct 1, 2024

cardoso marked this pull request as draft October 1, 2024 12:39

cardoso commented Oct 1, 2024

View reviewed changes

chore: remove screenshots

c197eb3

cardoso commented Oct 1, 2024

View reviewed changes

cardoso added 4 commits October 1, 2024 10:43

chore: remove more screenshots

096008b

chore: git ignore screenshots

68d7ea1

fix: jasmine createSpy

798a969

fix: revert freezeTemplate test

486a949

cardoso commented Oct 1, 2024

View reviewed changes

cardoso added 11 commits October 14, 2024 15:59

chore: shorten test names

1897f92

fix: use transform to replace NODE_ENV

ca1f992

chore: consolidate plugins

263dd0f

fix: handling API_VERSION env var in tests

a7d7330

chore: begin support for coverage

5acb1b5

chore: add workspace file

0851ec8

fix: scripts running all tests

6806648

fix: enable source maps

69c72c9

fix: enable excludeAfterRemap

cc36de4

Merge branch 'master' of https://github.com/salesforce/lwc into vites…

29ec4e5

…t-browser

fix: move @vitest/browser dependency to root

3b30a10

cardoso added 11 commits October 17, 2024 17:13

chore: transition to webdriverio

b8d5312

Merge branch 'master' of https://github.com/salesforce/lwc into vites…

066549d

…t-browser

Merge branch 'master' of https://github.com/salesforce/lwc into vites…

db88d27

…t-browser

feat: webdriverio browser config

799ee56

Merge branch 'master' of https://github.com/salesforce/lwc into vites…

41990c0

…t-browser

fix: import framework files via testerScripts

39e60e3

fix: use global LWC in hydration tests

ce70151

Merge branch 'master' of https://github.com/salesforce/lwc into vites…

7f838dc

…t-browser

Merge branch 'master' of https://github.com/salesforce/lwc into vites…

73a6a14

…t-browser

Merge branch 'master' of https://github.com/salesforce/lwc into vites…

0b58515

…t-browser

fix: add toHaveBeenCalledOnceWith matcher

3bfc774

cardoso closed this Oct 22, 2024

cardoso mentioned this pull request Jan 19, 2025

chore: upgrade from vitest 2.1.8 to vitest 3.0.3 #5144

Merged

cardoso changed the title ~~chore(integration-karma): migrate to playwright/vitest~~ chore(integration-karma): migrate to webdriverio/vitest Jan 24, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(integration-karma): migrate to webdriverio/vitest #4594

chore(integration-karma): migrate to webdriverio/vitest #4594

cardoso commented Oct 1, 2024 •

edited

Loading

cardoso left a comment

cardoso Oct 1, 2024

cardoso Oct 1, 2024

cardoso Oct 1, 2024

cardoso Oct 1, 2024

cardoso left a comment

cardoso Oct 1, 2024

cardoso Oct 1, 2024

cardoso Oct 1, 2024 •

edited

Loading

nolanlawson commented Oct 16, 2024

cardoso commented Oct 16, 2024

nolanlawson commented Oct 17, 2024

cardoso commented Oct 22, 2024 •

edited

Loading

nolanlawson commented Oct 22, 2024

chore(integration-karma): migrate to webdriverio/vitest #4594

chore(integration-karma): migrate to webdriverio/vitest #4594

Conversation

cardoso commented Oct 1, 2024 • edited Loading

Details

Current status

test-hydration (all)

test (firefox)

test (chromium)

Does this pull request introduce a breaking change?

Does this pull request introduce an observable change?

GUS work item

cardoso left a comment

Choose a reason for hiding this comment

cardoso Oct 1, 2024

Choose a reason for hiding this comment

cardoso Oct 1, 2024

Choose a reason for hiding this comment

cardoso Oct 1, 2024

Choose a reason for hiding this comment

cardoso Oct 1, 2024

Choose a reason for hiding this comment

cardoso left a comment

Choose a reason for hiding this comment

cardoso Oct 1, 2024

Choose a reason for hiding this comment

cardoso Oct 1, 2024

Choose a reason for hiding this comment

cardoso Oct 1, 2024 • edited Loading

Choose a reason for hiding this comment

nolanlawson commented Oct 16, 2024

cardoso commented Oct 16, 2024

nolanlawson commented Oct 17, 2024

cardoso commented Oct 22, 2024 • edited Loading

nolanlawson commented Oct 22, 2024

cardoso commented Oct 1, 2024 •

edited

Loading

cardoso Oct 1, 2024 •

edited

Loading

cardoso commented Oct 22, 2024 •

edited

Loading