test: misc test improvements (#10371)

packages/puppeteer-core/src/common/bidi/Browser.ts

@@ -99,8 +99,9 @@ export class Browser extends BrowserBase {
     this.#browserName = opts.browserName;
     this.#browserVersion = opts.browserVersion;
 
-    this.#process?.on('close', () => {
-      return this.emit(BrowserEmittedEvents.Disconnected);
+    this.#process?.once('close', () => {
+      this.#connection.dispose();
+      this.emit(BrowserEmittedEvents.Disconnected);
     });
   }
 
@@ -109,6 +110,9 @@ export class Browser extends BrowserBase {
   }
 
   override async close(): Promise<void> {
+    if (this.#connection.closed) {
+      return;
+    }
     this.#connection.dispose();
     await this.#closeCallback?.call(null);
   }

test/src/ariaqueryhandler.spec.ts

@@ -427,6 +427,9 @@ describe('AriaQueryHandler', () => {
         .waitForSelector('aria/inner', {visible: true})
         .then(() => {
           return (divVisible = true);
+        })
+        .catch(() => {
+          return (divVisible = false);
         });
       await page.setContent(
         `<div style='display: none; visibility: hidden;'><div aria-label="inner">hi</div></div>`
@@ -456,6 +459,9 @@ describe('AriaQueryHandler', () => {
         .waitForSelector('aria/[role="button"]', {hidden: true})
         .then(() => {
           return (divHidden = true);
+        })
+        .catch(() => {
+          return (divHidden = false);
         });
       await page.waitForSelector('aria/[role="button"]'); // do a round trip
       expect(divHidden).toBe(false);
@@ -479,6 +485,9 @@ describe('AriaQueryHandler', () => {
         .waitForSelector('aria/[role="main"]', {hidden: true})
         .then(() => {
           return (divHidden = true);
+        })
+        .catch(() => {
+          return (divHidden = false);
         });
       await page.waitForSelector('aria/[role="main"]'); // do a round trip
       expect(divHidden).toBe(false);
@@ -500,6 +509,9 @@ describe('AriaQueryHandler', () => {
         .waitForSelector('aria/[role="main"]', {hidden: true})
         .then(() => {
           return (divRemoved = true);
+        })
+        .catch(() => {
+          return (divRemoved = false);
         });
       await page.waitForSelector('aria/[role="main"]'); // do a round trip
       expect(divRemoved).toBe(false);
@@ -553,9 +565,14 @@ describe('AriaQueryHandler', () => {
       const {page} = getTestState();
 
       let divFound = false;
-      const waitForSelector = page.waitForSelector('aria/zombo').then(() => {
-        return (divFound = true);
-      });
+      const waitForSelector = page
+        .waitForSelector('aria/zombo')
+        .then(() => {
+          return (divFound = true);
+        })
+        .catch(() => {
+          return (divFound = false);
+        });
       await page.setContent(`<div aria-label='notZombo'></div>`);
       expect(divFound).toBe(false);
       await page.evaluate(() => {
@@ -569,7 +586,9 @@ describe('AriaQueryHandler', () => {
     it('should return the element handle', async () => {
       const {page} = getTestState();
 
-      const waitForSelector = page.waitForSelector('aria/zombo');
+      const waitForSelector = page.waitForSelector('aria/zombo').catch(err => {
+        return err;
+      });
       await page.setContent(`<div aria-label='zombo'>anything</div>`);
       expect(
         await page.evaluate(x => {

test/src/elementhandle.spec.ts

@@ -389,23 +389,29 @@ describe('ElementHandle specs', function () {
   describe('Element.waitForSelector', () => {
     it('should wait correctly with waitForSelector on an element', async () => {
       const {page} = getTestState();
-      const waitFor = page.waitForSelector('.foo') as Promise<
-        ElementHandle<HTMLDivElement>
-      >;
+      const waitFor = page.waitForSelector('.foo').catch(err => {
+        return err;
+      }) as Promise<ElementHandle<HTMLDivElement>>;
       // Set the page content after the waitFor has been started.
       await page.setContent(
         '<div id="not-foo"></div><div class="bar">bar2</div><div class="foo">Foo1</div>'
       );
       let element = (await waitFor)!;
+      if (element instanceof Error) {
+        throw element;
+      }
       expect(element).toBeDefined();
 
-      const innerWaitFor = element.waitForSelector('.bar') as Promise<
-        ElementHandle<HTMLDivElement>
-      >;
+      const innerWaitFor = element.waitForSelector('.bar').catch(err => {
+        return err;
+      }) as Promise<ElementHandle<HTMLDivElement>>;
       await element.evaluate(el => {
         el.innerHTML = '<div class="bar">bar1</div>';
       });
       element = (await innerWaitFor)!;
+      if (element instanceof Error) {
+        throw element;
+      }
       expect(element).toBeDefined();
       expect(
         await element.evaluate(el => {
@@ -683,7 +689,9 @@ describe('ElementHandle specs', function () {
           return (element as Element).querySelector(`.${selector}`);
         },
       });
-      const waitFor = page.waitForSelector('getByClass/foo');
+      const waitFor = page.waitForSelector('getByClass/foo').catch(err => {
+        return err;
+      });
 
       // Set the page content after the waitFor has been started.
       await page.setContent(
@@ -691,6 +699,10 @@ describe('ElementHandle specs', function () {
       );
       const element = await waitFor;
 
+      if (element instanceof Error) {
+        throw element;
+      }
+
       expect(element).toBeDefined();
     });
 
@@ -701,26 +713,34 @@ describe('ElementHandle specs', function () {
           return (element as Element).querySelector(`.${selector}`);
         },
       });
-      const waitFor = page.waitForSelector('getByClass/foo') as Promise<
-        ElementHandle<HTMLElement>
-      >;
+      const waitFor = page.waitForSelector('getByClass/foo').catch(err => {
+        return err;
+      }) as Promise<ElementHandle<HTMLElement>>;
 
       // Set the page content after the waitFor has been started.
       await page.setContent(
         '<div id="not-foo"></div><div class="bar">bar2</div><div class="foo">Foo1</div>'
       );
       let element = (await waitFor)!;
+      if (element instanceof Error) {
+        throw element;
+      }
       expect(element).toBeDefined();
 
-      const innerWaitFor = element.waitForSelector('getByClass/bar') as Promise<
-        ElementHandle<HTMLElement>
-      >;
+      const innerWaitFor = element
+        .waitForSelector('getByClass/bar')
+        .catch(err => {
+          return err;
+        }) as Promise<ElementHandle<HTMLElement>>;
 
       await element.evaluate(el => {
         el.innerHTML = '<div class="bar">bar1</div>';
       });
 
       element = (await innerWaitFor)!;
+      if (element instanceof Error) {
+        throw element;
+      }
       expect(element).toBeDefined();
       expect(
         await element.evaluate(el => {
@@ -738,7 +758,9 @@ describe('ElementHandle specs', function () {
           return (element as Element).querySelector(`.${selector}`);
         },
       });
-      const waitFor = page.waitForSelector('getByClass/foo');
+      const waitFor = page.waitForSelector('getByClass/foo').catch(err => {
+        return err;
+      });
 
       // Set the page content after the waitFor has been started.
       await page.setContent(
@@ -746,6 +768,10 @@ describe('ElementHandle specs', function () {
       );
       const element = await waitFor;
 
+      if (element instanceof Error) {
+        throw element;
+      }
+
       expect(element).toBeDefined();
     });
     it('should work when both queryOne and queryAll are registered', async () => {

test/src/locator.spec.ts

@@ -150,7 +150,12 @@ describe('Locator', function () {
         <button style="display: none;" onclick="this.innerText = 'clicked';">test</button>
       `);
       const button = await page.$('button');
-      const result = page.locator('button').click();
+      const result = page
+        .locator('button')
+        .click()
+        .catch(err => {
+          return err;
+        });
       expect(
         await button?.evaluate(el => {
           return el.innerText;
@@ -159,7 +164,10 @@ describe('Locator', function () {
       await button?.evaluate(el => {
         el.style.display = 'block';
       });
-      await result;
+      const maybeError = await result;
+      if (maybeError instanceof Error) {
+        throw maybeError;
+      }
       expect(
         await button?.evaluate(el => {
           return el.innerText;

test/src/utils.ts

@@ -20,6 +20,7 @@ import expect from 'expect';
 import {Frame} from 'puppeteer-core/internal/api/Frame.js';
 import {Page} from 'puppeteer-core/internal/api/Page.js';
 import {EventEmitter} from 'puppeteer-core/internal/common/EventEmitter.js';
+import {Deferred} from 'puppeteer-core/internal/util/Deferred.js';
 
 import {compare} from './golden-utils.js';
 
@@ -132,19 +133,27 @@ export const dumpFrames = (frame: Frame, indentation?: string): string[] => {
   return result;
 };
 
-export const waitEvent = <T = any>(
+export const waitEvent = async <T = any>(
   emitter: EventEmitter,
   eventName: string,
   predicate: (event: T) => boolean = () => {
     return true;
   }
 ): Promise<T> => {
-  return new Promise(fulfill => {
-    emitter.on(eventName, (event: T) => {
-      if (!predicate(event)) {
-        return;
-      }
-      fulfill(event);
-    });
+  const deferred = Deferred.create<T>({
+    timeout: 5000,
+    message: 'Waiting for test event timed out.',
   });
+  const handler = (event: T) => {
+    if (!predicate(event)) {
+      return;
+    }
+    deferred.resolve(event);
+  };
+  emitter.on(eventName, handler);
+  try {
+    return await deferred.valueOrThrow();
+  } finally {
+    emitter.off(eventName, handler);
+  }
 };