-
Notifications
You must be signed in to change notification settings - Fork 439
test(wtr): clean up hydration test logic @W-19098266 #5487
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 39 commits
8c7ed79
e2f88d5
81275d0
39533d3
33e58c8
d0b8003
976c002
8d39505
c66afad
f1c50b0
7a73bf0
9b7b21b
9deaca2
1cf1275
7fc137c
ae937cc
9250dd5
d4280e8
bbffbf3
1ef88ed
02f8ed1
7a5260a
1a08a62
f14c829
289deeb
b97f783
3cdb65c
7009a26
19c1090
5e75329
497bfa8
8ff59c5
885ce57
aa47c49
6185811
ec0186f
3463450
7c0eb0a
d91c8aa
371032d
c47a44d
639f754
83cbf04
7479525
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import { importMapsPlugin } from '@web/dev-server-import-maps'; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved from the base config to this config, because only this config needs this plugin. |
||
| import baseConfig from './base.js'; | ||
| import testPlugin from './plugins/serve-integration.js'; | ||
|
|
||
|
|
@@ -17,5 +18,9 @@ export default { | |
| // Implement objectContaining / arrayWithExactContents | ||
| '!test/profiler/mutation-logging/index.spec.js', | ||
| ], | ||
| plugins: [...baseConfig.plugins, testPlugin], | ||
| plugins: [ | ||
| ...baseConfig.plugins, | ||
| importMapsPlugin({ inject: { importMap: { imports: { lwc: './mocks/lwc.js' } } } }), | ||
| testPlugin, | ||
| ], | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,9 +18,8 @@ lwcSsr.setHooks({ | |
| }); | ||
|
|
||
| const ROOT_DIR = path.join(import.meta.dirname, '../..'); | ||
|
|
||
| let guid = 0; | ||
| const COMPONENT_UNDER_TEST = 'main'; | ||
| const COMPONENT_NAME = 'x-main'; | ||
| const COMPONENT_ENTRYPOINT = 'x/main/main.js'; | ||
|
|
||
| // Like `fs.existsSync` but async | ||
| async function exists(path) { | ||
|
|
@@ -32,13 +31,14 @@ async function exists(path) { | |
| } | ||
| } | ||
|
|
||
| async function getCompiledModule(dir, compileForSSR) { | ||
| async function compileModule(input, targetSSR, format) { | ||
| const modulesDir = path.join(ROOT_DIR, input.slice(0, -COMPONENT_ENTRYPOINT.length)); | ||
| const bundle = await rollup({ | ||
| input: path.join(dir, 'x', COMPONENT_UNDER_TEST, `${COMPONENT_UNDER_TEST}.js`), | ||
| input, | ||
| plugins: [ | ||
| lwcRollupPlugin({ | ||
| targetSSR: !!compileForSSR, | ||
| modules: [{ dir: path.join(ROOT_DIR, dir) }], | ||
| targetSSR, | ||
| modules: [{ dir: modulesDir }], | ||
| experimentalDynamicComponent: { | ||
| loader: fileURLToPath(new URL('../../helpers/loader.js', import.meta.url)), | ||
| strict: true, | ||
|
|
@@ -61,102 +61,49 @@ async function getCompiledModule(dir, compileForSSR) { | |
| }); | ||
|
|
||
| const { output } = await bundle.generate({ | ||
| format: 'iife', | ||
| name: 'Main', | ||
| format, | ||
| name: 'Component', | ||
| globals: { | ||
| lwc: 'LWC', | ||
| '@lwc/ssr-runtime': 'LWC', | ||
| 'test-utils': 'TestUtils', | ||
| }, | ||
| }); | ||
|
|
||
| return output[0].code; | ||
| } | ||
|
|
||
| function throwOnUnexpectedConsoleCalls(runnable, expectedConsoleCalls = {}) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was added in #4649 to suppress logs for cosmetic reasons. WTR surfaces logs differently, so we don't need to keep it. |
||
| // The console is shared between the VM and the main realm. Here we ensure that known warnings | ||
| // are ignored and any others cause an explicit error. | ||
| const methods = ['error', 'warn', 'log', 'info']; | ||
| const originals = {}; | ||
| for (const method of methods) { | ||
| // eslint-disable-next-line no-console | ||
| originals[method] = console[method]; | ||
| // eslint-disable-next-line no-console | ||
| console[method] = function (error) { | ||
| if ( | ||
| method === 'warn' && | ||
| // This eslint warning is a false positive due to RegExp.prototype.test | ||
| // eslint-disable-next-line vitest/no-conditional-tests | ||
| /Cannot set property "(inner|outer)HTML"/.test(error?.message) | ||
| ) { | ||
| return; | ||
| } else if ( | ||
| expectedConsoleCalls[method]?.some((matcher) => error.message.includes(matcher)) | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| throw new Error(`Unexpected console.${method} call: ${error}`); | ||
| }; | ||
| } | ||
| try { | ||
| return runnable(); | ||
| } finally { | ||
| Object.assign(console, originals); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * This is the function that takes SSR bundle code and test config, constructs a script that will | ||
| * run in a separate JS runtime environment with its own global scope. The `context` object | ||
| * (defined at the top of this file) is passed in as the global scope for that script. The script | ||
| * runs, utilizing the `LWC` object that we've attached to the global scope, it sets a | ||
| * new value (the rendered markup) to `globalThis.moduleOutput`, which corresponds to | ||
| * `context.moduleOutput in this file's scope. | ||
| * | ||
| * So, script runs, generates markup, & we get that markup out and return it for use | ||
| * in client-side tests. | ||
| * This function takes a path to a component definition and a config file and returns the | ||
| * SSR-generated markup for the component. It does so by compiling the component and then | ||
| * running a script in a separate JS runtime environment to render it. | ||
| */ | ||
| async function getSsrCode(moduleCode, testConfig, filePath, expectedSSRConsoleCalls) { | ||
| async function getSsrMarkup(componentEntrypoint, configPath) { | ||
| const componentIife = await compileModule(componentEntrypoint, !ENGINE_SERVER, 'iife'); | ||
| // To minimize the amount of code in the generated script, ideally we'd do `import Component` | ||
| // and delegate the bundling to the loader. However, that's complicated to configure and using | ||
| // imports with vm.Script/vm.Module is still experimental, so we use an IIFE for simplicity. | ||
| // Additionally, we could import LWC, but the framework requires configuration before each test | ||
| // (setHooks/setFeatureFlagForTest), so instead we configure it once in the top-level context | ||
| // and inject it as a global variable. | ||
| const script = new vm.Script( | ||
| `(() => { | ||
| ${testConfig} | ||
| ${moduleCode} | ||
| `(async () => { | ||
| const {default: config} = await import('./${configPath}'); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Converting from IIFE to ESM import because the config file doesn't need to be bundled, it just works. |
||
| ${componentIife /* var Component = ... */} | ||
| return LWC.renderComponent( | ||
| 'x-${COMPONENT_UNDER_TEST}-${guid++}', | ||
| Main, | ||
| '${COMPONENT_NAME}', | ||
| Component, | ||
| config.props || {}, | ||
| false, | ||
| 'sync' | ||
| ); | ||
| })()`, | ||
| { filename: `[SSR] ${filePath}` } | ||
| { | ||
| filename: `[SSR] ${configPath}`, | ||
| importModuleDynamically: vm.constants.USE_MAIN_CONTEXT_DEFAULT_LOADER, | ||
| } | ||
| ); | ||
|
|
||
| return throwOnUnexpectedConsoleCalls( | ||
| () => script.runInContext(vm.createContext({ LWC: lwcSsr })), | ||
| expectedSSRConsoleCalls | ||
| ); | ||
| } | ||
|
|
||
| async function getTestConfig(input) { | ||
| const bundle = await rollup({ | ||
| input, | ||
| external: ['lwc', 'test-utils', '@test/loader'], | ||
| }); | ||
|
|
||
| const { output } = await bundle.generate({ | ||
| format: 'iife', | ||
| globals: { | ||
| lwc: 'LWC', | ||
| 'test-utils': 'TestUtils', | ||
| }, | ||
| name: 'config', | ||
| }); | ||
|
|
||
| const { code } = output[0]; | ||
|
|
||
| return code; | ||
| return await script.runInContext(vm.createContext({ LWC: lwcSsr })); | ||
| } | ||
|
|
||
| async function existsUp(dir, file) { | ||
|
|
@@ -172,45 +119,32 @@ async function existsUp(dir, file) { | |
| * Hydration test `index.spec.js` files are actually config files, not spec files. | ||
| * This function wraps those configs in the test code to be executed. | ||
| */ | ||
| async function wrapHydrationTest(filePath) { | ||
| const { | ||
| default: { expectedSSRConsoleCalls, requiredFeatureFlags }, | ||
| } = await import(path.join(ROOT_DIR, filePath)); | ||
| async function wrapHydrationTest(configPath) { | ||
| const { default: config } = await import(path.join(ROOT_DIR, configPath)); | ||
|
|
||
| try { | ||
| requiredFeatureFlags?.forEach((featureFlag) => { | ||
| config.requiredFeatureFlags?.forEach((featureFlag) => { | ||
| lwcSsr.setFeatureFlagForTest(featureFlag, true); | ||
| }); | ||
|
|
||
| const suiteDir = path.dirname(filePath); | ||
| // You can add an `.only` file alongside an `index.spec.js` file to make it `fdescribe()` | ||
| const suiteDir = path.dirname(configPath); | ||
| const componentEntrypoint = path.join(suiteDir, COMPONENT_ENTRYPOINT); | ||
| // You can add an `.only` file alongside an `index.spec.js` file to make the test focused | ||
| const onlyFileExists = await existsUp(suiteDir, '.only'); | ||
| const ssrOutput = await getSsrMarkup(componentEntrypoint, configPath); | ||
|
|
||
| const componentDefCSR = await getCompiledModule(suiteDir, false); | ||
| const componentDefSSR = ENGINE_SERVER | ||
| ? componentDefCSR | ||
| : await getCompiledModule(suiteDir, true); | ||
| const ssrOutput = await getSsrCode( | ||
| componentDefSSR, | ||
| await getTestConfig(filePath), | ||
| filePath, | ||
| expectedSSRConsoleCalls | ||
| ); | ||
|
|
||
| // FIXME: can we turn these IIFEs into ESM imports? | ||
| return ` | ||
| import * as LWC from 'lwc'; | ||
| import { runTest } from '/helpers/test-hydrate.js'; | ||
| import config from '/${filePath}?original=1'; | ||
| ${onlyFileExists ? 'it.only' : 'it'}('${filePath}', async () => { | ||
| const ssrRendered = ${JSON.stringify(ssrOutput) /* escape quotes */}; | ||
| // Component code, IIFE set as Main | ||
| ${componentDefCSR}; | ||
| return await runTest(ssrRendered, Main, config); | ||
| }); | ||
| runTest( | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved as much logic as I could from the generated code into |
||
| '/${configPath}?original=1', | ||
| '/${componentEntrypoint}', | ||
| ${JSON.stringify(ssrOutput) /* escape quotes */}, | ||
| ${onlyFileExists} | ||
| ); | ||
| `; | ||
| } finally { | ||
| requiredFeatureFlags?.forEach((featureFlag) => { | ||
| config.requiredFeatureFlags?.forEach((featureFlag) => { | ||
| lwcSsr.setFeatureFlagForTest(featureFlag, false); | ||
| }); | ||
| } | ||
|
|
@@ -226,6 +160,8 @@ export default { | |
| // to return the file unmodified. | ||
| if (ctx.path.endsWith('.spec.js') && !ctx.query.original) { | ||
| return await wrapHydrationTest(ctx.path.slice(1)); // remove leading / | ||
| } else if (ctx.path.endsWith('/' + COMPONENT_ENTRYPOINT)) { | ||
| return await compileModule(ctx.path.slice(1) /* remove leading / */, false, 'esm'); | ||
| } | ||
| }, | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,39 +22,56 @@ function setFeatureFlags(requiredFeatureFlags, value) { | |
| }); | ||
| } | ||
|
|
||
| async function runTest(ssrRendered, Component, testConfig) { | ||
| const container = appendTestTarget(ssrRendered); | ||
| const selector = container.firstChild.tagName.toLowerCase(); | ||
| let target = container.querySelector(selector); | ||
|
|
||
| let testResult; | ||
| const consoleSpy = spyConsole(); | ||
| setFeatureFlags(testConfig.requiredFeatureFlags, true); | ||
|
|
||
| if (testConfig.test) { | ||
| const snapshot = testConfig.snapshot ? testConfig.snapshot(target) : {}; | ||
|
|
||
| const props = testConfig.props || {}; | ||
| const clientProps = testConfig.clientProps || props; | ||
|
|
||
| LWC.hydrateComponent(target, Component, clientProps); | ||
|
|
||
| // let's select again the target, it should be the same elements as in the snapshot | ||
| target = container.querySelector(selector); | ||
| testResult = await testConfig.test(target, snapshot, consoleSpy.calls); | ||
| } else if (testConfig.advancedTest) { | ||
| testResult = await testConfig.advancedTest(target, { | ||
| Component, | ||
| hydrateComponent: LWC.hydrateComponent.bind(LWC), | ||
| consoleSpy, | ||
| container, | ||
| selector, | ||
| }); | ||
| } | ||
|
|
||
| consoleSpy.reset(); | ||
|
|
||
| return testResult; | ||
| } | ||
| // Must be sync to properly register tests; async behavior can happen in before/after blocks | ||
| export function runTest(configPath, componentPath, ssrRendered, focused) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed this to define the tests (and use idiomatic setup/teardown) rather than be a helper within a test. |
||
| const test = focused ? it.only : it; | ||
| const description = new URL(configPath, location.href).pathname; | ||
| let consoleSpy; | ||
| let testConfig; | ||
| let Component; | ||
|
|
||
| beforeAll(async () => { | ||
| testConfig = await import(configPath); | ||
| Component = await import(componentPath); | ||
| setFeatureFlags(testConfig.requiredFeatureFlags, true); | ||
| }); | ||
|
|
||
| beforeEach(async () => { | ||
| consoleSpy = spyConsole(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| consoleSpy.reset(); | ||
| }); | ||
|
|
||
| export { runTest }; | ||
| afterAll(() => { | ||
| setFeatureFlags(testConfig.requiredFeatureFlags, false); | ||
| }); | ||
|
|
||
| test(description, async () => { | ||
| const container = appendTestTarget(ssrRendered); | ||
| const selector = container.firstChild.tagName.toLowerCase(); | ||
| let target = container.querySelector(selector); | ||
|
|
||
| if (testConfig.test) { | ||
| const snapshot = testConfig.snapshot ? testConfig.snapshot(target) : {}; | ||
|
|
||
| const props = testConfig.props || {}; | ||
| const clientProps = testConfig.clientProps || props; | ||
|
|
||
| LWC.hydrateComponent(target, Component, clientProps); | ||
|
|
||
| // let's select again the target, it should be the same elements as in the snapshot | ||
| target = container.querySelector(selector); | ||
| await testConfig.test(target, snapshot, consoleSpy.calls); | ||
| } else if (testConfig.advancedTest) { | ||
| await testConfig.advancedTest(target, { | ||
| Component, | ||
| hydrateComponent: LWC.hydrateComponent.bind(LWC), | ||
| consoleSpy, | ||
| container, | ||
| selector, | ||
| }); | ||
| } | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ | |
| "@lwc/synthetic-shadow": "8.21.6", | ||
| "@types/chai": "^5.2.2", | ||
| "@types/jasmine": "^5.1.9", | ||
| "@vitest/spy": "^3.2.4", | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Explicitly declaring an already-used transitive dependency. |
||
| "@web/dev-server-import-maps": "^0.2.1", | ||
| "@web/dev-server-rollup": "^0.6.4", | ||
| "@web/test-runner": "^0.20.2", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We no longer import from
test-utils, so we don't need to resolve it. (I missed this in the other PRs.)