test(wtr): clean up hydration test logic @W-19098266#5487
Conversation
instead of weird fake module
There's no setup/teardown needed, it's a single test, and WTR provides per-file encapsulation
working toward just importing and executing things, but not quite there yet
two env vars for the same goal is unnecessary
I think the last one was a concurrency related timeout, which was previously addressed.
helper files shouldn't have side effects; all setup should be in the setup file
no longer necessary
all tests are run in isolation
we want to keep the generated wrappers as thin as possible and use regular js files as much as possible
It was originally implemented to suppress distracting warnings in Karma tests, but WTR's log output is different / managed differently.
| return output[0].code; | ||
| } | ||
|
|
||
| function throwOnUnexpectedConsoleCalls(runnable, expectedConsoleCalls = {}) { |
There was a problem hiding this comment.
This was added in #4649 to suppress logs for cosmetic reasons. WTR surfaces logs differently, so we don't need to keep it.
| ${testConfig} | ||
| ${moduleCode} | ||
| `(async () => { | ||
| const {default: config} = await import('./${configPath}'); |
There was a problem hiding this comment.
Converting from IIFE to ESM import because the config file doesn't need to be bundled, it just works.
| ${componentDefCSR}; | ||
| return await runTest(ssrRendered, Main, config); | ||
| }); | ||
| runTest( |
There was a problem hiding this comment.
Moved as much logic as I could from the generated code into runTest.
| if (source === 'test-utils') { | ||
| return '/helpers/utils.js'; | ||
| } else if (source === 'wire-service') { | ||
| if (source === 'wire-service') { |
There was a problem hiding this comment.
We no longer import from test-utils, so we don't need to resolve it. (I missed this in the other PRs.)
| @@ -1,3 +1,4 @@ | |||
| import { importMapsPlugin } from '@web/dev-server-import-maps'; | |||
There was a problem hiding this comment.
Moved from the base config to this config, because only this config needs this plugin.
| return testResult; | ||
| } | ||
| // Must be sync to properly register tests; async behavior can happen in before/after blocks | ||
| export function runTest(configPath, componentPath, ssrRendered, focused) { |
There was a problem hiding this comment.
Changed this to define the tests (and use idiomatic setup/teardown) rather than be a helper within a test.
| "@lwc/synthetic-shadow": "8.21.6", | ||
| "@types/chai": "^5.2.2", | ||
| "@types/jasmine": "^5.1.9", | ||
| "@vitest/spy": "^3.2.4", |
There was a problem hiding this comment.
Explicitly declaring an already-used transitive dependency.
|
/nucleus ignore --reason meh |
2 participants
Details
The hydration tests are complicated because they require running component code on the server and in the browser. For Karma, this meant that there was a lot of bundling/wrapping/indirection happening. It makes for a very confusing code base, so as part of the migration to Web Test Runner I have been trying to clean things up where I can. My goal with this PR is to remove unnecessary indirection and try to get the generated code as small as possible.
Does this pull request introduce a breaking change?
Does this pull request introduce an observable change?
GUS work item