From 7994315d0b5178fd5c78e34486c31ee9d9dd0263 Mon Sep 17 00:00:00 2001
From: Yury Semikhatsky <yurys@chromium.org>
Date: Thu, 23 May 2024 15:28:13 -0700
Subject: [PATCH] address comments

---
 packages/playwright/src/common/globals.ts  | 18 ---------------
 packages/playwright/src/matchers/expect.ts | 26 +++++++++++++++++-----
 packages/playwright/types/test.d.ts        |  2 +-
 utils/generate_types/overrides-test.d.ts   |  2 +-
 4 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/packages/playwright/src/common/globals.ts b/packages/playwright/src/common/globals.ts
index c2ead505e6..746e1ec847 100644
--- a/packages/playwright/src/common/globals.ts
+++ b/packages/playwright/src/common/globals.ts
@@ -33,24 +33,6 @@ export function currentlyLoadingFileSuite() {
   return currentFileSuite;
 }
 
-let currentExpectConfigureTimeout: number | undefined;
-
-export function setCurrentExpectConfigureTimeout(timeout: number | undefined) {
-  currentExpectConfigureTimeout = timeout;
-}
-
-export function currentExpectTimeout(options: { timeout?: number }) {
-  const testInfo = currentTestInfo();
-  if (options.timeout !== undefined)
-    return options.timeout;
-  if (currentExpectConfigureTimeout !== undefined)
-    return currentExpectConfigureTimeout;
-  let defaultExpectTimeout = testInfo?._projectInternal?.expect?.timeout;
-  if (typeof defaultExpectTimeout === 'undefined')
-    defaultExpectTimeout = 5000;
-  return defaultExpectTimeout;
-}
-
 let _isWorkerProcess = false;
 
 export function setIsWorkerProcess() {
diff --git a/packages/playwright/src/matchers/expect.ts b/packages/playwright/src/matchers/expect.ts
index ae629fae74..9a010992da 100644
--- a/packages/playwright/src/matchers/expect.ts
+++ b/packages/playwright/src/matchers/expect.ts
@@ -50,7 +50,7 @@ import {
 } from './matchers';
 import { toMatchSnapshot, toHaveScreenshot, toHaveScreenshotStepTitle } from './toMatchSnapshot';
 import type { Expect, ExpectMatcherState } from '../../types/test';
-import { currentTestInfo, currentExpectTimeout, setCurrentExpectConfigureTimeout } from '../common/globals';
+import { currentTestInfo } from '../common/globals';
 import { filteredStackTrace, trimLongString } from '../util';
 import {
   expect as expectLibrary,
@@ -129,18 +129,18 @@ function createExpect(info: ExpectMetaInfo) {
       if (property === 'extend') {
         return (matchers: any) => {
           const wrappedMatchers: any = {};
-          Object.entries(matchers).forEach(([name, matcher]) => {
+          for (const [name, matcher] of Object.entries(matchers)) {
             wrappedMatchers[name] = function(...args: any[]) {
               const { isNot, promise, utils } = this;
               const newThis: ExpectMatcherState = {
                 isNot,
                 promise,
                 utils,
-                timeout: currentExpectTimeout({})
+                timeout: currentExpectTimeout()
               };
               return (matcher as any).call(newThis, ...args);
             };
-          });
+          }
           expectLibrary.extend(wrappedMatchers);
           return expectInstance;
         };
@@ -255,7 +255,7 @@ class ExpectMetaInfoProxyHandler implements ProxyHandler<any> {
     if (this._info.isPoll) {
       if ((customAsyncMatchers as any)[matcherName] || matcherName === 'resolves' || matcherName === 'rejects')
         throw new Error(`\`expect.poll()\` does not support "${matcherName}" matcher.`);
-      matcher = (...args: any[]) => pollMatcher(matcherName, !!this._info.isNot, this._info.pollIntervals, currentExpectTimeout({ timeout: this._info.pollTimeout }), this._info.generator!, ...args);
+      matcher = (...args: any[]) => pollMatcher(matcherName, !!this._info.isNot, this._info.pollIntervals, this._info.pollTimeout ?? currentExpectTimeout(), this._info.generator!, ...args);
     }
     return (...args: any[]) => {
       const testInfo = currentTestInfo();
@@ -347,6 +347,22 @@ async function pollMatcher(matcherName: any, isNot: boolean, pollIntervals: numb
   }
 }
 
+let currentExpectConfigureTimeout: number | undefined;
+
+function setCurrentExpectConfigureTimeout(timeout: number | undefined) {
+  currentExpectConfigureTimeout = timeout;
+}
+
+function currentExpectTimeout() {
+  if (currentExpectConfigureTimeout !== undefined)
+    return currentExpectConfigureTimeout;
+  const testInfo = currentTestInfo();
+  let defaultExpectTimeout = testInfo?._projectInternal?.expect?.timeout;
+  if (typeof defaultExpectTimeout === 'undefined')
+    defaultExpectTimeout = 5000;
+  return defaultExpectTimeout;
+}
+
 function computeArgsSuffix(matcherName: string, args: any[]) {
   let value = '';
   if (matcherName === 'toHaveScreenshot')
diff --git a/packages/playwright/types/test.d.ts b/packages/playwright/types/test.d.ts
index f426448578..e1299272a8 100644
--- a/packages/playwright/types/test.d.ts
+++ b/packages/playwright/types/test.d.ts
@@ -6511,7 +6511,7 @@ export interface ExpectMatcherUtils {
 
 export type ExpectMatcherState = {
   /**
-   * Wehther this matcher was called with the negated .not modifier.
+   * Whether this matcher was called with the negated .not modifier.
    */
   isNot: boolean;
   /**
diff --git a/utils/generate_types/overrides-test.d.ts b/utils/generate_types/overrides-test.d.ts
index b6c5197ea1..0367f3259c 100644
--- a/utils/generate_types/overrides-test.d.ts
+++ b/utils/generate_types/overrides-test.d.ts
@@ -359,7 +359,7 @@ export interface ExpectMatcherUtils {
 
 export type ExpectMatcherState = {
   /**
-   * Wehther this matcher was called with the negated .not modifier.
+   * Whether this matcher was called with the negated .not modifier.
    */
   isNot: boolean;
   /**