chore(ui): ensure isTTY is correct (#38044)

Author: Skn0tt

packages/playwright/src/reporters/base.ts

@@ -49,7 +49,6 @@ type TestSummary = {
 export type CommonReporterOptions = {
   configDir: string,
   _mode?: 'list' | 'test' | 'merge',
-  _isTestServer?: boolean,
   _commandHash?: string,
 };
 

packages/playwright/src/reporters/html.ts

@@ -150,10 +150,10 @@ class HtmlReporter implements ReporterV2 {
     if (process.env.CI || !this._buildResult)
       return;
     const { ok, singleTestId } = this._buildResult;
-    const shouldOpen = !this._options._isTestServer && (this._open === 'always' || (!ok && this._open === 'on-failure'));
+    const shouldOpen = !!process.stdin.isTTY && (this._open === 'always' || (!ok && this._open === 'on-failure'));
     if (shouldOpen) {
       await showHTMLReport(this._outputFolder, this._host, this._port, singleTestId);
-    } else if (this._options._mode === 'test' && !this._options._isTestServer) {
+    } else if (this._options._mode === 'test' && !!process.stdin.isTTY) {
       const packageManagerCommand = getPackageManagerExecCommand();
       const relativeReportPath = this._outputFolder === standaloneDefaultFolder() ? '' : ' ' + path.relative(process.cwd(), this._outputFolder);
       const hostArg = this._host ? ` --host ${this._host}` : '';

packages/playwright/src/reporters/merge.ts

@@ -42,7 +42,7 @@ type ReportData = {
 };
 
 export async function createMergedReport(config: FullConfigInternal, dir: string, reporterDescriptions: ReporterDescription[], rootDirOverride: string | undefined) {
-  const reporters = await createReporters(config, 'merge', false, reporterDescriptions);
+  const reporters = await createReporters(config, 'merge', reporterDescriptions);
   const multiplexer = new Multiplexer(reporters);
   const stringPool = new StringInternPool();
 

packages/playwright/src/runner/reporters.ts

@@ -36,7 +36,7 @@ import type { BuiltInReporter, FullConfigInternal } from '../common/config';
 import type { CommonReporterOptions, Screen } from '../reporters/base';
 import type { ReporterV2 } from '../reporters/reporterV2';
 
-export async function createReporters(config: FullConfigInternal, mode: 'list' | 'test' | 'merge', isTestServer: boolean, descriptions?: ReporterDescription[]): Promise<ReporterV2[]> {
+export async function createReporters(config: FullConfigInternal, mode: 'list' | 'test' | 'merge', descriptions?: ReporterDescription[]): Promise<ReporterV2[]> {
   const defaultReporters: { [key in BuiltInReporter]: new(arg: any) => ReporterV2 } = {
     blob: BlobReporter,
     dot: mode === 'list' ? ListModeReporter : DotReporter,
@@ -52,7 +52,7 @@ export async function createReporters(config: FullConfigInternal, mode: 'list' |
   descriptions ??= config.config.reporter;
   if (config.configCLIOverrides.additionalReporters)
     descriptions = [...descriptions, ...config.configCLIOverrides.additionalReporters];
-  const runOptions = reporterOptions(config, mode, isTestServer);
+  const runOptions = reporterOptions(config, mode);
   for (const r of descriptions) {
     const [name, arg] = r;
     const options = { ...runOptions, ...arg };
@@ -103,11 +103,10 @@ export function createErrorCollectingReporter(screen: Screen): ErrorCollectingRe
   };
 }
 
-function reporterOptions(config: FullConfigInternal, mode: 'list' | 'test' | 'merge', isTestServer: boolean): CommonReporterOptions {
+function reporterOptions(config: FullConfigInternal, mode: 'list' | 'test' | 'merge'): CommonReporterOptions {
   return {
     configDir: config.configDir,
     _mode: mode,
-    _isTestServer: isTestServer,
     _commandHash: computeCommandHash(config),
   };
 }

packages/playwright/src/runner/testRunner.ts

@@ -340,7 +340,7 @@ export class TestRunner extends EventEmitter<TestRunnerEventMap> {
       config.preOnlyTestFilters.push(test => testIdSet.has(test.id));
     }
 
-    const configReporters = params.disableConfigReporters ? [] : await createReporters(config, 'test', true);
+    const configReporters = params.disableConfigReporters ? [] : await createReporters(config, 'test');
     const reporter = new InternalReporter([...configReporters, userReporter]);
     const stop = new ManualPromise();
     const tasks = [
@@ -453,7 +453,7 @@ export async function runAllTestsWithConfig(config: FullConfigInternal): Promise
   // Legacy webServer support.
   webServerPluginsForConfig(config).forEach(p => config.plugins.push({ factory: p }));
 
-  const reporters = await createReporters(config, listOnly ? 'list' : 'test', false);
+  const reporters = await createReporters(config, listOnly ? 'list' : 'test');
   const lastRun = new LastRunReporter(config);
   if (config.cliLastFailed)
     await lastRun.filterLastFailed();

packages/playwright/src/runner/testServer.ts

@@ -40,6 +40,8 @@ const originalStdoutWrite = process.stdout.write;
 // eslint-disable-next-line no-restricted-properties
 const originalStderrWrite = process.stderr.write;
 
+const originalStdinIsTTY = process.stdin.isTTY;
+
 class TestServer {
   private _configLocation: ConfigLocation;
   private _configCLIOverrides: ConfigCLIOverrides;
@@ -250,10 +252,17 @@ export class TestServerDispatcher implements TestServerInterface {
       };
       process.stdout.write = stdoutWrite;
       process.stderr.write = stderrWrite;
+
+      // Override isTTY to prevent reporters from thinking they can block and wait for user SIGINT.
+      // We don't have a test for this, so be careful!
+      // https://github.com/microsoft/playwright/issues/37867
+      // @ts-expect-error types are wrong, isTTY can be undefined
+      process.stdin.isTTY = undefined;
     } else {
       debug.log = originalDebugLog;
       process.stdout.write = originalStdoutWrite;
       process.stderr.write = originalStderrWrite;
+      process.stdin.isTTY = originalStdinIsTTY;
     }
     /* eslint-enable no-restricted-properties */
   }

tests/playwright-test/reporter-html.spec.ts

@@ -1494,7 +1494,7 @@ for (const useIntermediateMergeReport of [true, false] as const) {
       expect(output).toContain('html-report');
     });
 
-    test('it should only identify exact matches as clashing folders', async ({ runInlineTest, useIntermediateMergeReport }) => {
+    test('it should only identify exact matches as clashing folders', async ({ runInlineTest, useIntermediateMergeReport }, testInfo) => {
       test.skip(useIntermediateMergeReport);
       const result = await runInlineTest({
         'playwright.config.ts': `
@@ -1509,9 +1509,8 @@ for (const useIntermediateMergeReport of [true, false] as const) {
         `,
       });
       expect(result.exitCode).toBe(0);
-      const output = result.output;
-      expect(output).not.toContain('Configuration Error');
-      expect(output).toContain('test-results-html');
+      expect(result.output).not.toContain('Configuration Error');
+      expect(fs.existsSync(testInfo.outputPath('test-results-html'))).toBeTruthy();
     });
 
     test.describe('report location', () => {