chore: migrate tests to TypeScript (#6075)

This CL migrates all the tests to TypeScript. The main benefits of this is that we start consuming our TypeScript definitions and therefore find errors in them. The act of migrating found some bugs in our definitions and now we can be sure to avoid them going forwards.

You'll notice the addition of some `TODO`s in the code; I didn't want this CL to get any bigger than it already is but I intend to follow those up once this lands. It's mostly figuring out how to extend the `expect` types with our `toBeGolden` helpers and some other slight confusions with types that the tests exposed.

Co-authored-by: Mathias Bynens <mathias@qiwi.be>

mocha-config/puppeteer-unit-tests.js

@@ -18,8 +18,9 @@ const base = require('./base');
 
 module.exports = {
   ...base,
-  require: ['./test/mocha-utils.js'],
-  spec: 'test/*.spec.js',
+  require: ['ts-node/register', './test/mocha-utils.ts'],
+  spec: 'test/*.spec.ts',
+  extension: ['ts'],
   parallel: process.env.CI && !process.env.COVERAGE,
   // retry twice more, so we run each test up to 3 times if needed.
   retries: process.env.CI ? 2 : 0,

package.json

@@ -63,7 +63,8 @@
     "@microsoft/api-extractor": "7.8.12",
     "@types/debug": "0.0.31",
     "@types/mime": "^2.0.0",
-    "@types/node": "^10.17.14",
+    "@types/mocha": "^7.0.2",
+    "@types/node": "^14.0.13",
     "@types/proxy-from-env": "^1.0.1",
     "@types/rimraf": "^2.0.2",
     "@types/tar-fs": "^1.16.2",
@@ -89,6 +90,7 @@
     "prettier": "^2.0.5",
     "sinon": "^9.0.2",
     "text-diff": "^1.0.1",
+    "ts-node": "^8.10.2",
     "typescript": "3.9.2"
   },
   "browser": {

src/common/Browser.ts

@@ -310,7 +310,7 @@ export class BrowserContext extends EventEmitter {
 
   waitForTarget(
     predicate: (x: Target) => boolean,
-    options: { timeout?: number }
+    options: { timeout?: number } = {}
   ): Promise<Target> {
     return this._browser.waitForTarget(
       (target) => target.browserContext() === this && predicate(target),

src/common/Coverage.ts

@@ -36,20 +36,24 @@ export class Coverage {
     this._cssCoverage = new CSSCoverage(client);
   }
 
-  async startJSCoverage(options: {
-    resetOnNavigation?: boolean;
-    reportAnonymousScripts?: boolean;
-  }): Promise<void> {
+  async startJSCoverage(
+    options: {
+      resetOnNavigation?: boolean;
+      reportAnonymousScripts?: boolean;
+    } = {}
+  ): Promise<void> {
     return await this._jsCoverage.start(options);
   }
 
   async stopJSCoverage(): Promise<CoverageEntry[]> {
     return await this._jsCoverage.stop();
   }
 
-  async startCSSCoverage(options: {
-    resetOnNavigation?: boolean;
-  }): Promise<void> {
+  async startCSSCoverage(
+    options: {
+      resetOnNavigation?: boolean;
+    } = {}
+  ): Promise<void> {
     return await this._cssCoverage.start(options);
   }
 

src/common/FrameManager.ts

@@ -417,10 +417,12 @@ export class Frame {
     return await this._frameManager.navigateFrame(this, url, options);
   }
 
-  async waitForNavigation(options: {
-    timeout?: number;
-    waitUntil?: PuppeteerLifeCycleEvent | PuppeteerLifeCycleEvent[];
-  }): Promise<HTTPResponse | null> {
+  async waitForNavigation(
+    options: {
+      timeout?: number;
+      waitUntil?: PuppeteerLifeCycleEvent | PuppeteerLifeCycleEvent[];
+    } = {}
+  ): Promise<HTTPResponse | null> {
     return await this._frameManager.waitForFrameNavigation(this, options);
   }
 
@@ -523,7 +525,11 @@ export class Frame {
 
   async click(
     selector: string,
-    options: { delay?: number; button?: MouseButtonInput; clickCount?: number }
+    options: {
+      delay?: number;
+      button?: MouseButtonInput;
+      clickCount?: number;
+    } = {}
   ): Promise<void> {
     return this._secondaryWorld.click(selector, options);
   }
@@ -584,7 +590,7 @@ export class Frame {
 
   async waitForSelector(
     selector: string,
-    options: WaitForSelectorOptions
+    options: WaitForSelectorOptions = {}
   ): Promise<ElementHandle | null> {
     const handle = await this._secondaryWorld.waitForSelector(
       selector,
@@ -599,7 +605,7 @@ export class Frame {
 
   async waitForXPath(
     xpath: string,
-    options: WaitForSelectorOptions
+    options: WaitForSelectorOptions = {}
   ): Promise<ElementHandle | null> {
     const handle = await this._secondaryWorld.waitForXPath(xpath, options);
     if (!handle) return null;

src/common/Input.ts

@@ -122,7 +122,7 @@ export class Keyboard {
     return !!keyDefinitions[char];
   }
 
-  async type(text: string, options: { delay?: number }): Promise<void> {
+  async type(text: string, options: { delay?: number } = {}): Promise<void> {
     const delay = (options && options.delay) || null;
     for (const char of text) {
       if (this.charIsKey(char)) {

src/common/JSHandle.ts

@@ -430,7 +430,7 @@ export class ElementHandle extends JSHandle {
    * uses {@link Page.mouse} to click in the center of the element.
    * If the element is detached from DOM, the method throws an error.
    */
-  async click(options: ClickOptions): Promise<void> {
+  async click(options: ClickOptions = {}): Promise<void> {
     await this._scrollIntoViewIfNeeded();
     const { x, y } = await this._clickablePoint();
     await this._page.mouse.click(x, y, options);

src/common/Page.ts

@@ -44,7 +44,7 @@ import Protocol from '../protocol';
 
 const writeFileAsync = helper.promisify(fs.writeFile);
 
-interface Metrics {
+export interface Metrics {
   Timestamp?: number;
   Documents?: number;
   Frames?: number;
@@ -129,14 +129,13 @@ const paperFormats: Record<string, PaperFormat> = {
   a6: { width: 4.13, height: 5.83 },
 } as const;
 
-enum VisionDeficiency {
-  none = 'none',
-  achromatopsia = 'achromatopsia',
-  blurredVision = 'blurredVision',
-  deuteranopia = 'deuteranopia',
-  protanopia = 'protanopia',
-  tritanopia = 'tritanopia',
-}
+type VisionDeficiency =
+  | 'none'
+  | 'achromatopsia'
+  | 'blurredVision'
+  | 'deuteranopia'
+  | 'protanopia'
+  | 'tritanopia';
 
 /**
  * All the events that a page instance may emit.
@@ -837,7 +836,7 @@ export class Page extends EventEmitter {
     return await this._frameManager.mainFrame().content();
   }
 
-  async setContent(html: string, options: WaitForOptions): Promise<void> {
+  async setContent(html: string, options: WaitForOptions = {}): Promise<void> {
     await this._frameManager.mainFrame().setContent(html, options);
   }
 
@@ -913,11 +912,11 @@ export class Page extends EventEmitter {
     );
   }
 
-  async goBack(options: WaitForOptions): Promise<HTTPResponse | null> {
+  async goBack(options: WaitForOptions = {}): Promise<HTTPResponse | null> {
     return this._go(-1, options);
   }
 
-  async goForward(options: WaitForOptions): Promise<HTTPResponse | null> {
+  async goForward(options: WaitForOptions = {}): Promise<HTTPResponse | null> {
     return this._go(+1, options);
   }
 
@@ -1005,7 +1004,14 @@ export class Page extends EventEmitter {
   }
 
   async emulateVisionDeficiency(type?: VisionDeficiency): Promise<void> {
-    const visionDeficiencies = new Set(Object.keys(VisionDeficiency));
+    const visionDeficiencies = new Set([
+      'none',
+      'achromatopsia',
+      'blurredVision',
+      'deuteranopia',
+      'protanopia',
+      'tritanopia',
+    ]);
     try {
       assert(
         !type || visionDeficiencies.has(type),
@@ -1356,7 +1362,7 @@ export class Page extends EventEmitter {
   }
 
   waitForFunction(
-    pageFunction: Function,
+    pageFunction: Function | string,
     options: {
       timeout?: number;
       polling?: string | number;

src/common/Puppeteer.ts

@@ -134,7 +134,7 @@ export class Puppeteer {
     return puppeteerErrors;
   }
 
-  defaultArgs(options: ChromeArgOptions): string[] {
+  defaultArgs(options: ChromeArgOptions = {}): string[] {
     return this._launcher.defaultArgs(options);
   }
 

src/index.ts

@@ -31,4 +31,4 @@ const puppeteer = initializePuppeteer({
  * And therefore consuming via require('puppeteer') would break / require the user
  * to access require('puppeteer').default;
  */
-module.exports = puppeteer;
+export = puppeteer;

src/node/BrowserFetcher.ts

@@ -215,7 +215,7 @@ export class BrowserFetcher {
    */
   async download(
     revision: string,
-    progressCallback: (x: number, y: number) => void
+    progressCallback: (x: number, y: number) => void = (): void => {}
   ): Promise<BrowserFetcherRevisionInfo> {
     const url = downloadURL(
       this._product,

test/CDPSession.spec.js → test/CDPSession.spec.ts

@@ -14,13 +14,14 @@
  * limitations under the License.
  */
 
-const { waitEvent } = require('./utils');
-const expect = require('expect');
-const {
+import { waitEvent } from './utils';
+import expect from 'expect';
+import {
   getTestState,
   setupTestBrowserHooks,
   setupTestPageAndContextHooks,
-} = require('./mocha-utils');
+  describeChromeOnly,
+} from './mocha-utils';
 
 describeChromeOnly('Target.createCDPSession', function () {
   setupTestBrowserHooks();
@@ -35,7 +36,7 @@ describeChromeOnly('Target.createCDPSession', function () {
       client.send('Runtime.enable'),
       client.send('Runtime.evaluate', { expression: 'window.foo = "bar"' }),
     ]);
-    const foo = await page.evaluate(() => window.foo);
+    const foo = await page.evaluate(() => globalThis.foo);
     expect(foo).toBe('bar');
   });
   it('should send events', async () => {
@@ -96,6 +97,9 @@ describeChromeOnly('Target.createCDPSession', function () {
     expect(error.message).toContain('ThisCommand.DoesNotExist');
 
     async function theSourceOfTheProblems() {
+      // This fails in TS as it knows that command does not exist but we want to
+      // have this tests for our users who consume in JS not TS.
+      // @ts-expect-error
       await client.send('ThisCommand.DoesNotExist');
     }
   });

test/EventEmitter.spec.js → test/EventEmitter.spec.ts

@@ -1,6 +1,22 @@
-const { EventEmitter } = require('../lib/common/EventEmitter');
-const sinon = require('sinon');
-const expect = require('expect');
+/**
+ * Copyright 2020 Google Inc. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { EventEmitter } from '../src/common/EventEmitter';
+import sinon from 'sinon';
+import expect from 'expect';
 
 describe('EventEmitter', () => {
   let emitter;
@@ -10,7 +26,7 @@ describe('EventEmitter', () => {
   });
 
   describe('on', () => {
-    const onTests = (methodName) => {
+    const onTests = (methodName: 'on' | 'addListener'): void => {
       it(`${methodName}: adds an event listener that is fired when the event is emitted`, () => {
         const listener = sinon.spy();
         emitter[methodName]('foo', listener);
@@ -39,7 +55,7 @@ describe('EventEmitter', () => {
   });
 
   describe('off', () => {
-    const offTests = (methodName) => {
+    const offTests = (methodName: 'off' | 'removeListener'): void => {
       it(`${methodName}: removes the listener so it is no longer called`, () => {
         const listener = sinon.spy();
         emitter.on('foo', listener);

test/accessibility.spec.js → test/accessibility.spec.ts

@@ -14,12 +14,13 @@
  * limitations under the License.
  */
 
-const expect = require('expect');
-const {
+import expect from 'expect';
+import {
   getTestState,
   setupTestBrowserHooks,
   setupTestPageAndContextHooks,
-} = require('./mocha-utils');
+  describeFailsFirefox,
+} from './mocha-utils';
 
 describeFailsFirefox('Accessibility', function () {
   setupTestBrowserHooks();

test/assets/input/textarea.html

@@ -7,9 +7,9 @@
     <textarea></textarea>
     <script src='mouse-helper.js'></script>
     <script>
-      window.result = '';
-      let textarea = document.querySelector('textarea');
+      globalThis.result = '';
+      globalThis.textarea = document.querySelector('textarea');
       textarea.addEventListener('input', () => result = textarea.value, false);
     </script>
   </body>
-</html>
+</html>

test/assets/shadow.html

@@ -1,8 +1,8 @@
 <script>
 
 let h1 = null;
-let button = null;
-let clicked = false;
+window.button = null;
+window.clicked = false;
 
 window.addEventListener('DOMContentLoaded', () => {
   const shadowRoot = document.body.attachShadow({mode: 'open'});

test/browser.spec.js → test/browser.spec.ts

@@ -14,8 +14,8 @@
  * limitations under the License.
  */
 
-const expect = require('expect');
-const { getTestState, setupTestBrowserHooks } = require('./mocha-utils');
+import expect from 'expect';
+import { getTestState, setupTestBrowserHooks } from './mocha-utils';
 
 describe('Browser specs', function () {
   setupTestBrowserHooks();

test/browsercontext.spec.js → test/browsercontext.spec.ts

@@ -14,9 +14,13 @@
  * limitations under the License.
  */
 
-const expect = require('expect');
-const { getTestState, setupTestBrowserHooks } = require('./mocha-utils');
-const utils = require('./utils');
+import expect from 'expect';
+import {
+  getTestState,
+  setupTestBrowserHooks,
+  itFailsFirefox,
+} from './mocha-utils';
+import utils from './utils';
 
 describe('BrowserContext', function () {
   setupTestBrowserHooks();