From b41b8026627445c06bafe4ca2608a43c57c11293 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Mon, 18 Mar 2024 17:17:58 -0700 Subject: [PATCH] fix(test runner): avoid dependency tracking colliding between esm and cjs (#29994) 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. References #29747. --- 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 d28f75da84..315b42665c 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'], }); });