chore: migrate Karma to Web Test Runner - #5121
Conversation
…/dequelabs/axe-core into migrate-karma-to-web-test-runner
| if (logger) { | ||
| logger(...args); | ||
| } else if (typeof console === 'object' && console.log) { | ||
| console.log(...args); |
There was a problem hiding this comment.
Who needs Function.prototype.apply when we can spread now? :D
There was a problem hiding this comment.
If there is ever the concern of potentially exceeding the maximum call stack size...
| elementInternalsStartTime = performance.now(); | ||
| return Promise.resolve([]); | ||
| return new Promise(res => { | ||
| setTimeout(() => res([]), 100); |
There was a problem hiding this comment.
Firefox would resolve this almost instantly so the two times were equal. Adding a timeout to ensure we can track one starting after the other
| </form>`; | ||
| const node = fixture.querySelector('gather-internals-element'); | ||
| const form = fixture.querySelector('#form'); | ||
| node._internals.form = form; |
There was a problem hiding this comment.
This would fail in strict mode. It's automatically set by the browser and is a read only property
|
Doing some re-runs to just check for flakiness. If they pass all clear, then this all looks good to me. |
Garbee
left a comment
There was a problem hiding this comment.
A few re-runs of the tests and no failures. It looks good to me.
WilcoFiers
left a comment
There was a problem hiding this comment.
Summary
Solid migration from Karma to Web Test Runner. The custom DirectSeleniumLauncher is a reasonable fix for WTR’s iframe wrapper, and the axe._setLogger / integration-test generation changes address real gaps exposed by the move. Garbee’s re-runs give me confidence in stability.
A few follow-ups would help contributors and reduce confusion—mostly docs and small cleanups, nothing that blocks merge from my side.
Important
1. Update contributor docs for WTR
doc/developer-guide.md, CONTRIBUTING.md, and test/integration/rules/README.md still describe Karma and the old testDirs debug API.
Please update them to match this PR:
- WTR +
--manualfortest:debug(pressD, file list landing page) --files 'test/core/**/*.js'instead oftestDirs=core- Rule integration tests need
npm run build:integration-testsfirst - WTR behavior:
.onlyonly within a single test file
Also worth replacing “Mocha / Karma” in the watch section with WTR.
2. Remove unused @web/test-runner-selenium
package.json lists @web/test-runner-selenium, but test/wtr.config.mjs uses selenium-webdriver directly with DirectSeleniumLauncher. Consider dropping the unused dependency to avoid confusion about which Selenium integration is canonical.
3. Fix misleading comment in wtr.debug.config.mjs
test/wtr.debug.config.mjs says “The browser opens automatically,” but the PR description notes that with --manual you press D and land on a file list. Aligning the header comment with that flow would save future debug sessions.
Suggestions
4. Update Gruntfile clean path for integration tests
Gruntfile.js still cleans tmp/integration-tests.js, while generation now writes to tmp/integration-tests/. Updating clean.tests to the directory (or relying on the existing tmp ignore) keeps grunt clean accurate.
5. Document axe._setLogger as test-only
lib/core/log.js / lib/core/core.js — a short @private / “for tests only” JSDoc on setLogger and axe._setLogger would clarify this isn’t supported public API (no axe.d.ts change needed if it stays internal).
6. Consider surfacing errors in the Selenium session chain
this._chain = navigate.then(() => sessionDone).catch(() => {});Swallowing navigation failures may make Selenium/WTR issues harder to diagnose. Either rethrow/log, or add a one-line comment explaining why the chain must continue after failure.
|
That was Cursor. I checked all of those, and agree. Nothing for myself to add. |
|
I don't disagree with most of what Wilco's AI review pointed out. However, I would say we just ignore the grunt-related bit since we're already talking about removing that? It's basically a race condition of which lands first and how to handle it. But once one lands the other has to update and account for things anyways. So, seems moot to spend cycles on it. (Although, it seems like it could be a one string change. In which case, just do it.) |
Migrating Karma to use Web Test Runner (WTR). Out of the box it has some problems that needed to be addressed, and some things we'll need to get use to using a new test runner. * WTR uses Playwright by default, but Playwright doesn't support Firefox Nightly. We'll use the Selenium driver in order to support that * However the Selenium driver automatically runs each test inside an iframe, which we do not want (and also breaks our tests). The Selenium driver doesn't have an option to disable this, so had to hack a way around this to ensure each test ran in the root document and no iframe * WTR runs each test file in an isolated Mocha context, so using `.only` no longer works across test files, just within the file it's used in. A bit annoying, but we can get use to this (tried to hack a way around this but it became way too complicated and broke a bunch of other stuff). `develop` still will watch files and run the changed test file for us * WTR `--manual` is used to replace `test:debug`. This didn't have a UI at first, so added the `html` style mocha reporter that we are use to. The only difference now is that the browser doesn't open automatically (have to press 'D') and that the landing page is a list of links to each test file * WTR doesn't have a preprocess step like Karma had, so we had to move it to it's own file and do the preprocessing for integration tests before we run them Additionally: * Noticed that our axe.log stubbing in various tests weren't working since we moved to ES6 modules (the code directly calls `log` and doesn't go through `axe.log`), this meant the asserts testing the messages weren't being run at all. So I fixed that by adding a `axe._setLogger` helper that would let us designate the logger stub. This allows us to both stub the logger or suppress it (so we don't have to see performance timer logs in tests, etc.) * Puppeteer was [failing a lot recently](https://github.com/dequelabs/axe-core/actions/runs/26545263689/job/78398503218) so fixed that while I was here * WTR can support both our current test loading (relying on global axe and properties) and es6 imports to the `lib` directory. I updated `performance-timer` to show this being possible. This means we should be able to remove the `_thisWillBeDeletedDoNotUse` property that was added just to get things to the test and we can import those directly in the tests that need them. However, this isn't something we can do right away as the imported file is a different context from the same file being imported in axe (since we bundle the axe files together). This means file scope/state is different so we can't import the new `setLogger` function directly in tests and use it as it doesn't set the axe.js bundled `logger` variable scope. Closes dequelabs#5115
Migrating Karma to use Web Test Runner (WTR). Out of the box it has some problems that needed to be addressed, and some things we'll need to get use to using a new test runner.
.onlyno longer works across test files, just within the file it's used in. A bit annoying, but we can get use to this (tried to hack a way around this but it became way too complicated and broke a bunch of other stuff).developstill will watch files and run the changed test file for us--manualis used to replacetest:debug. This didn't have a UI at first, so added thehtmlstyle mocha reporter that we are use to. The only difference now is that the browser doesn't open automatically (have to press 'D') and that the landing page is a list of links to each test fileAdditionally:
logand doesn't go throughaxe.log), this meant the asserts testing the messages weren't being run at all. So I fixed that by adding aaxe._setLoggerhelper that would let us designate the logger stub. This allows us to both stub the logger or suppress it (so we don't have to see performance timer logs in tests, etc.)libdirectory. I updatedperformance-timerto show this being possible. This means we should be able to remove the_thisWillBeDeletedDoNotUseproperty that was added just to get things to the test and we can import those directly in the tests that need them. However, this isn't something we can do right away as the imported file is a different context from the same file being imported in axe (since we bundle the axe files together). This means file scope/state is different so we can't import the newsetLoggerfunction directly in tests and use it as it doesn't set the axe.js bundledloggervariable scope.Closes #5115