PRO-14812 fix: really skip skipped tests in Playwright

Author: uid11

src/types/checks.ts

@@ -1,15 +1,15 @@
 /**
  * This type checks that the type `true` is passed to it.
  */
-export type Expect<T extends true> = T;
+export type Expect<Type extends true> = Type;
 
 /**
  * Returns `true` if types are exactly equal and `false` otherwise.
  * IsEqual<{foo: string}, {foo: string}> = true.
  * IsEqual<{readonly foo: string}, {foo: string}> = false.
  */
 export type IsEqual<X, Y> =
-  (<T>() => T extends X ? 1 : 2) extends <T>() => T extends Y ? 1 : 2 ? true : false;
+  (<Type>() => Type extends X ? 1 : 2) extends <Type>() => Type extends Y ? 1 : 2 ? true : false;
 
 /**
  * Returns `true` if key is readonly in object and `false` otherwise.

src/utils/expect/additionalMatchers.ts

@@ -5,7 +5,7 @@ import {toMatchScreenshot} from './toMatchScreenshot';
 import type {Expect} from './Expect';
 import type {NonSelectorAdditionalMatchers, SelectorMatchers} from './types';
 
-import {expect} from '@playwright/test';
+import {expect as playwrightExpect} from '@playwright/test';
 
 /**
  * Addition matchers.
@@ -16,68 +16,68 @@ export const additionalMatchers: NonSelectorAdditionalMatchers<unknown> & Select
     const {actualValue, description} = this;
 
     if (typeof actualValue === 'string' || Array.isArray(actualValue)) {
-      return expect(actualValue, description).toContain(expected);
+      return playwrightExpect(actualValue, description).toContain(expected);
     }
 
-    return expect(actualValue, description).toEqual(
-      expect.objectContaining(expected as Record<string, unknown>),
+    return playwrightExpect(actualValue, description).toEqual(
+      playwrightExpect.objectContaining(expected as Record<string, unknown>),
     );
   },
   async eql(this: Expect, expected) {
     const {actualValue, description} = this;
 
-    return expect(actualValue, description).toEqual(expected);
+    return playwrightExpect(actualValue, description).toEqual(expected);
   },
   async gt(this: Expect, expected) {
     const {actualValue, description} = this;
 
-    return expect(actualValue, description).toBeGreaterThan(expected);
+    return playwrightExpect(actualValue, description).toBeGreaterThan(expected);
   },
   async gte(this: Expect, expected) {
     const {actualValue, description} = this;
 
-    return expect(actualValue, description).toBeGreaterThanOrEqual(expected);
+    return playwrightExpect(actualValue, description).toBeGreaterThanOrEqual(expected);
   },
   async lt(this: Expect, expected) {
     const {actualValue, description} = this;
 
-    return expect(actualValue, description).toBeLessThan(expected);
+    return playwrightExpect(actualValue, description).toBeLessThan(expected);
   },
   async lte(this: Expect, expected) {
     const {actualValue, description} = this;
 
-    return expect(actualValue, description).toBeLessThanOrEqual(expected);
+    return playwrightExpect(actualValue, description).toBeLessThanOrEqual(expected);
   },
   async match(this: Expect, expected) {
     const {actualValue, description} = this;
 
-    return expect(actualValue, description).toMatch(expected);
+    return playwrightExpect(actualValue, description).toMatch(expected);
   },
   async notContains(this: Expect, expected) {
     const {actualValue, description} = this;
 
     if (typeof actualValue === 'string' || Array.isArray(actualValue)) {
-      return expect(actualValue, description).not.toContain(expected);
+      return playwrightExpect(actualValue, description).not.toContain(expected);
     }
 
-    return expect(actualValue, description).not.toEqual(
-      expect.objectContaining(expected as Record<string, unknown>),
+    return playwrightExpect(actualValue, description).not.toEqual(
+      playwrightExpect.objectContaining(expected as Record<string, unknown>),
     );
   },
   async notEql(this: Expect, expected) {
     const {actualValue, description} = this;
 
-    return expect(actualValue, description).not.toEqual(expected);
+    return playwrightExpect(actualValue, description).not.toEqual(expected);
   },
   async notOk(this: Expect) {
     const {actualValue, description} = this;
 
-    return expect(actualValue, description).not.toBeTruthy();
+    return playwrightExpect(actualValue, description).not.toBeTruthy();
   },
   async ok(this: Expect) {
     const {actualValue, description} = this;
 
-    return expect(actualValue, description).toBeTruthy();
+    return playwrightExpect(actualValue, description).toBeTruthy();
   },
 
   toMatchScreenshot(this: Expect, expectedScreenshotId, options = {}) {

src/utils/expect/applyAdditionalMatcher.ts

@@ -4,7 +4,7 @@ import type {Fn, Selector, SelectorPropertyRetryData} from '../../types/internal
 
 import type {Expect} from './Expect';
 
-import {expect} from '@playwright/test';
+import {expect as playwrightExpect} from '@playwright/test';
 
 /**
  * Apply additional matcher (with retrying, if needed).
@@ -25,7 +25,7 @@ export const applyAdditionalMatcher = (
 
   let context: Expect;
 
-  return expect(() => {
+  return playwrightExpect(() => {
     const {args: selectorArgs, property, selector} = selectorPropertyRetryData;
 
     const actualValue =

src/utils/expect/createExpectMethod.ts

@@ -18,7 +18,7 @@ import type {Fn, SelectorPropertyRetryData} from '../../types/internal';
 import type {Expect} from './Expect';
 import type {AssertionFunction, ExpectMethod} from './types';
 
-import {expect} from '@playwright/test';
+import {expect as playwrightExpect} from '@playwright/test';
 
 const additionalAssertionTimeoutInMs = 1_000;
 
@@ -66,7 +66,7 @@ export const createExpectMethod = (
         });
       }
 
-      const assertion = expect(value, ctx.description) as unknown as Record<
+      const assertion = playwrightExpect(value, ctx.description) as unknown as Record<
         string,
         Fn<unknown[], Promise<unknown>>
       >;

src/utils/expect/toMatchScreenshot.ts

@@ -20,7 +20,7 @@ import type {FilePathFromRoot, Selector, ToMatchScreenshotOptions, Url} from '..
 
 import type {Expect} from './Expect';
 
-import {expect} from '@playwright/test';
+import {expect as playwrightExpect} from '@playwright/test';
 
 type AdditionalLogFields = {
   actualScreenshotId: string | undefined;
@@ -99,7 +99,7 @@ export const toMatchScreenshot = async (
   try {
     const playwrightLocator = actualValue.getPlaywrightLocator();
 
-    await expect(playwrightLocator, description).toHaveScreenshot(screenshotFileName, {
+    await playwrightExpect(playwrightLocator, description).toHaveScreenshot(screenshotFileName, {
       mask: mask.map((selector) => selector.getPlaywrightLocator()),
       ...restOptions,
     });

src/utils/report/client/groupLogEvents.ts

@@ -1,4 +1,4 @@
-import {LogEventType} from '../../../constants/internal';
+import {LogEventStatus, LogEventType} from '../../../constants/internal';
 
 import type {LogEvent, LogEventWithChildren} from '../../../types/internal';
 
@@ -16,14 +16,18 @@ export const groupLogEvents = (logEvents: readonly LogEvent[]): readonly LogEven
     LogEventType.InternalAssert,
   ];
 
+  const isTopLevelEvent = (logEvent: LogEvent): boolean =>
+    topLevelTypes.includes(logEvent.type) ||
+    logEvent.payload?.logEventStatus === LogEventStatus.Failed;
+
   const result: LogEventWithChildren[] = [];
 
   for (const logEvent of logEvents) {
     const last = result.at(-1);
     const newEvent: LogEventWithChildren = {children: [], ...logEvent};
 
-    if (topLevelTypes.includes(logEvent.type)) {
-      if (last && !topLevelTypes.includes(last.type)) {
+    if (isTopLevelEvent(logEvent)) {
+      if (last && !isTopLevelEvent(last)) {
         const firstTopLevel: LogEventWithChildren = {
           children: [...result],
           message: 'Initialization',
@@ -38,7 +42,7 @@ export const groupLogEvents = (logEvents: readonly LogEvent[]): readonly LogEven
       }
 
       result.push(newEvent);
-    } else if (last && topLevelTypes.includes(last.type)) {
+    } else if (last && isTopLevelEvent(last)) {
       (last.children as LogEventWithChildren[]).push(newEvent);
     } else {
       result.push(newEvent);

src/utils/report/client/render/TestRunDescription.tsx

@@ -31,7 +31,7 @@ export const TestRunDescription: JSX.Component<Props> = ({fullTestRun}) => {
     const valueWithLinks = parseMarkdownLinks`${value}`;
     const metaHtml = (
       <>
-        <dt class="test-description__term">{key}</dt>
+        <dt class="test-description__term">{key}</dt>{' '}
         <dd class="test-description__definition">{valueWithLinks}</dd>
       </>
     );
@@ -49,7 +49,7 @@ export const TestRunDescription: JSX.Component<Props> = ({fullTestRun}) => {
 
     traceHtml = (
       <>
-        <dt class="test-description__term">{traceLabel}</dt>
+        <dt class="test-description__term">{traceLabel}</dt>{' '}
         <dd class="test-description__definition">
           <a href={traceUrl} download={traceName} aria-label={traceLabel}>
             {traceName}
@@ -63,11 +63,11 @@ export const TestRunDescription: JSX.Component<Props> = ({fullTestRun}) => {
     <dl class="test-description" aria-label="Test meta data">
       <List elements={metaHtmls} />
       {traceHtml}
-      <dt class="test-description__term">Date</dt>
+      <dt class="test-description__term">Date</dt>{' '}
       <dd class="test-description__definition">
         <DatesInterval endTimeInMs={endTimeInMs} startTimeInMs={startTimeInMs} />
       </dd>
-      <dt class="test-description__term">Duration</dt>
+      <dt class="test-description__term">Duration</dt>{' '}
       <dd class="test-description__definition">
         <Duration durationInMs={endTimeInMs - startTimeInMs} />
       </dd>

src/utils/test/beforeTest.ts

@@ -21,7 +21,7 @@ import {getTestFnAndReject} from './getTestFnAndReject';
 
 import type {TestRunEvent, TestUnit, UtcTimeInMs} from '../../types/internal';
 
-import {test} from '@playwright/test';
+import {test as playwrightTest} from '@playwright/test';
 
 const additionToPlaywrightTestTimeout = 500;
 
@@ -59,7 +59,9 @@ export const beforeTest = ({
   const testTimeout =
     IS_DEBUG || isUiMode ? MAX_TIMEOUT_IN_MS : (options.testTimeout ?? testTimeoutFromConfig);
 
-  test.setTimeout(testTimeout + additionToPlaywrightTestTimeout + (Date.now() - startTimeInMs));
+  playwrightTest.setTimeout(
+    testTimeout + additionToPlaywrightTestTimeout + (Date.now() - startTimeInMs),
+  );
 
   setTestIdleTimeout(testIdleTimeout);
   setTestTimeout(testTimeout);
@@ -75,6 +77,7 @@ export const beforeTest = ({
   const {onlog, reject, testFnWithReject} = getTestFnAndReject({
     isSkipped,
     runId,
+    skipReason,
     testFn,
     testIdleTimeout,
     testTimeout,

src/utils/test/getTestFnAndReject.ts

@@ -8,22 +8,19 @@ import {getPromiseWithResolveAndReject} from '../promise';
 
 import type {Onlog, RejectTestRun, RunId, TestFn, Void} from '../../types/internal';
 
+import {test as playwrightTest} from '@playwright/test';
+
 type Options = Readonly<{
   isSkipped: boolean;
   runId: RunId;
+  skipReason: string | undefined;
   testFn: TestFn;
   testIdleTimeout: number;
   testTimeout: number;
 }>;
 
 type Return = Readonly<{onlog: Onlog; reject: RejectTestRun; testFnWithReject: TestFn}>;
 
-const skippedTestFnAndReject: Return = {
-  onlog: () => undefined,
-  reject: () => undefined,
-  testFnWithReject: () => RESOLVED_PROMISE,
-};
-
 /**
  * Get test function with execution timeout, idle timeout, reject and onlog functions,
  * by isSkipped flag, test function, runId, test execution timeout and test idle timeouts.
@@ -32,12 +29,23 @@ const skippedTestFnAndReject: Return = {
 export const getTestFnAndReject = ({
   isSkipped,
   runId,
+  skipReason,
   testFn,
   testIdleTimeout,
   testTimeout,
 }: Options): Return => {
   if (isSkipped) {
-    return skippedTestFnAndReject;
+    return {
+      onlog: () => undefined,
+      reject: () => undefined,
+      testFnWithReject: () => {
+        try {
+          playwrightTest.skip(true, skipReason);
+        } catch {}
+
+        return RESOLVED_PROMISE;
+      },
+    };
   }
 
   const {

src/utils/test/waitBeforeRetry.ts

@@ -7,7 +7,7 @@ import {getPreviousRunId} from './getPreviousRunId';
 
 import type {FullTestRun, RunId, TestStaticOptions} from '../../types/internal';
 
-import {test} from '@playwright/test';
+import {test as playwrightTest} from '@playwright/test';
 
 const additionToTimeout = 10_000;
 
@@ -57,7 +57,7 @@ export const waitBeforeRetry = async (
       return;
     }
 
-    test.setTimeout(timeoutInMs + additionToTimeout);
+    playwrightTest.setTimeout(timeoutInMs + additionToTimeout);
 
     const timeoutObject = setInterval(() => {
       void writeLogEventTime();