From efea8ead803e988f2dc9c24a41161c7d5d7f3ed2 Mon Sep 17 00:00:00 2001 From: Andy Weiss Date: Fri, 9 Aug 2024 17:21:29 -0400 Subject: [PATCH 1/3] Allow for Instanbul coverage with Playwright This makes the PlaywrightLauncherPage function similarly to the ChromeLauncher where we attempt to load the coverage from the browser and return that if availble during the stopSession method. Currently the Playwright launcher breaks early if the native instrumentation is not enabled. We changed to using the Playwright launcher and were suprised that our coverage stopped working even though we are using the babel istanbul plugin which worked when we were using the ChromeLauncher. --- .changeset/tidy-knives-agree.md | 5 +++++ .../test-runner-playwright/src/PlaywrightLauncherPage.ts | 8 +++++--- 2 files changed, 10 insertions(+), 3 deletions(-) create mode 100644 .changeset/tidy-knives-agree.md diff --git a/.changeset/tidy-knives-agree.md b/.changeset/tidy-knives-agree.md new file mode 100644 index 000000000..5e91490a6 --- /dev/null +++ b/.changeset/tidy-knives-agree.md @@ -0,0 +1,5 @@ +--- +'@web/test-runner-playwright': patch +--- + +Support browser generated coverage without native instrumentation diff --git a/packages/test-runner-playwright/src/PlaywrightLauncherPage.ts b/packages/test-runner-playwright/src/PlaywrightLauncherPage.ts index fa4de283a..98dd3e84a 100644 --- a/packages/test-runner-playwright/src/PlaywrightLauncherPage.ts +++ b/packages/test-runner-playwright/src/PlaywrightLauncherPage.ts @@ -44,9 +44,7 @@ export class PlaywrightLauncherPage { } async stopSession(): Promise { - const testCoverage = this.nativeInstrumentationEnabledOnPage - ? await this.collectTestCoverage(this.config, this.testFiles) - : undefined; + const testCoverage = await this.collectTestCoverage(this.config, this.testFiles); // navigate to an empty page to kill any running code on the page, stopping timers and // breaking a potential endless reload loop @@ -82,6 +80,10 @@ export class PlaywrightLauncherPage { ); } + if (!this.nativeInstrumentationEnabledOnPage) { + return undefined; + } + // get native coverage from playwright let coverage: V8Coverage[]; if (this.product === 'chromium') { From 1d608fc57011e089c8f4b384db7a0a42155484a7 Mon Sep 17 00:00:00 2001 From: Andy Weiss Date: Wed, 14 Aug 2024 21:22:39 -0400 Subject: [PATCH 2/3] Add a test --- .../basic/browser-tests/coverage.test.js | 8 +++++ .../test-runner/tests/basic/runBasicTest.ts | 31 +++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 integration/test-runner/tests/basic/browser-tests/coverage.test.js diff --git a/integration/test-runner/tests/basic/browser-tests/coverage.test.js b/integration/test-runner/tests/basic/browser-tests/coverage.test.js new file mode 100644 index 000000000..36c5b6b54 --- /dev/null +++ b/integration/test-runner/tests/basic/browser-tests/coverage.test.js @@ -0,0 +1,8 @@ +import { expect } from '../../../../../node_modules/@esm-bundle/chai/esm/chai.js'; + +describe('basic test', () => { + it('works', () => { + window.__coverage__ = 'something'; + expect(true).to.equal(true); + }); +}); diff --git a/integration/test-runner/tests/basic/runBasicTest.ts b/integration/test-runner/tests/basic/runBasicTest.ts index ce9d9c4b1..8dfb00502 100644 --- a/integration/test-runner/tests/basic/runBasicTest.ts +++ b/integration/test-runner/tests/basic/runBasicTest.ts @@ -11,6 +11,37 @@ export function runBasicTest( const browserCount = config.browsers.length; let allSessions: TestSession[]; + describe('coverage', () => { + before(async () => { + const result = await runTests({ + ...config, + coverageConfig: { + nativeInstrumentation: false, + }, + files: [...(config.files ?? []), resolve(__dirname, 'browser-tests', 'coverage.test.js')], + plugins: [...(config.plugins ?? []), legacyPlugin()], + }); + allSessions = result.sessions; + + expect(allSessions.every(s => s.passed)).to.equal(true, 'All sessions should have passed'); + }); + + it('passes coverage test', () => { + const sessions = allSessions.filter(s => s.testFile.endsWith('coverage.test.js')); + expect(sessions.length === browserCount).to.equal( + true, + 'Each browser should run coverage.test.js', + ); + for (const session of sessions) { + expect(session.testCoverage).to.equal('something'); + expect(session.testResults!.tests.length).to.equal(0); + expect(session.testResults!.suites.length).to.equal(1); + expect(session.testResults!.suites[0].tests.length).to.equal(1); + expect(session.testResults!.suites[0].tests.map(t => t.name)).to.eql(['works']); + } + }); + }); + before(async () => { const result = await runTests({ ...config, From 8484bd3f7ccc3b4b326c3f327484a7ad5e57a230 Mon Sep 17 00:00:00 2001 From: Andy Weiss Date: Thu, 15 Aug 2024 09:57:41 -0400 Subject: [PATCH 3/3] Fix tests --- .../basic/browser-tests/coverage.test.js | 8 - .../test-runner/tests/basic/coverage.test.js | 18 ++ .../test-runner/tests/basic/runBasicTest.ts | 158 +++++++++--------- 3 files changed, 98 insertions(+), 86 deletions(-) delete mode 100644 integration/test-runner/tests/basic/browser-tests/coverage.test.js create mode 100644 integration/test-runner/tests/basic/coverage.test.js diff --git a/integration/test-runner/tests/basic/browser-tests/coverage.test.js b/integration/test-runner/tests/basic/browser-tests/coverage.test.js deleted file mode 100644 index 36c5b6b54..000000000 --- a/integration/test-runner/tests/basic/browser-tests/coverage.test.js +++ /dev/null @@ -1,8 +0,0 @@ -import { expect } from '../../../../../node_modules/@esm-bundle/chai/esm/chai.js'; - -describe('basic test', () => { - it('works', () => { - window.__coverage__ = 'something'; - expect(true).to.equal(true); - }); -}); diff --git a/integration/test-runner/tests/basic/coverage.test.js b/integration/test-runner/tests/basic/coverage.test.js new file mode 100644 index 000000000..a7db86629 --- /dev/null +++ b/integration/test-runner/tests/basic/coverage.test.js @@ -0,0 +1,18 @@ +import { expect } from '../../../../../node_modules/@esm-bundle/chai/esm/chai.js'; + +describe('basic test', () => { + it('works', () => { + window.__coverage__ = { + basic: { + path: 'basic.js', + statementMap: {}, + fnMap: {}, + branchMap: {}, + s: {}, + f: {}, + b: {}, + }, + }; + expect(true).to.equal(true); + }); +}); diff --git a/integration/test-runner/tests/basic/runBasicTest.ts b/integration/test-runner/tests/basic/runBasicTest.ts index 8dfb00502..d7083d446 100644 --- a/integration/test-runner/tests/basic/runBasicTest.ts +++ b/integration/test-runner/tests/basic/runBasicTest.ts @@ -11,29 +11,29 @@ export function runBasicTest( const browserCount = config.browsers.length; let allSessions: TestSession[]; - describe('coverage', () => { - before(async () => { + describe('with coverage', () => { + it('passes coverage test', async () => { const result = await runTests({ ...config, + coverage: true, coverageConfig: { nativeInstrumentation: false, }, - files: [...(config.files ?? []), resolve(__dirname, 'browser-tests', 'coverage.test.js')], + files: [...(config.files ?? []), resolve(__dirname, 'coverage.test.js')], plugins: [...(config.plugins ?? []), legacyPlugin()], }); - allSessions = result.sessions; + const allSessions = result.sessions; expect(allSessions.every(s => s.passed)).to.equal(true, 'All sessions should have passed'); - }); - it('passes coverage test', () => { const sessions = allSessions.filter(s => s.testFile.endsWith('coverage.test.js')); expect(sessions.length === browserCount).to.equal( true, 'Each browser should run coverage.test.js', ); for (const session of sessions) { - expect(session.testCoverage).to.equal('something'); + expect(session.testCoverage).not.to.be.undefined; + expect(session.testCoverage!['basic']['path']).to.equal('basic.js'); expect(session.testResults!.tests.length).to.equal(0); expect(session.testResults!.suites.length).to.equal(1); expect(session.testResults!.suites[0].tests.length).to.equal(1); @@ -42,82 +42,84 @@ export function runBasicTest( }); }); - before(async () => { - const result = await runTests({ - ...config, - files: [...(config.files ?? []), resolve(__dirname, 'browser-tests', '*.test.js')], - plugins: [...(config.plugins ?? []), legacyPlugin()], - }); - allSessions = result.sessions; + describe('without coverage', () => { + before(async () => { + const result = await runTests({ + ...config, + files: [...(config.files ?? []), resolve(__dirname, 'browser-tests', '*.test.js')], + plugins: [...(config.plugins ?? []), legacyPlugin()], + }); + allSessions = result.sessions; - expect(allSessions.every(s => s.passed)).to.equal(true, 'All sessions should have passed'); - }); + expect(allSessions.every(s => s.passed)).to.equal(true, 'All sessions should have passed'); + }); - it('passes basic test', () => { - const sessions = allSessions.filter(s => s.testFile.endsWith('basic.test.js')); - expect(sessions.length === browserCount).to.equal( - true, - 'Each browser should run basic.test.js', - ); - for (const session of sessions) { - expect(session.testResults!.tests.length).to.equal(0); - expect(session.testResults!.suites.length).to.equal(1); - expect(session.testResults!.suites[0].tests.length).to.equal(1); - expect(session.testResults!.suites[0].tests.map(t => t.name)).to.eql(['works']); - } - }); + it('passes basic test', () => { + const sessions = allSessions.filter(s => s.testFile.endsWith('basic.test.js')); + expect(sessions.length === browserCount).to.equal( + true, + 'Each browser should run basic.test.js', + ); + for (const session of sessions) { + expect(session.testResults!.tests.length).to.equal(0); + expect(session.testResults!.suites.length).to.equal(1); + expect(session.testResults!.suites[0].tests.length).to.equal(1); + expect(session.testResults!.suites[0].tests.map(t => t.name)).to.eql(['works']); + } + }); - it('passes js-syntax test', () => { - const sessions = allSessions.filter(s => s.testFile.endsWith('js-syntax.test.js')); - expect(sessions.length === browserCount).to.equal( - true, - 'Each browser should run js-syntax.test.js', - ); - for (const session of sessions) { - expect(session.testResults!.tests.map(t => t.name)).to.eql([ - 'supports object spread', - 'supports async functions', - 'supports exponentiation', - 'supports classes', - 'supports template literals', - 'supports optional chaining', - 'supports nullish coalescing', - ]); - } - }); + it('passes js-syntax test', () => { + const sessions = allSessions.filter(s => s.testFile.endsWith('js-syntax.test.js')); + expect(sessions.length === browserCount).to.equal( + true, + 'Each browser should run js-syntax.test.js', + ); + for (const session of sessions) { + expect(session.testResults!.tests.map(t => t.name)).to.eql([ + 'supports object spread', + 'supports async functions', + 'supports exponentiation', + 'supports classes', + 'supports template literals', + 'supports optional chaining', + 'supports nullish coalescing', + ]); + } + }); - it('passes module-features test', () => { - const sessions = allSessions.filter(s => s.testFile.endsWith('module-features.test.js')); - expect(sessions.length === browserCount).to.equal( - true, - 'Each browser should run module-features.test.js', - ); - for (const session of sessions) { - expect(session.testResults!.tests.map(t => t.name)).to.eql([ - 'supports static imports', - 'supports dynamic imports', - 'supports import meta', - ]); - } - }); + it('passes module-features test', () => { + const sessions = allSessions.filter(s => s.testFile.endsWith('module-features.test.js')); + expect(sessions.length === browserCount).to.equal( + true, + 'Each browser should run module-features.test.js', + ); + for (const session of sessions) { + expect(session.testResults!.tests.map(t => t.name)).to.eql([ + 'supports static imports', + 'supports dynamic imports', + 'supports import meta', + ]); + } + }); - it('passes timers test', () => { - const sessions = allSessions.filter(s => s.testFile.endsWith('timers.test.js')); - expect(sessions.length === browserCount).to.equal( - true, - 'Each browser should run timers.test.js', - ); - for (const session of sessions) { - expect(session.testResults!.tests.length).to.equal(0); - expect(session.testResults!.suites.length).to.equal(1); - expect(session.testResults!.suites[0].tests.map(t => t.name)).to.eql([ - 'can call setTimeout', - 'can cancel setTimeout', - 'can call and cancel setInterval', - 'can call requestAnimationFrame', - 'can cancel requestAnimationFrame', - ]); - } + it('passes timers test', () => { + const sessions = allSessions.filter(s => s.testFile.endsWith('timers.test.js')); + expect(sessions.length === browserCount).to.equal( + true, + 'Each browser should run timers.test.js', + ); + for (const session of sessions) { + expect(session.testResults!.tests.length).to.equal(0); + expect(session.testResults!.suites.length).to.equal(1); + expect(session.testResults!.suites[0].tests.map(t => t.name)).to.eql([ + 'can call setTimeout', + 'can cancel setTimeout', + 'can call and cancel setInterval', + 'can call requestAnimationFrame', + 'can cancel requestAnimationFrame', + ]); + } + }); }); }); }