Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: allow calling spawnSync on Node.js file inside test #26429

Merged
merged 1 commit into from
Aug 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 8 additions & 6 deletions packages/playwright-test/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import fs from 'fs';
import path from 'path';
import { Runner } from './runner/runner';
import { stopProfiling, startProfiling, gracefullyProcessExitDoNotHang } from 'playwright-core/lib/utils';
import { experimentalLoaderOption, fileIsModule, serializeError } from './util';
import { execArgvWithoutExperimentalLoaderOptions, execArgvWithExperimentalLoaderOptions, fileIsModule, serializeError } from './util';
import { showHTMLReport } from './reporters/html';
import { createMergedReport } from './reporters/merge';
import { ConfigLoader, resolveConfigFile } from './common/configLoader';
Expand Down Expand Up @@ -275,17 +275,19 @@ function restartWithExperimentalTsEsm(configFile: string | null): boolean {
return false;
if (process.env.PW_DISABLE_TS_ESM)
return false;
if (process.env.PW_TS_ESM_ON)
if (process.env.PW_TS_ESM_ON) {
// clear execArgv after restart, so that childProcess.fork in user code does not inherit our loader.
process.execArgv = execArgvWithoutExperimentalLoaderOptions();
return false;
}
if (!fileIsModule(configFile))
return false;
const NODE_OPTIONS = (process.env.NODE_OPTIONS || '') + experimentalLoaderOption();
const innerProcess = require('child_process').fork(require.resolve('./cli'), process.argv.slice(2), {
const innerProcess = (require('child_process') as typeof import('child_process')).fork(require.resolve('./cli'), process.argv.slice(2), {
env: {
...process.env,
NODE_OPTIONS,
PW_TS_ESM_ON: '1',
}
},
execArgv: execArgvWithExperimentalLoaderOptions(),
});

innerProcess.on('close', (code: number | null) => {
Expand Down
5 changes: 4 additions & 1 deletion packages/playwright-test/src/common/process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import type { WriteStream } from 'tty';
import type { EnvProducedPayload, ProcessInitParams, TtyParams } from './ipc';
import { startProfiling, stopProfiling } from 'playwright-core/lib/utils';
import type { TestInfoError } from '../../types/test';
import { serializeError } from '../util';
import { execArgvWithoutExperimentalLoaderOptions, serializeError } from '../util';

export type ProtocolRequest = {
id: number;
Expand Down Expand Up @@ -51,6 +51,9 @@ process.on('disconnect', gracefullyCloseAndExit);
process.on('SIGINT', () => {});
process.on('SIGTERM', () => {});

// Clear execArgv immediately, so that the user-code does not inherit our loader.
process.execArgv = execArgvWithoutExperimentalLoaderOptions();

let processRunner: ProcessRunner;
let processName: string;
const startingEnv = { ...process.env };
Expand Down
3 changes: 1 addition & 2 deletions packages/playwright-test/src/plugins/webServerPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import { raceAgainstDeadline, launchProcess, httpRequest, monotonicTime } from '
import type { FullConfig } from '../../types/testReporter';
import type { TestRunnerPlugin } from '.';
import type { FullConfigInternal } from '../common/config';
import { envWithoutExperimentalLoaderOptions } from '../util';
import type { ReporterV2 } from '../reporters/reporterV2';


Expand Down Expand Up @@ -95,7 +94,7 @@ export class WebServerPlugin implements TestRunnerPlugin {
command: this._options.command,
env: {
...DEFAULT_ENVIRONMENT_VARIABLES,
...envWithoutExperimentalLoaderOptions(),
...process.env,
...this._options.env,
},
cwd: this._options.cwd,
Expand Down
2 changes: 2 additions & 0 deletions packages/playwright-test/src/runner/processHost.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { EventEmitter } from 'events';
import { debug } from 'playwright-core/lib/utilsBundle';
import type { EnvProducedPayload, ProcessInitParams } from '../common/ipc';
import type { ProtocolResponse } from '../common/process';
import { execArgvWithExperimentalLoaderOptions } from '../util';

export type ProcessExitData = {
unexpectedly: boolean;
Expand Down Expand Up @@ -50,6 +51,7 @@ export class ProcessHost extends EventEmitter {
detached: false,
env: { ...process.env, ...this._extraEnv },
stdio: inheritStdio ? ['ignore', 'inherit', 'inherit', 'ipc'] : ['ignore', 'ignore', process.env.PW_RUNNER_DEBUG ? 'inherit' : 'ignore', 'ipc'],
...(process.env.PW_TS_ESM_ON ? { execArgv: execArgvWithExperimentalLoaderOptions() } : {}),
});
this.process.on('exit', (code, signal) => {
this.didExit = true;
Expand Down
20 changes: 12 additions & 8 deletions packages/playwright-test/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -303,16 +303,20 @@ function folderIsModule(folder: string): boolean {
return require(packageJsonPath).type === 'module';
}

export function experimentalLoaderOption() {
return ` --no-warnings --experimental-loader=${url.pathToFileURL(require.resolve('@playwright/test/lib/transform/esmLoader')).toString()}`;
const kExperimentalLoaderOptions = [
'--no-warnings',
`--experimental-loader=${url.pathToFileURL(require.resolve('@playwright/test/lib/transform/esmLoader')).toString()}`,
];

export function execArgvWithExperimentalLoaderOptions() {
return [
...process.execArgv,
...kExperimentalLoaderOptions,
];
}

export function envWithoutExperimentalLoaderOptions(): NodeJS.ProcessEnv {
const substring = experimentalLoaderOption();
const result = { ...process.env };
if (result.NODE_OPTIONS)
result.NODE_OPTIONS = result.NODE_OPTIONS.replace(substring, '').trim() || undefined;
return result;
export function execArgvWithoutExperimentalLoaderOptions() {
return process.execArgv.filter(arg => !kExperimentalLoaderOptions.includes(arg));
}

// This follows the --moduleResolution=bundler strategy from tsc.
Expand Down
4 changes: 2 additions & 2 deletions tests/playwright-test/assets/simple-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ const requestListener = function (req, res) {
res.end('hello');
return;
}
if (req.url === '/env-FOO') {
res.end(process.env.FOO);
if (req.url.startsWith('/env-')) {
res.end(process.env[req.url.substring(5)]);
return;
}
if (req.url === '/port') {
Expand Down
85 changes: 85 additions & 0 deletions tests/playwright-test/esm.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -547,3 +547,88 @@ test('should disallow ESM when config is cjs', async ({ runInlineTest }) => {
expect(result.exitCode).toBe(1);
expect(result.output).toContain('Unknown file extension ".ts"');
});

test('should be able to use use execSync with a Node.js file inside a spec', async ({ runInlineTest }) => {
test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/24516' });
const result = await runInlineTest({
'global-setup.ts': `
import { execSync, spawnSync, fork } from 'child_process';
console.log('%%global-setup import level');
console.log('%%execSync: ' + execSync('node hello.js').toString());
console.log('%%spawnSync: ' + spawnSync('node', ['hello.js']).stdout.toString());
export default async () => {
console.log('%%global-setup export level');
console.log('%%execSync: ' + execSync('node hello.js').toString());
console.log('%%spawnSync: ' + spawnSync('node', ['hello.js']).stdout.toString());
const child = fork('hellofork.js');
child.on('message', (m) => console.log('%%fork: ' + m));
await new Promise((resolve) => child.on('exit', (code) => resolve(code)));
}
`,
'global-teardown.ts': `
import { execSync, spawnSync, fork } from 'child_process';
console.log('%%global-teardown import level');
console.log('%%execSync: ' + execSync('node hello.js').toString());
console.log('%%spawnSync: ' + spawnSync('node', ['hello.js']).stdout.toString());
export default async () => {
console.log('%%global-teardown export level');
console.log('%%execSync: ' + execSync('node hello.js').toString());
console.log('%%spawnSync: ' + spawnSync('node', ['hello.js']).stdout.toString());
const child = fork('hellofork.js');
child.on('message', (m) => console.log('%%fork: ' + m));
await new Promise((resolve) => child.on('exit', (code) => resolve(code)));
}
`,
'package.json': `{ "type": "module" }`,
'playwright.config.ts': `export default {
projects: [{name: 'foo'}],
globalSetup: './global-setup.ts',
globalTeardown: './global-teardown.ts',
};`,
'hello.js': `console.log('hello from hello.js');`,
'hellofork.js': `process.send('hello from hellofork.js');`,
'a.test.ts': `
import { test, expect } from '@playwright/test';
import { execSync, spawnSync, fork } from 'child_process';
console.log('%%inside test file');
console.log('%%execSync: ' + execSync('node hello.js').toString());
console.log('%%spawnSync: ' + spawnSync('node', ['hello.js']).stdout.toString());
test('check project name', async ({}) => {
console.log('%%inside test');
console.log('%%execSync: ' + execSync('node hello.js').toString());
console.log('%%spawnSync: ' + spawnSync('node', ['hello.js']).stdout.toString());
const child = fork('hellofork.js');
child.on('message', (m) => console.log('%%fork: ' + m));
await new Promise((resolve) => child.on('exit', (code) => resolve(code)));
});
`,
});
expect(result.exitCode).toBe(0);
expect(result.passed).toBe(1);
expect(result.outputLines).toEqual([
'global-setup import level',
'execSync: hello from hello.js',
'spawnSync: hello from hello.js',
'global-teardown import level',
'execSync: hello from hello.js',
'spawnSync: hello from hello.js',
'global-setup export level',
'execSync: hello from hello.js',
'spawnSync: hello from hello.js',
'fork: hello from hellofork.js',
'inside test file',
'execSync: hello from hello.js',
'spawnSync: hello from hello.js',
'inside test file',
'execSync: hello from hello.js',
'spawnSync: hello from hello.js',
'inside test',
'execSync: hello from hello.js',
'spawnSync: hello from hello.js',
'fork: hello from hellofork.js',
'global-teardown export level',
'execSync: hello from hello.js',
'spawnSync: hello from hello.js',
'fork: hello from hellofork.js',
]);
});
5 changes: 5 additions & 0 deletions tests/playwright-test/web-server.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,17 @@ test('should create a server', async ({ runInlineTest }, { workerIndex }) => {

test('should create a server with environment variables', async ({ runInlineTest }, { workerIndex }) => {
const port = workerIndex * 2 + 10500;
process.env['FOOEXTERNAL'] = 'EXTERNAL-BAR';
const result = await runInlineTest({
'test.spec.ts': `
import { test, expect } from '@playwright/test';
test('connect to the server', async ({baseURL, page}) => {
expect(baseURL).toBe('http://localhost:${port}');
await page.goto(baseURL + '/env-FOO');
expect(await page.textContent('body')).toBe('BAR');

await page.goto(baseURL + '/env-FOOEXTERNAL');
expect(await page.textContent('body')).toBe('EXTERNAL-BAR');
});
`,
'playwright.config.ts': `
Expand All @@ -115,6 +119,7 @@ test('should create a server with environment variables', async ({ runInlineTest
expect(result.output).toContain('[WebServer] listening');
expect(result.output).toContain('[WebServer] error from server');
expect(result.report.suites[0].specs[0].tests[0].results[0].status).toContain('passed');
delete process.env['FOOEXTERNAL'];
});

test('should default cwd to config directory', async ({ runInlineTest }, testInfo) => {
Expand Down