microsoft/playwright
1
1
Fork 0
mirror of https://github.com/microsoft/playwright.git synced 2025-01-05 19:04:43 +03:00
Code Issues Projects Releases Wiki Activity

feat(test runner): do not mock tty in the worker process (#30107)

Browse Source 
This was historically done to make `console.log()` have colors. However,
this makes any other code that checks `process.stdout.isTTY` incorrectly
assume real TTY support.

Node18 and Node20 now respect `FORCE_COLOR=1` in console, so our default
behavior of forcing colors in the worker process just works out of the
box. See https://github.com/nodejs/node/pull/48034.
This commit is contained in:
Dmitry Gozman 2024-03-25 15:31:58 -07:00 committed by GitHub
parent 4b3c596874
commit 911d8effb9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 11 additions and 107 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
packages/playwright/src/common/ipc.ts
View File

@ -46,15 +46,7 @@ export type SerializedConfig = {
compilationCache?: SerializedCompilationCache;
};
export type TtyParams = {
rows: number | undefined;
columns: number | undefined;
colorDepth: number;
};
export type ProcessInitParams = {
stdoutParams: TtyParams;
stderrParams: TtyParams;
processName: string;
};

42
packages/playwright/src/common/process.ts
View File

@ -14,8 +14,7 @@
* limitations under the License.
*/
import type { WriteStream } from 'tty';
import type { EnvProducedPayload, ProcessInitParams, TtyParams } from './ipc';
import type { EnvProducedPayload, ProcessInitParams } from './ipc';
import { startProfiling, stopProfiling } from 'playwright-core/lib/utils';
import type { TestInfoError } from '../../types/test';
import { serializeError } from '../util';
@ -67,8 +66,6 @@ const startingEnv = { ...process.env };
process.on('message', async (message: any) => {
if (message.method === '__init__') {
const { processParams, runnerParams, runnerScript } = message.params as { processParams: ProcessInitParams, runnerParams: any, runnerScript: string };
setTtyParams(process.stdout, processParams.stdoutParams);
setTtyParams(process.stderr, processParams.stderrParams);
void startProfiling();
const { create } = require(runnerScript);
processRunner = create(runnerParams) as ProcessRunner;
@ -117,40 +114,3 @@ function sendMessageToParent(message: { method: string, params?: any }) {
// Can throw when closing.
}
}
function setTtyParams(stream: WriteStream, params: TtyParams) {
stream.isTTY = true;
if (params.rows)
stream.rows = params.rows;
if (params.columns)
stream.columns = params.columns;
stream.getColorDepth = () => params.colorDepth;
stream.hasColors = ((count = 16) => {
// count is optional and the first argument may actually be env.
if (typeof count !== 'number')
count = 16;
return count <= 2 ** params.colorDepth;
})as any;
// Stubs for the rest of the methods to avoid exceptions in user code.
stream.clearLine = (dir: any, callback?: () => void) => {
callback?.();
return true;
};
stream.clearScreenDown = (callback?: () => void) => {
callback?.();
return true;
};
(stream as any).cursorTo = (x: number, y?: number | (() => void), callback?: () => void) => {
if (callback)
callback();
else if (y instanceof Function)
y();
return true;
};
stream.moveCursor = (dx: number, dy: number, callback?: () => void) => {
callback?.();
return true;
};
stream.getWindowSize = () => [stream.columns, stream.rows];
}

10
packages/playwright/src/runner/processHost.ts
View File

@ -112,16 +112,6 @@ export class ProcessHost extends EventEmitter {
return error;
const processParams: ProcessInitParams = {
stdoutParams: {
rows: process.stdout.rows,
columns: process.stdout.columns,
colorDepth: process.stdout.getColorDepth?.() || 8
},
stderrParams: {
rows: process.stderr.rows,
columns: process.stderr.columns,
colorDepth: process.stderr.getColorDepth?.() || 8
},
processName: this._processName
};

58
tests/playwright-test/stdio.spec.ts
View File

@ -78,7 +78,14 @@ test('should ignore stdio when quiet', async ({ runInlineTest }) => {
expect(result.output).not.toContain('%%');
});
test('should support console colors', async ({ runInlineTest }) => {
test('should support console colors but not tty', {
annotation: [
{ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/15366' },
{ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/29839' },
],
}, async ({ runInlineTest, nodeVersion }) => {
test.skip(nodeVersion.major < 18, 'Node16 does not respect FORCE_COLOR in onsole');
const result = await runInlineTest({
'a.spec.js': `
import { test, expect } from '@playwright/test';
@ -90,31 +97,13 @@ test('should support console colors', async ({ runInlineTest }) => {
});
`
});
expect(result.output).toContain(`process.stdout.isTTY = true`);
expect(result.output).toContain(`process.stderr.isTTY = true`);
expect(result.output).toContain(`process.stdout.isTTY = undefined`);
expect(result.output).toContain(`process.stderr.isTTY = undefined`);
// The output should have colors.
expect(result.rawOutput).toContain(`{ b: \x1b[33mtrue\x1b[39m, n: \x1b[33m123\x1b[39m, s: \x1b[32m'abc'\x1b[39m }`);
expect(result.rawOutput).toContain(`{ b: \x1b[33mfalse\x1b[39m, n: \x1b[33m123\x1b[39m, s: \x1b[32m'abc'\x1b[39m }`);
});
test('should override hasColors and getColorDepth', async ({ runInlineTest }) => {
const result = await runInlineTest({
'a.spec.js': `
import { test, expect } from '@playwright/test';
test('console log', () => {
console.log('process.stdout.hasColors(1) = ' + process.stdout.hasColors(1));
console.log('process.stderr.hasColors(1) = ' + process.stderr.hasColors(1));
console.log('process.stdout.getColorDepth() > 0 = ' + (process.stdout.getColorDepth() > 0));
console.log('process.stderr.getColorDepth() > 0 = ' + (process.stderr.getColorDepth() > 0));
});
`
});
expect(result.output).toContain(`process.stdout.hasColors(1) = true`);
expect(result.output).toContain(`process.stderr.hasColors(1) = true`);
expect(result.output).toContain(`process.stdout.getColorDepth() > 0 = true`);
expect(result.output).toContain(`process.stderr.getColorDepth() > 0 = true`);
});
test('should not throw type error when using assert', async ({ runInlineTest }) => {
const result = await runInlineTest({
'a.spec.js': `
@ -128,30 +117,3 @@ test('should not throw type error when using assert', async ({ runInlineTest })
expect(result.output).not.toContain(`TypeError: process.stderr.hasColors is not a function`);
expect(result.output).toContain(`AssertionError`);
});
test('should provide stubs for tty.WriteStream methods on process.stdout/stderr', async ({ runInlineTest }) => {
test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/29839' });
const result = await runInlineTest({
'a.spec.ts': `
import { test, expect } from '@playwright/test';
import type * as tty from 'tty';
async function checkMethods(stream: tty.WriteStream) {
expect(stream.isTTY).toBe(true);
await new Promise<void>(r => stream.clearLine(-1, r));;
await new Promise<void>(r => stream.clearScreenDown(r));;
await new Promise<void>(r => stream.cursorTo(0, 0, r));;
await new Promise<void>(r => stream.cursorTo(0, r));;
await new Promise<void>(r => stream.moveCursor(1, 1, r));;
expect(stream.getWindowSize()).toEqual([stream.columns, stream.rows]);
// getColorDepth() and hasColors() are covered in other tests.
}
test('process.stdout implementd tty.WriteStream methods', () => {
checkMethods(process.stdout);
});
test('process.stderr implementd tty.WriteStream methods', () => {
checkMethods(process.stderr);
});
`
});
expect(result.exitCode).toBe(0);
});