chore(test runner): do not produce some of the fake skipped test results (#27730)

packages/playwright/src/reporters/base.ts

@@ -35,6 +35,7 @@ type ErrorDetails = {
 };
 
 type TestSummary = {
+  didNotRun: number;
   skipped: number;
   expected: number;
   interrupted: TestCase[];
@@ -158,7 +159,7 @@ export class BaseReporter implements ReporterV2 {
     return fileDurations.filter(([, duration]) => duration > threshold).slice(0, count);
   }
 
-  protected generateSummaryMessage({ skipped, expected, interrupted, unexpected, flaky, fatalErrors }: TestSummary) {
+  protected generateSummaryMessage({ didNotRun, skipped, expected, interrupted, unexpected, flaky, fatalErrors }: TestSummary) {
     const tokens: string[] = [];
     if (unexpected.length) {
       tokens.push(colors.red(`  ${unexpected.length} failed`));
@@ -177,6 +178,8 @@ export class BaseReporter implements ReporterV2 {
     }
     if (skipped)
       tokens.push(colors.yellow(`  ${skipped} skipped`));
+    if (didNotRun)
+      tokens.push(colors.yellow(`  ${didNotRun} did not run`));
     if (expected)
       tokens.push(colors.green(`  ${expected} passed`) + colors.dim(` (${milliseconds(this.result.duration)})`));
     if (this.result.status === 'timedout')
@@ -188,6 +191,7 @@ export class BaseReporter implements ReporterV2 {
   }
 
   protected generateSummary(): TestSummary {
+    let didNotRun = 0;
     let skipped = 0;
     let expected = 0;
     const interrupted: TestCase[] = [];
@@ -202,6 +206,8 @@ export class BaseReporter implements ReporterV2 {
             if (test.results.some(result => !!result.error))
               interruptedToPrint.push(test);
             interrupted.push(test);
+          } else if (!test.results.length) {
+            ++didNotRun;
           } else {
             ++skipped;
           }
@@ -215,6 +221,7 @@ export class BaseReporter implements ReporterV2 {
 
     const failuresToPrint = [...unexpected, ...flaky, ...interruptedToPrint];
     return {
+      didNotRun,
       skipped,
       expected,
       interrupted,

packages/playwright/src/reporters/markdown.ts

@@ -63,7 +63,8 @@ class MarkdownReporter extends BaseReporter {
       lines.push(``);
     }
     const skipped = summary.skipped ? `, ${summary.skipped} skipped` : '';
-    lines.push(`**${summary.expected} passed${skipped}**`);
+    const didNotRun = summary.didNotRun ? `, ${summary.didNotRun} did not run` : '';
+    lines.push(`**${summary.expected} passed${skipped}${didNotRun}**`);
     lines.push(`:heavy_check_mark::heavy_check_mark::heavy_check_mark:`);
     lines.push(``);
 

packages/playwright/src/runner/dispatcher.ts

@@ -140,11 +140,6 @@ export class Dispatcher {
     if (this._workerSlots.some(w => w.busy))
       return;
 
-    for (const test of this._allTests) {
-      // Emulate skipped test run if we have stopped early.
-      if (!test.results.length)
-        test._appendTestResult().status = 'skipped';
-    }
     this._finished.resolve();
   }
 
@@ -513,8 +508,13 @@ class JobDispatcher {
     // with skipped tests mixed in-between non-skipped. This makes
     // for a better reporter experience.
     const allTestsSkipped = this._job.tests.every(test => test.expectedStatus === 'skipped');
-    if (allTestsSkipped) {
-      this._massSkipTestsFromRemaining(new Set(this._remainingByTestId.keys()), []);
+    if (allTestsSkipped && !this._failureTracker.hasReachedMaxFailures()) {
+      for (const test of this._job.tests) {
+        const result = test._appendTestResult();
+        this._reporter.onTestBegin(test, result);
+        result.status = 'skipped';
+        this._reportTestEnd(test, result);
+      }
       return true;
     }
     return false;

packages/playwright/src/runner/tasks.ts

@@ -274,14 +274,8 @@ function createRunTestsTask(): Task<TestRun> {
           extraEnvByProjectId.set(project.id, extraEnv);
 
           const hasFailedDeps = project.deps.some(p => !successfulProjects.has(p));
-          if (!hasFailedDeps) {
+          if (!hasFailedDeps)
             phaseTestGroups.push(...testGroups);
-          } else {
-            for (const testGroup of testGroups) {
-              for (const test of testGroup.tests)
-                test._appendTestResult().status = 'skipped';
-            }
-          }
         }
 
         if (phaseTestGroups.length) {

tests/playwright-test/deps.spec.ts

@@ -133,7 +133,7 @@ test('should not run project if dependency failed', async ({ runInlineTest }) =>
   expect(result.exitCode).toBe(1);
   expect(result.passed).toBe(1);
   expect(result.failed).toBe(1);
-  expect(result.skipped).toBe(1);
+  expect(result.didNotRun).toBe(1);
   expect(result.output).toContain('Failed project B');
   expect(result.outputLines).toEqual(['A', 'B']);
 });
@@ -297,7 +297,7 @@ test('should not filter dependency by only 3', async ({ runInlineTest }) => {
   expect(result.outputLines).toEqual(['setup 2 in setup']);
 });
 
-test('should report skipped dependent tests', async ({ runInlineTest }) => {
+test('should not report skipped dependent tests', async ({ runInlineTest }) => {
   const result = await runInlineTest({
     'playwright.config.ts': `
       module.exports = { projects: [
@@ -318,8 +318,8 @@ test('should report skipped dependent tests', async ({ runInlineTest }) => {
   });
   expect(result.exitCode).toBe(1);
   expect(result.passed).toBe(0);
-  expect(result.skipped).toBe(1);
-  expect(result.results.length).toBe(2);
+  expect(result.didNotRun).toBe(1);
+  expect(result.results.length).toBe(1);
 });
 
 test('should report circular dependencies', async ({ runInlineTest }) => {
@@ -499,7 +499,7 @@ test('should run teardown after failure', async ({ runInlineTest }) => {
   expect(result.exitCode).toBe(1);
   expect(result.passed).toBe(1);
   expect(result.failed).toBe(1);
-  expect(result.skipped).toBe(2);
+  expect(result.didNotRun).toBe(2);
   expect(result.outputLines).toEqual(['A', 'D']);
 });
 

tests/playwright-test/hooks.spec.ts

@@ -370,7 +370,7 @@ test('max-failures should still run afterEach/afterAll', async ({ runInlineTest
   expect(result.exitCode).toBe(1);
   expect(result.passed).toBe(0);
   expect(result.failed).toBe(1);
-  expect(result.skipped).toBe(1);
+  expect(result.didNotRun).toBe(1);
   expect(result.outputLines).toEqual([
     'test',
     'afterEach',

tests/playwright-test/max-failures.spec.ts

@@ -38,8 +38,6 @@ test('max-failures should work', async ({ runInlineTest }) => {
   expect(result.exitCode).toBe(1);
   expect(result.failed).toBe(8);
   expect(result.output.split('\n').filter(l => l.includes('expect(')).length).toBe(16);
-  expect(result.report.suites[0].specs.map(spec => spec.tests[0].results.length)).toEqual(new Array(10).fill(1));
-  expect(result.report.suites[1].specs.map(spec => spec.tests[0].results.length)).toEqual(new Array(10).fill(1));
 });
 
 test('-x should work', async ({ runInlineTest }) => {
@@ -111,7 +109,7 @@ test('max-failures should stop workers', async ({ runInlineTest }) => {
   expect(result.passed).toBe(2);
   expect(result.failed).toBe(1);
   expect(result.interrupted).toBe(1);
-  expect(result.skipped).toBe(1);
+  expect(result.didNotRun).toBe(1);
   expect(result.output).toContain('%%interrupted');
   expect(result.output).not.toContain('%%skipped');
 });
@@ -177,7 +175,7 @@ test('max-failures should work across phases', async ({ runInlineTest }) => {
   expect(result.exitCode).toBe(1);
   expect(result.failed).toBe(1);
   expect(result.passed).toBe(2);
-  expect(result.skipped).toBe(1);
+  expect(result.didNotRun).toBe(1);
   expect(result.output).toContain('running a');
   expect(result.output).toContain('running b');
   expect(result.output).toContain('running c');

tests/playwright-test/playwright-test-fixtures.ts

@@ -43,6 +43,7 @@ export type RunResult = {
   flaky: number,
   skipped: number,
   interrupted: number,
+  didNotRun: number,
   report: JSONReport,
   results: any[],
 };
@@ -417,6 +418,7 @@ export function parseTestRunnerOutput(output: string) {
   const flaky = summary(/(\d+) flaky/g);
   const skipped = summary(/(\d+) skipped/g);
   const interrupted = summary(/(\d+) interrupted/g);
+  const didNotRun = summary(/(\d+) did not run/g);
 
   const strippedOutput = stripAnsi(output);
   return {
@@ -428,5 +430,6 @@ export function parseTestRunnerOutput(output: string) {
     flaky,
     skipped,
     interrupted,
+    didNotRun,
   };
 }

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

@@ -205,7 +205,7 @@ for (const useIntermediateMergeReport of [false, true] as const) {
       expect(result.exitCode).toBe(1);
       expect(result.failed).toBe(1);
       expect(result.passed).toBe(0);
-      expect(result.skipped).toBe(2);
+      expect(result.didNotRun).toBe(2);
       expect(result.output).toContain('Testing stopped early after 1 maximum allowed failures.');
     });
 

tests/playwright-test/runner.spec.ts

@@ -150,7 +150,7 @@ test('should ignore subprocess creation error because of SIGINT', async ({ inter
   const result = parseTestRunnerOutput(testProcess.output);
   expect(result.passed).toBe(0);
   expect(result.failed).toBe(0);
-  expect(result.skipped).toBe(2);
+  expect(result.didNotRun).toBe(2);
   expect(result.output).not.toContain('worker process exited unexpectedly');
 });
 
@@ -190,7 +190,7 @@ test('sigint should stop workers', async ({ interactWithTestRunner }) => {
   const result = parseTestRunnerOutput(testProcess.output);
   expect(result.passed).toBe(0);
   expect(result.failed).toBe(0);
-  expect(result.skipped).toBe(2);
+  expect(result.didNotRun).toBe(2);
   expect(result.interrupted).toBe(2);
   expect(result.output).toContain('%%SEND-SIGINT%%1');
   expect(result.output).toContain('%%SEND-SIGINT%%2');
@@ -206,7 +206,7 @@ test('sigint should stop workers', async ({ interactWithTestRunner }) => {
 
   const skipped2 = report.suites[1].specs[1];
   expect(skipped2.title).toBe('skipped2');
-  expect(skipped2.tests[0].results[0].workerIndex).toBe(-1);
+  expect(skipped2.tests[0].results).toHaveLength(0);
 });
 
 test('should use the first occurring error when an unhandled exception was thrown', async ({ runInlineTest }) => {