From 762958791447a93a7d0ec52904ea5bd9d2323015 Mon Sep 17 00:00:00 2001 From: Joel Einbinder Date: Tue, 29 Jun 2021 15:28:41 -0700 Subject: [PATCH] fix(test-runner): work with .mjs files (#7373) --- .../esm-playwright-test.mjs | 19 +++++++++++-- packages/playwright-test/index.mjs | 3 +- src/test/loader.ts | 17 +++++++++-- src/test/runner.ts | 4 +-- src/test/transform.ts | 6 +++- src/test/workerRunner.ts | 4 +-- tests/playwright-test/entry/index.mjs | 20 +++++++++++++ tests/playwright-test/loader.spec.ts | 28 +++++++++++++++++++ .../playwright-test-fixtures.ts | 11 ++++++-- 9 files changed, 98 insertions(+), 14 deletions(-) create mode 100644 tests/playwright-test/entry/index.mjs diff --git a/packages/installation-tests/esm-playwright-test.mjs b/packages/installation-tests/esm-playwright-test.mjs index c824e203b4..2799e11129 100644 --- a/packages/installation-tests/esm-playwright-test.mjs +++ b/packages/installation-tests/esm-playwright-test.mjs @@ -14,11 +14,26 @@ * limitations under the License. */ -import { chromium, firefox, webkit, selectors, devices, errors, test } from '@playwright/test'; +import { chromium, firefox, webkit, selectors, devices, errors, test, expect } from '@playwright/test'; import * as playwright from '@playwright/test'; import defaultExport from '@playwright/test'; import errorsFile from '@playwright/test/lib/utils/errors.js'; import testESM from './esm.mjs'; -if (defaultExport !== test) + +if (defaultExport !== test) { + console.error('default export is not test.'); process.exit(1); +} + +if (typeof test !== 'function') { + console.error('test is not a function'); + process.exit(1); +} + +if (typeof expect !== 'function') { + console.error('expect is not a function'); + process.exit(1); +} +expect(1).toBe(1); + testESM({ chromium, firefox, webkit, selectors, devices, errors, playwright, errorsFile }, [chromium, firefox, webkit]); diff --git a/packages/playwright-test/index.mjs b/packages/playwright-test/index.mjs index 6999af4234..e0a7cf3731 100644 --- a/packages/playwright-test/index.mjs +++ b/packages/playwright-test/index.mjs @@ -14,7 +14,7 @@ * limitations under the License. */ -import * as playwright from './index.js'; +import playwright from './index.js'; export const chromium = playwright.chromium; export const firefox = playwright.firefox; @@ -25,4 +25,5 @@ export const errors = playwright.errors; export const _electron = playwright._electron; export const _android = playwright._android; export const test = playwright.test; +export const expect = playwright.expect; export default playwright.test; diff --git a/src/test/loader.ts b/src/test/loader.ts index e4c27ced1d..a92529258e 100644 --- a/src/test/loader.ts +++ b/src/test/loader.ts @@ -21,6 +21,7 @@ import { setCurrentlyLoadingFileSuite } from './globals'; import { Suite } from './test'; import { SerializedLoaderData } from './ipc'; import * as path from 'path'; +import * as url from 'url'; import { ProjectImpl } from './project'; import { Reporter } from './reporter'; @@ -108,7 +109,7 @@ export class Loader { this._fullConfig.projects = this._projects.map(p => p.config); } - loadTestFile(file: string) { + async loadTestFile(file: string) { if (this._fileSuites.has(file)) return this._fileSuites.get(file)!; const revertBabelRequire = installTransform(); @@ -117,9 +118,19 @@ export class Loader { suite._requireFile = file; suite.file = file; setCurrentlyLoadingFileSuite(suite); - require(file); + if (file.endsWith('.mjs')) { + // eval to prevent typescript from transpiling us here. + await eval(`import(${JSON.stringify(url.pathToFileURL(file))})`); + } else { + require(file); + } this._fileSuites.set(file, suite); return suite; + } catch (error) { + if (error instanceof SyntaxError && error.message.includes('Cannot use import statement outside a module')) + throw errorWithFile(file, 'JavaScript files must end with .mjs to use import.'); + + throw error; } finally { revertBabelRequire(); setCurrentlyLoadingFileSuite(undefined); @@ -191,7 +202,7 @@ export class Loader { name: takeFirst(this._configOverrides.name, projectConfig.name, this._config.name, ''), testDir, testIgnore: takeFirst(this._configOverrides.testIgnore, projectConfig.testIgnore, this._config.testIgnore, []), - testMatch: takeFirst(this._configOverrides.testMatch, projectConfig.testMatch, this._config.testMatch, '**/?(*.)@(spec|test).[jt]s'), + testMatch: takeFirst(this._configOverrides.testMatch, projectConfig.testMatch, this._config.testMatch, '**/?(*.)@(spec|test).@(ts|js|mjs)'), timeout: takeFirst(this._configOverrides.timeout, projectConfig.timeout, this._config.timeout, 10000), use: mergeObjects(mergeObjects(this._config.use, projectConfig.use), this._configOverrides.use), }; diff --git a/src/test/runner.ts b/src/test/runner.ts index 934ae4f166..2ba52b87cf 100644 --- a/src/test/runner.ts +++ b/src/test/runner.ts @@ -160,7 +160,7 @@ export class Runner { const allFiles = await collectFiles(project.config.testDir); const testMatch = createMatcher(project.config.testMatch); const testIgnore = createMatcher(project.config.testIgnore); - const testFileExtension = (file: string) => ['.js', '.ts'].includes(path.extname(file)); + const testFileExtension = (file: string) => ['.js', '.ts', '.mjs'].includes(path.extname(file)); const testFiles = allFiles.filter(file => !testIgnore(file) && testMatch(file) && testFileFilter(file) && testFileExtension(file)); files.set(project, testFiles); testFiles.forEach(file => allTestFiles.add(file)); @@ -171,7 +171,7 @@ export class Runner { globalSetupResult = await this._loader.loadGlobalHook(config.globalSetup, 'globalSetup')(this._loader.fullConfig()); try { for (const file of allTestFiles) - this._loader.loadTestFile(file); + await this._loader.loadTestFile(file); const rootSuite = new Suite(''); for (const fileSuite of this._loader.fileSuites().values()) diff --git a/src/test/transform.ts b/src/test/transform.ts index 84ce9fc116..e55081305f 100644 --- a/src/test/transform.ts +++ b/src/test/transform.ts @@ -21,6 +21,7 @@ import * as fs from 'fs'; import * as pirates from 'pirates'; import * as babel from '@babel/core'; import * as sourceMapSupport from 'source-map-support'; +import * as url from 'url'; import type { Location } from './types'; const version = 4; @@ -107,8 +108,11 @@ export function wrapFunctionWithLocation(func: (location: Lo const oldPrepareStackTrace = Error.prepareStackTrace; Error.prepareStackTrace = (error, stackFrames) => { const frame: NodeJS.CallSite = sourceMapSupport.wrapCallSite(stackFrames[1]); + const fileName = frame.getFileName(); + // Node error stacks for modules use file:// urls instead of paths. + const file = (fileName && fileName.startsWith('file://')) ? url.fileURLToPath(fileName) : fileName; return { - file: frame.getFileName(), + file, line: frame.getLineNumber(), column: frame.getColumnNumber(), }; diff --git a/src/test/workerRunner.ts b/src/test/workerRunner.ts index a0e025fbaf..d5ec484beb 100644 --- a/src/test/workerRunner.ts +++ b/src/test/workerRunner.ts @@ -114,7 +114,7 @@ export class WorkerRunner extends EventEmitter { this._remaining = new Map(runPayload.entries.map(e => [ e.testId, e ])); this._loadIfNeeded(); - const fileSuite = this._loader.loadTestFile(runPayload.file); + const fileSuite = await this._loader.loadTestFile(runPayload.file); let anySpec: Spec | undefined; fileSuite.findSpec(spec => { const test = this._project.generateTests(spec, this._params.repeatEachIndex)[0]; @@ -184,7 +184,7 @@ export class WorkerRunner extends EventEmitter { const testId = test._id; const baseOutputDir = (() => { - const relativeTestFilePath = path.relative(this._project.config.testDir, spec._requireFile.replace(/\.(spec|test)\.(js|ts)/, '')); + const relativeTestFilePath = path.relative(this._project.config.testDir, spec._requireFile.replace(/\.(spec|test)\.(js|ts|mjs)/, '')); const sanitizedRelativePath = relativeTestFilePath.replace(process.platform === 'win32' ? new RegExp('\\\\', 'g') : new RegExp('/', 'g'), '-'); let testOutputDir = sanitizedRelativePath + '-' + sanitizeForFilePath(spec.title); if (this._uniqueProjectNamePathSegment) diff --git a/tests/playwright-test/entry/index.mjs b/tests/playwright-test/entry/index.mjs new file mode 100644 index 0000000000..8d90fcd9e4 --- /dev/null +++ b/tests/playwright-test/entry/index.mjs @@ -0,0 +1,20 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import playwright from './index.js'; + +export const test = playwright.test; +export const expect = playwright.expect; +export default playwright.test; \ No newline at end of file diff --git a/tests/playwright-test/loader.spec.ts b/tests/playwright-test/loader.spec.ts index 02d278d29e..0ed6ba8675 100644 --- a/tests/playwright-test/loader.spec.ts +++ b/tests/playwright-test/loader.spec.ts @@ -148,3 +148,31 @@ test('should match tests well', async ({ runInlineTest }) => { expect(result.exitCode).toBe(0); expect(result.passed).toBe(5); }); + +test('should load an mjs file', async ({ runInlineTest }) => { + const { exitCode, passed } = await runInlineTest({ + 'a.spec.mjs': ` + const { test } = pwt; + test('succeeds', () => { + expect(1 + 1).toBe(2); + }); + ` + }); + expect(passed).toBe(1); + expect(exitCode).toBe(0); +}); + +test('should throw a nice error if a js file uses import', async ({ runInlineTest }) => { + const { exitCode, output } = await runInlineTest({ + 'a.spec.js': ` + import fs from 'fs'; + const { test } = folio; + test('succeeds', () => { + expect(1 + 1).toBe(2); + }); + ` + }); + expect(exitCode).toBe(1); + expect(output).toContain('a.spec.js'); + expect(output).toContain('JavaScript files must end with .mjs to use import.'); +}); diff --git a/tests/playwright-test/playwright-test-fixtures.ts b/tests/playwright-test/playwright-test-fixtures.ts index ad9ea9d834..cd2fa583f3 100644 --- a/tests/playwright-test/playwright-test-fixtures.ts +++ b/tests/playwright-test/playwright-test-fixtures.ts @@ -22,6 +22,7 @@ import * as os from 'os'; import type { ReportFormat } from '../../src/test/reporters/json'; import rimraf from 'rimraf'; import { promisify } from 'util'; +import * as url from 'url'; const removeFolderAsync = promisify(rimraf); @@ -55,6 +56,9 @@ async function writeFiles(testInfo: TestInfo, files: Files) { const headerTS = ` import * as pwt from ${internalPath}; `; + const headerMJS = ` + import * as pwt from ${JSON.stringify(url.pathToFileURL(path.join(__dirname, 'entry', 'index.mjs')))}; + `; const hasConfig = Object.keys(files).some(name => name.includes('.config.')); if (!hasConfig) { @@ -69,9 +73,10 @@ async function writeFiles(testInfo: TestInfo, files: Files) { await Promise.all(Object.keys(files).map(async name => { const fullName = path.join(baseDir, name); await fs.promises.mkdir(path.dirname(fullName), { recursive: true }); - const isTypeScriptSourceFile = name.endsWith('ts') && !name.endsWith('d.ts'); - const header = isTypeScriptSourceFile ? headerTS : headerJS; - if (/(spec|test)\.(js|ts)$/.test(name)) { + const isTypeScriptSourceFile = name.endsWith('.ts') && !name.endsWith('.d.ts'); + const isJSModule = name.endsWith('.mjs'); + const header = isTypeScriptSourceFile ? headerTS : (isJSModule ? headerMJS : headerJS); + if (/(spec|test)\.(js|ts|mjs)$/.test(name)) { const fileHeader = header + 'const { expect } = pwt;\n'; await fs.promises.writeFile(fullName, fileHeader + files[name]); } else if (/\.(js|ts)$/.test(name) && !name.endsWith('d.ts')) {