From 2e883d6e5338abca3f262cbd9b2e3e6770876c0e Mon Sep 17 00:00:00 2001 From: aghArdeshir Date: Tue, 27 Aug 2024 23:25:25 +0200 Subject: [PATCH 1/3] ISSUE-2351 Run all browser tests with and without polyfill remove unnecessary polyfill loading scripts that need polyfill import themselves + unnecessary mocking removal separate tests that require polyfill from tests that do not remove unneeded test web test runner create groups for tests to run with and without polyfills refactor: rename --- .../core/test/SlotMixin.polyfill.test.js | 40 +++++++ .../ui/components/core/test/SlotMixin.test.js | 102 ------------------ web-test-runner.config.mjs | 60 ++++++++--- 3 files changed, 86 insertions(+), 116 deletions(-) create mode 100644 packages/ui/components/core/test/SlotMixin.polyfill.test.js diff --git a/packages/ui/components/core/test/SlotMixin.polyfill.test.js b/packages/ui/components/core/test/SlotMixin.polyfill.test.js new file mode 100644 index 0000000000..5b9ebdd4f3 --- /dev/null +++ b/packages/ui/components/core/test/SlotMixin.polyfill.test.js @@ -0,0 +1,40 @@ +import sinon from 'sinon'; +import { defineCE, expect, fixture, unsafeStatic, html } from '@open-wc/testing'; +import { ScopedElementsMixin } from '@open-wc/scoped-elements/lit-element.js'; +import { SlotMixin } from '@lion/ui/core.js'; +import { LitElement } from 'lit'; + +it('supports scoped elements when polyfill loaded', async () => { + // @ts-ignore the scoped-custom-element-registry polyfill makes sure `ShadowRoot.prototype.createElement` is defined + const createElementSpy = sinon.spy(ShadowRoot.prototype, 'createElement'); + + class ScopedEl extends LitElement {} + + const tagName = defineCE( + class extends ScopedElementsMixin(SlotMixin(LitElement)) { + static get scopedElements() { + return { + // @ts-expect-error + ...super.scopedElements, + 'scoped-elm': ScopedEl, + }; + } + + get slots() { + return { + ...super.slots, + template: () => html``, + }; + } + + render() { + return html``; + } + }, + ); + + const tag = unsafeStatic(tagName); + await fixture(html`<${tag}>`); + + expect(createElementSpy.getCalls()).to.have.length(1); +}); diff --git a/packages/ui/components/core/test/SlotMixin.test.js b/packages/ui/components/core/test/SlotMixin.test.js index 413dbf77c9..e9b6dd57cc 100644 --- a/packages/ui/components/core/test/SlotMixin.test.js +++ b/packages/ui/components/core/test/SlotMixin.test.js @@ -1,6 +1,4 @@ -import sinon from 'sinon'; import { defineCE, expect, fixture, fixtureSync, unsafeStatic, html } from '@open-wc/testing'; -import { ScopedElementsMixin } from '@open-wc/scoped-elements/lit-element.js'; import { SlotMixin } from '@lion/ui/core.js'; import { LitElement } from 'lit'; @@ -8,24 +6,6 @@ import { LitElement } from 'lit'; * @typedef {import('../types/SlotMixinTypes.js').SlotHost} SlotHost */ -// @ts-ignore -const createElementNative = ShadowRoot.prototype.createElement; -function mockScopedRegistry() { - const outputObj = { createElementCallCount: 0 }; - // @ts-expect-error wait for browser support - ShadowRoot.prototype.createElement = (tagName, options) => { - outputObj.createElementCallCount += 1; - // Return an element that lit can use as render target - return createElementNative(tagName, options); - }; - return outputObj; -} - -function unMockScopedRegistry() { - // @ts-expect-error wait for browser support - ShadowRoot.prototype.createElement = createElementNative; -} - describe('SlotMixin', () => { it('inserts provided element into light dom and sets slot', async () => { const tag = defineCE( @@ -566,86 +546,4 @@ describe('SlotMixin', () => { expect(el._templateNode.textContent).to.equal('1'); }); }); - - describe('Scoped Registries', () => { - it('supports scoped elements when polyfill loaded', async () => { - const outputObj = mockScopedRegistry(); - - class ScopedEl extends LitElement {} - - const tagName = defineCE( - // @ts-ignore - class extends ScopedElementsMixin(SlotMixin(LitElement)) { - static get scopedElements() { - return { - // @ts-expect-error - ...super.scopedElements, - 'scoped-elm': ScopedEl, - }; - } - - get slots() { - return { - ...super.slots, - template: () => html``, - }; - } - - render() { - return html``; - } - }, - ); - - const tag = unsafeStatic(tagName); - await fixture(html`<${tag}>`); - - expect(outputObj.createElementCallCount).to.equal(1); - - unMockScopedRegistry(); - }); - - it('does not scope elements when polyfill not loaded', async () => { - // @ts-expect-error - ShadowRoot.prototype.createElement = null; - class ScopedEl extends LitElement {} - - const tagName = defineCE( - class extends ScopedElementsMixin(SlotMixin(LitElement)) { - static get scopedElements() { - return { - // @ts-expect-error - ...super.scopedElements, - 'scoped-el': ScopedEl, - }; - } - - get slots() { - return { - ...super.slots, - template: () => html``, - }; - } - - render() { - return html``; - } - }, - ); - - const renderTarget = document.createElement('div'); - const el = document.createElement(tagName); - - // We don't use fixture, so we limit the amount of calls to document.createElement - const docSpy = sinon.spy(document, 'createElement'); - document.body.appendChild(renderTarget); - renderTarget.appendChild(el); - - expect(docSpy.callCount).to.equal(2); - - document.body.removeChild(renderTarget); - docSpy.restore(); - unMockScopedRegistry(); - }); - }); }); diff --git a/web-test-runner.config.mjs b/web-test-runner.config.mjs index 5f269c6da5..f89be72ae8 100644 --- a/web-test-runner.config.mjs +++ b/web-test-runner.config.mjs @@ -3,7 +3,7 @@ import { playwrightLauncher } from '@web/test-runner-playwright'; const devMode = process.argv.includes('--dev-mode'); -const packages = fs +const testGroups = fs .readdirSync('packages') .filter( dir => fs.statSync(`packages/${dir}`).isDirectory() && fs.existsSync(`packages/${dir}/test`), @@ -18,21 +18,54 @@ const packages = fs fs.existsSync(`packages/ui/components/${dir}/test`), ) .map(dir => ({ name: dir, path: `packages/ui/components/${dir}/test` })), - ); + ) + .map(pkg => ({ + name: pkg.name, + files: `${pkg.path}/**/*.test.js`, + })); -/** - * @type {import('@web/test-runner').TestRunnerConfig['testRunnerHtml']} - * - */ -const testRunnerHtml = testRunnerImport => - ` +const testsThatShouldRunWithScopedCustomElementRegistryPolyfill = testGroups.map(testGroup => { + const files = [ + testGroup.files, + `!${testGroup.files.replace('*.test.js', '*.no-polyfill.test.js')}`, + ]; + return { + name: testGroup.name, + files, + /** + * @type {import('@web/test-runner').TestRunnerConfig['testRunnerHtml']} + * + */ + testRunnerHtml: testRunnerImport => ` -`; +`, + }; +}); + +const testsThatShouldNotRunWithPolyfill = testGroups.map(testGroup => { + const files = [testGroup.files, `!${testGroup.files.replace('*.test.js', '*.polyfill.test.js')}`]; + + return { + name: testGroup.name, + files, + /** + * @type {import('@web/test-runner').TestRunnerConfig['testRunnerHtml']} + * + */ + testRunnerHtml: testRunnerImport => ` + + + + + +`, + }; +}); export default { nodeResolve: { exportConditions: [devMode && 'development'] }, @@ -51,14 +84,13 @@ export default { timeout: '5000', }, }, - testRunnerHtml, browsers: [ playwrightLauncher({ product: 'firefox', concurrency: 1 }), playwrightLauncher({ product: 'chromium' }), playwrightLauncher({ product: 'webkit' }), ], - groups: packages.map(pkg => ({ - name: pkg.name, - files: `${pkg.path}/**/*.test.js`, - })), + groups: [].concat([ + ...testsThatShouldRunWithScopedCustomElementRegistryPolyfill, + ...testsThatShouldNotRunWithPolyfill, + ]), }; From a3f1bf32bc0aaaf5b4cf5bc8389c7f3de953a207 Mon Sep 17 00:00:00 2001 From: aghArdeshir Date: Mon, 14 Oct 2024 07:55:51 +0200 Subject: [PATCH 2/3] refactor: rename --- web-test-runner.config.mjs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/web-test-runner.config.mjs b/web-test-runner.config.mjs index f89be72ae8..705cd8fa7d 100644 --- a/web-test-runner.config.mjs +++ b/web-test-runner.config.mjs @@ -24,7 +24,7 @@ const testGroups = fs files: `${pkg.path}/**/*.test.js`, })); -const testsThatShouldRunWithScopedCustomElementRegistryPolyfill = testGroups.map(testGroup => { +const testsThatMustRunWithScopedCustomElementRegistryPolyfill = testGroups.map(testGroup => { const files = [ testGroup.files, `!${testGroup.files.replace('*.test.js', '*.no-polyfill.test.js')}`, @@ -47,7 +47,7 @@ const testsThatShouldRunWithScopedCustomElementRegistryPolyfill = testGroups.map }; }); -const testsThatShouldNotRunWithPolyfill = testGroups.map(testGroup => { +const testsThatMustRunWithoutPolyfill = testGroups.map(testGroup => { const files = [testGroup.files, `!${testGroup.files.replace('*.test.js', '*.polyfill.test.js')}`]; return { @@ -90,7 +90,7 @@ export default { playwrightLauncher({ product: 'webkit' }), ], groups: [].concat([ - ...testsThatShouldRunWithScopedCustomElementRegistryPolyfill, - ...testsThatShouldNotRunWithPolyfill, + ...testsThatMustRunWithScopedCustomElementRegistryPolyfill, + ...testsThatMustRunWithoutPolyfill, ]), }; From b0b1e12f5fbf0cb264c2f28ea4446ee5563cf89b Mon Sep 17 00:00:00 2001 From: aghArdeshir Date: Mon, 14 Oct 2024 07:57:42 +0200 Subject: [PATCH 3/3] refactor: Make the origin of an array a test runner config so we don't get TS errors --- web-test-runner.config.mjs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/web-test-runner.config.mjs b/web-test-runner.config.mjs index 705cd8fa7d..679f4736b9 100644 --- a/web-test-runner.config.mjs +++ b/web-test-runner.config.mjs @@ -89,8 +89,7 @@ export default { playwrightLauncher({ product: 'chromium' }), playwrightLauncher({ product: 'webkit' }), ], - groups: [].concat([ + groups: testsThatMustRunWithoutPolyfill.concat([ ...testsThatMustRunWithScopedCustomElementRegistryPolyfill, - ...testsThatMustRunWithoutPolyfill, ]), };