Skip to content

Commit 116c042

Browse files
committed
feat(test runner): do not mock tty in the worker process
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.
1 parent 1539cde commit 116c042

File tree

4 files changed

+9
-107
lines changed

4 files changed

+9
-107
lines changed

packages/playwright/src/common/ipc.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,7 @@ export type SerializedConfig = {
4646
compilationCache?: SerializedCompilationCache;
4747
};
4848

49-
export type TtyParams = {
50-
rows: number | undefined;
51-
columns: number | undefined;
52-
colorDepth: number;
53-
};
54-
5549
export type ProcessInitParams = {
56-
stdoutParams: TtyParams;
57-
stderrParams: TtyParams;
5850
processName: string;
5951
};
6052

packages/playwright/src/common/process.ts

Lines changed: 1 addition & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
import type { WriteStream } from 'tty';
18-
import type { EnvProducedPayload, ProcessInitParams, TtyParams } from './ipc';
17+
import type { EnvProducedPayload, ProcessInitParams } from './ipc';
1918
import { startProfiling, stopProfiling } from 'playwright-core/lib/utils';
2019
import type { TestInfoError } from '../../types/test';
2120
import { serializeError } from '../util';
@@ -67,8 +66,6 @@ const startingEnv = { ...process.env };
6766
process.on('message', async (message: any) => {
6867
if (message.method === '__init__') {
6968
const { processParams, runnerParams, runnerScript } = message.params as { processParams: ProcessInitParams, runnerParams: any, runnerScript: string };
70-
setTtyParams(process.stdout, processParams.stdoutParams);
71-
setTtyParams(process.stderr, processParams.stderrParams);
7269
void startProfiling();
7370
const { create } = require(runnerScript);
7471
processRunner = create(runnerParams) as ProcessRunner;
@@ -117,40 +114,3 @@ function sendMessageToParent(message: { method: string, params?: any }) {
117114
// Can throw when closing.
118115
}
119116
}
120-
121-
function setTtyParams(stream: WriteStream, params: TtyParams) {
122-
stream.isTTY = true;
123-
if (params.rows)
124-
stream.rows = params.rows;
125-
if (params.columns)
126-
stream.columns = params.columns;
127-
stream.getColorDepth = () => params.colorDepth;
128-
stream.hasColors = ((count = 16) => {
129-
// count is optional and the first argument may actually be env.
130-
if (typeof count !== 'number')
131-
count = 16;
132-
return count <= 2 ** params.colorDepth;
133-
})as any;
134-
135-
// Stubs for the rest of the methods to avoid exceptions in user code.
136-
stream.clearLine = (dir: any, callback?: () => void) => {
137-
callback?.();
138-
return true;
139-
};
140-
stream.clearScreenDown = (callback?: () => void) => {
141-
callback?.();
142-
return true;
143-
};
144-
(stream as any).cursorTo = (x: number, y?: number | (() => void), callback?: () => void) => {
145-
if (callback)
146-
callback();
147-
else if (y instanceof Function)
148-
y();
149-
return true;
150-
};
151-
stream.moveCursor = (dx: number, dy: number, callback?: () => void) => {
152-
callback?.();
153-
return true;
154-
};
155-
stream.getWindowSize = () => [stream.columns, stream.rows];
156-
}

packages/playwright/src/runner/processHost.ts

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -112,16 +112,6 @@ export class ProcessHost extends EventEmitter {
112112
return error;
113113

114114
const processParams: ProcessInitParams = {
115-
stdoutParams: {
116-
rows: process.stdout.rows,
117-
columns: process.stdout.columns,
118-
colorDepth: process.stdout.getColorDepth?.() || 8
119-
},
120-
stderrParams: {
121-
rows: process.stderr.rows,
122-
columns: process.stderr.columns,
123-
colorDepth: process.stderr.getColorDepth?.() || 8
124-
},
125115
processName: this._processName
126116
};
127117

tests/playwright-test/stdio.spec.ts

Lines changed: 8 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,12 @@ test('should ignore stdio when quiet', async ({ runInlineTest }) => {
7878
expect(result.output).not.toContain('%%');
7979
});
8080

81-
test('should support console colors', async ({ runInlineTest }) => {
81+
test('should support console colors but not tty', {
82+
annotation: [
83+
{ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/15366' },
84+
{ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/29839' },
85+
],
86+
}, async ({ runInlineTest }) => {
8287
const result = await runInlineTest({
8388
'a.spec.js': `
8489
import { test, expect } from '@playwright/test';
@@ -90,31 +95,13 @@ test('should support console colors', async ({ runInlineTest }) => {
9095
});
9196
`
9297
});
93-
expect(result.output).toContain(`process.stdout.isTTY = true`);
94-
expect(result.output).toContain(`process.stderr.isTTY = true`);
98+
expect(result.output).toContain(`process.stdout.isTTY = undefined`);
99+
expect(result.output).toContain(`process.stderr.isTTY = undefined`);
95100
// The output should have colors.
96101
expect(result.rawOutput).toContain(`{ b: \x1b[33mtrue\x1b[39m, n: \x1b[33m123\x1b[39m, s: \x1b[32m'abc'\x1b[39m }`);
97102
expect(result.rawOutput).toContain(`{ b: \x1b[33mfalse\x1b[39m, n: \x1b[33m123\x1b[39m, s: \x1b[32m'abc'\x1b[39m }`);
98103
});
99104

100-
test('should override hasColors and getColorDepth', async ({ runInlineTest }) => {
101-
const result = await runInlineTest({
102-
'a.spec.js': `
103-
import { test, expect } from '@playwright/test';
104-
test('console log', () => {
105-
console.log('process.stdout.hasColors(1) = ' + process.stdout.hasColors(1));
106-
console.log('process.stderr.hasColors(1) = ' + process.stderr.hasColors(1));
107-
console.log('process.stdout.getColorDepth() > 0 = ' + (process.stdout.getColorDepth() > 0));
108-
console.log('process.stderr.getColorDepth() > 0 = ' + (process.stderr.getColorDepth() > 0));
109-
});
110-
`
111-
});
112-
expect(result.output).toContain(`process.stdout.hasColors(1) = true`);
113-
expect(result.output).toContain(`process.stderr.hasColors(1) = true`);
114-
expect(result.output).toContain(`process.stdout.getColorDepth() > 0 = true`);
115-
expect(result.output).toContain(`process.stderr.getColorDepth() > 0 = true`);
116-
});
117-
118105
test('should not throw type error when using assert', async ({ runInlineTest }) => {
119106
const result = await runInlineTest({
120107
'a.spec.js': `
@@ -128,30 +115,3 @@ test('should not throw type error when using assert', async ({ runInlineTest })
128115
expect(result.output).not.toContain(`TypeError: process.stderr.hasColors is not a function`);
129116
expect(result.output).toContain(`AssertionError`);
130117
});
131-
132-
test('should provide stubs for tty.WriteStream methods on process.stdout/stderr', async ({ runInlineTest }) => {
133-
test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/29839' });
134-
const result = await runInlineTest({
135-
'a.spec.ts': `
136-
import { test, expect } from '@playwright/test';
137-
import type * as tty from 'tty';
138-
async function checkMethods(stream: tty.WriteStream) {
139-
expect(stream.isTTY).toBe(true);
140-
await new Promise<void>(r => stream.clearLine(-1, r));;
141-
await new Promise<void>(r => stream.clearScreenDown(r));;
142-
await new Promise<void>(r => stream.cursorTo(0, 0, r));;
143-
await new Promise<void>(r => stream.cursorTo(0, r));;
144-
await new Promise<void>(r => stream.moveCursor(1, 1, r));;
145-
expect(stream.getWindowSize()).toEqual([stream.columns, stream.rows]);
146-
// getColorDepth() and hasColors() are covered in other tests.
147-
}
148-
test('process.stdout implementd tty.WriteStream methods', () => {
149-
checkMethods(process.stdout);
150-
});
151-
test('process.stderr implementd tty.WriteStream methods', () => {
152-
checkMethods(process.stderr);
153-
});
154-
`
155-
});
156-
expect(result.exitCode).toBe(0);
157-
});

0 commit comments

Comments
 (0)