From 1dcc04bb6a1780ee6ad8ca5a9fd0aca2756f9634 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Mon, 18 Mar 2024 16:05:09 -0700 Subject: [PATCH] fix(test runner): avoid dependency tracking colliding between esm and cjs When collecting dependencies both from CJS loader and from ESM loader, the latter would overwrite the dependencies set instead of appending. Also make sure cts/cjs/mts/mjs are all supported equally. --- packages/playwright/src/common/ipc.ts | 4 +- .../src/transform/compilationCache.ts | 20 +++-- .../playwright/src/transform/transform.ts | 2 +- packages/playwright/src/worker/testInfo.ts | 2 +- tests/playwright-test/watch.spec.ts | 81 ++++++++++++++++++- 5 files changed, 95 insertions(+), 14 deletions(-) diff --git a/packages/playwright/src/common/ipc.ts b/packages/playwright/src/common/ipc.ts index bb35f6c06d..3c3ad40f02 100644 --- a/packages/playwright/src/common/ipc.ts +++ b/packages/playwright/src/common/ipc.ts @@ -15,7 +15,7 @@ */ import util from 'util'; -import { serializeCompilationCache } from '../transform/compilationCache'; +import { type SerializedCompilationCache, serializeCompilationCache } from '../transform/compilationCache'; import type { ConfigLocation, FullConfigInternal } from './config'; import type { ReporterDescription, TestInfoError, TestStatus } from '../../types/test'; @@ -43,7 +43,7 @@ export type ConfigCLIOverrides = { export type SerializedConfig = { location: ConfigLocation; configCLIOverrides: ConfigCLIOverrides; - compilationCache: any; + compilationCache?: SerializedCompilationCache; }; export type TtyParams = { diff --git a/packages/playwright/src/transform/compilationCache.ts b/packages/playwright/src/transform/compilationCache.ts index caa4b08d23..e0154840b0 100644 --- a/packages/playwright/src/transform/compilationCache.ts +++ b/packages/playwright/src/transform/compilationCache.ts @@ -27,7 +27,7 @@ export type MemoryCache = { moduleUrl?: string; }; -type SerializedCompilationCache = { +export type SerializedCompilationCache = { sourceMaps: [string, string][], memoryCache: [string, MemoryCache][], fileDependencies: [string, string[]][], @@ -158,15 +158,19 @@ export function serializeCompilationCache(): SerializedCompilationCache { }; } -export function addToCompilationCache(payload: any) { +export function addToCompilationCache(payload: SerializedCompilationCache) { for (const entry of payload.sourceMaps) sourceMaps.set(entry[0], entry[1]); for (const entry of payload.memoryCache) memoryCache.set(entry[0], entry[1]); - for (const entry of payload.fileDependencies) - fileDependencies.set(entry[0], new Set(entry[1])); - for (const entry of payload.externalDependencies) - externalDependencies.set(entry[0], new Set(entry[1])); + for (const entry of payload.fileDependencies) { + const existing = fileDependencies.get(entry[0]) || []; + fileDependencies.set(entry[0], new Set([...entry[1], ...existing])); + } + for (const entry of payload.externalDependencies) { + const existing = externalDependencies.get(entry[0]) || []; + externalDependencies.set(entry[0], new Set([...entry[1], ...existing])); + } } function calculateCachePath(filePath: string, hash: string): string { @@ -249,9 +253,9 @@ const kPlaywrightCoveragePrefix = path.resolve(__dirname, '../../../../tests/con export function belongsToNodeModules(file: string) { if (file.includes(`${path.sep}node_modules${path.sep}`)) return true; - if (file.startsWith(kPlaywrightInternalPrefix) && file.endsWith('.js')) + if (file.startsWith(kPlaywrightInternalPrefix) && (file.endsWith('.js') || file.endsWith('.mjs'))) return true; - if (file.startsWith(kPlaywrightCoveragePrefix) && file.endsWith('.js')) + if (file.startsWith(kPlaywrightCoveragePrefix) && (file.endsWith('.js') || file.endsWith('.mjs'))) return true; return false; } diff --git a/packages/playwright/src/transform/transform.ts b/packages/playwright/src/transform/transform.ts index a2e376d043..2f6431c4f1 100644 --- a/packages/playwright/src/transform/transform.ts +++ b/packages/playwright/src/transform/transform.ts @@ -240,7 +240,7 @@ function installTransform(): () => void { if (!shouldTransform(filename)) return code; return transformHook(code, filename).code; - }, { exts: ['.ts', '.tsx', '.js', '.jsx', '.mjs'] }); + }, { exts: ['.ts', '.tsx', '.js', '.jsx', '.mjs', '.mts', '.cjs', '.cts'] }); return () => { reverted = true; diff --git a/packages/playwright/src/worker/testInfo.ts b/packages/playwright/src/worker/testInfo.ts index e3ec7a46c1..0c40f9a91e 100644 --- a/packages/playwright/src/worker/testInfo.ts +++ b/packages/playwright/src/worker/testInfo.ts @@ -171,7 +171,7 @@ export class TestInfoImpl implements TestInfo { this._timeoutManager = new TimeoutManager(this.project.timeout); this.outputDir = (() => { - const relativeTestFilePath = path.relative(this.project.testDir, this._requireFile.replace(/\.(spec|test)\.(js|ts|mjs)$/, '')); + const relativeTestFilePath = path.relative(this.project.testDir, this._requireFile.replace(/\.(spec|test)\.(js|ts|jsx|tsx|mjs|mts|cjs|cts)$/, '')); const sanitizedRelativePath = relativeTestFilePath.replace(process.platform === 'win32' ? new RegExp('\\\\', 'g') : new RegExp('/', 'g'), '-'); const fullTitleWithoutSpec = this.titlePath.slice(1).join(' '); diff --git a/tests/playwright-test/watch.spec.ts b/tests/playwright-test/watch.spec.ts index c4b978819f..d697037801 100644 --- a/tests/playwright-test/watch.spec.ts +++ b/tests/playwright-test/watch.spec.ts @@ -91,8 +91,85 @@ test('should print dependencies in ESM mode', async ({ runInlineTest }) => { const output = result.output; const deps = JSON.parse(output.match(/###(.*)###/)![1]); expect(deps).toEqual({ - 'a.test.ts': ['helperA.ts', 'index.mjs'], - 'b.test.ts': ['helperA.ts', 'helperB.ts', 'index.mjs'], + 'a.test.ts': ['helperA.ts'], + 'b.test.ts': ['helperA.ts', 'helperB.ts'], + }); +}); + +test('should print dependencies in mixed CJS/ESM mode 1', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'package.json': `{ "type": "module" }`, + 'playwright.config.ts': ` + import { defineConfig } from '@playwright/test'; + export default defineConfig({ + globalTeardown: './globalTeardown.ts', + }); + `, + 'helperA.cjs': `exports.foo = () => {}`, + 'helperB.cjs': `require('./helperA');`, + 'a.test.ts': ` + import './helperA'; + import { test, expect } from '@playwright/test'; + test('passes', () => {}); + `, + 'b.test.cjs': ` + require('./helperB'); + const { test, expect } = require('@playwright/test'); + test('passes', () => {}); + `, + 'globalTeardown.ts': ` + import { fileDependencies } from 'playwright/lib/internalsForTest'; + export default () => { + console.log('###' + JSON.stringify(fileDependencies()) + '###'); + }; + ` + }, {}); + + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(2); + const output = result.output; + const deps = JSON.parse(output.match(/###(.*)###/)![1]); + expect(deps).toEqual({ + 'a.test.ts': ['helperA.cjs'], + 'b.test.cjs': ['helperA.cjs', 'helperB.cjs'], + }); +}); + +test('should print dependencies in mixed CJS/ESM mode 2', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'playwright.config.mts': ` + import { defineConfig } from '@playwright/test'; + export default defineConfig({ + globalTeardown: './globalTeardown.ts', + }); + `, + 'helperA.cjs': `exports.foo = () => {}`, + 'helperB.cts': `import './helperA';`, + 'a.test.mts': ` + import './helperA'; + import { test, expect } from '@playwright/test'; + test('passes', () => {}); + `, + 'b.test.ts': ` + import './helperB'; + const { test, expect } = require('@playwright/test'); + test('passes', () => {}); + `, + 'globalTeardown.ts': ` + import { fileDependencies } from 'playwright/lib/internalsForTest'; + export default () => { + console.log('###' + JSON.stringify(fileDependencies()) + '###'); + }; + ` + }, {}); + + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(2); + const output = result.output; + const deps = JSON.parse(output.match(/###(.*)###/)![1]); + expect(deps).toEqual({ + 'a.test.mts': ['helperA.cjs'], + 'b.test.ts': ['helperA.cjs', 'helperB.cts'], }); });