test(integration): more web-test-runner test fixes @W-18763051 #5414
Conversation
everything passed first try, which is kinda sus
# Conflicts: # packages/@lwc/integration-not-karma/helpers/wtr-utils.mjs
code is now in helpers/matchers/index.mjs
reduces test failures from 291 to 220
who needs convention?
instead of just inner logic
fixes ~50 test failures
I'm not sure why the async matters, but it does.
turns out they just had browser log warnings, not failures
| }); | ||
| } | ||
|
|
||
| beforeAll(function () { |
There was a problem hiding this comment.
toEqualWireSettings is never actually used anywhere.
| expect(args[i][0]).toBeInstanceOf(Error); | ||
| expect(args[i][0].message).toMatch(regexes[i]); |
There was a problem hiding this comment.
Karma's toMatch coerces the received value (in this case, an error) to a string, but the setup we have for web-test-runner expects to receive only a string.
| describe('wiring', () => { | ||
| describe('component lifecycle and wire adapter', () => { | ||
| it('should call a connect when component is connected', () => { | ||
| it('should call a connect when component is connected', async () => { |
There was a problem hiding this comment.
The tests in this file use a shared global state. Most of the tests, but not all, have asynchronous behavior. In Karma, the state was properly cleaned for both sync and async tests. With web-test-runner, the state got polluted with the synchronous tests. Converting the sync tests to async functions (even with no inner await/promises) solved the problem, although I don't fully understand why.
Note that I added async to all the functions, not just the sync ones, because that's best practice for anything that returns a promise.
| if (ctx.path.endsWith('.spec.js') && !ctx.query.original) { | ||
| return await wrapHydrationTest(ctx.path.slice(1)); // remove leading / | ||
| // `/__wds-outside-roout__/${depth}/` === '../'.repeat(depth) | ||
| return '/__wds-outside-root__/1/wire-service/dist/index.js'; |
There was a problem hiding this comment.
Turns out what I originally had never actually worked. 😅
There was a problem hiding this comment.
I don't understand this path, is that where modules compiled by wtr ends up?
There was a problem hiding this comment.
It's a magic path that WTR1 uses to serve files that exist outside the web root (the package directory). In our case, it's primarily used by WTR for resolving node_modules in the monorepo root. I think it's mostly intended as an internal detail to make node imports "just work", so this solution is a bit of a hack.
1 technically @web/dev-server, hence wds
| return '@lwc/wire-service'; | ||
| } | ||
| }, | ||
| async serve(ctx) { |
There was a problem hiding this comment.
serve is moved to a separate plugin file, since it is different for regular tests vs hydration tests.
| // Use native shadow by default in hydration tests; MUST be set before imports | ||
| process.env.DISABLE_SYNTHETIC ??= 'true'; |
There was a problem hiding this comment.
There's probably a more idiomatic way of doing this, but the hack works for now. 🤷
wjhsf
changed the title
tests manipulate the body, so we use head for scripts to keep the body clean
| if (ctx.path.endsWith('.spec.js') && !ctx.query.original) { | ||
| return await wrapHydrationTest(ctx.path.slice(1)); // remove leading / | ||
| // `/__wds-outside-roout__/${depth}/` === '../'.repeat(depth) | ||
| return '/__wds-outside-root__/1/wire-service/dist/index.js'; |
There was a problem hiding this comment.
I don't understand this path, is that where modules compiled by wtr ends up?
Details
I cleaned up the organization of the config files a bit, plus a mishmash of things to get more of the tests passing.
Before these changes, we had 2769 tests passing in 248 files. After these changes, we have 3194 tests passing in 297 files.
Does this pull request introduce a breaking change?
Does this pull request introduce an observable change?
GUS work item