From 28797dee41dbe0acb0a8d15209ca5845cef7f64c Mon Sep 17 00:00:00 2001 From: Jack Franklin Date: Tue, 23 Jun 2020 06:18:46 +0100 Subject: [PATCH] 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 --- mocha-config/puppeteer-unit-tests.js | 5 +- package.json | 4 +- src/common/Browser.ts | 2 +- src/common/Coverage.ts | 18 +- src/common/FrameManager.ts | 20 +- src/common/Input.ts | 2 +- src/common/JSHandle.ts | 2 +- src/common/Page.ts | 34 ++-- src/common/Puppeteer.ts | 2 +- src/index.ts | 2 +- src/node/BrowserFetcher.ts | 2 +- ...{CDPSession.spec.js => CDPSession.spec.ts} | 14 +- ...ntEmitter.spec.js => EventEmitter.spec.ts} | 26 ++- ...sibility.spec.js => accessibility.spec.ts} | 7 +- test/assets/input/textarea.html | 6 +- test/assets/shadow.html | 4 +- test/{browser.spec.js => browser.spec.ts} | 4 +- ...context.spec.js => browsercontext.spec.ts} | 10 +- ...omiumonly.spec.js => chromiumonly.spec.ts} | 7 +- test/{click.spec.js => click.spec.ts} | 57 +++--- test/{cookies.spec.js => cookies.spec.ts} | 7 +- test/coverage-utils.js | 3 + test/{coverage.spec.js => coverage.spec.ts} | 11 +- ....spec.js => defaultbrowsercontext.spec.ts} | 7 +- test/{dialog.spec.js => dialog.spec.ts} | 9 +- ...nthandle.spec.js => elementhandle.spec.ts} | 73 +++++--- test/{emulation.spec.js => emulation.spec.ts} | 33 +++- ...{evaluation.spec.js => evaluation.spec.ts} | 47 ++--- test/{fixtures.spec.js => fixtures.spec.ts} | 12 +- test/{frame.spec.js => frame.spec.ts} | 27 +-- test/golden-utils.js | 1 + test/{headful.spec.js => headful.spec.ts} | 28 +-- ...rors.spec.js => ignorehttpserrors.spec.ts} | 8 +- test/{input.spec.js => input.spec.ts} | 21 ++- test/{jshandle.spec.js => jshandle.spec.ts} | 21 ++- test/{keyboard.spec.js => keyboard.spec.ts} | 79 ++++---- test/{launcher.spec.js => launcher.spec.ts} | 48 +++-- test/{mocha-utils.js => mocha-utils.ts} | 117 ++++++++---- test/{mouse.spec.js => mouse.spec.ts} | 66 +++---- ...{navigation.spec.js => navigation.spec.ts} | 37 ++-- test/{network.spec.js => network.spec.ts} | 23 ++- test/{oopif.spec.js => oopif.spec.ts} | 4 +- test/{page.spec.js => page.spec.ts} | 175 ++++++++++-------- ...selector.spec.js => queryselector.spec.ts} | 6 +- ...on.spec.js => requestinterception.spec.ts} | 25 +-- ...{screenshot.spec.js => screenshot.spec.ts} | 41 +++- test/{target.spec.js => target.spec.ts} | 27 ++- ...ouchscreen.spec.js => touchscreen.spec.ts} | 11 +- test/{tracing.spec.js => tracing.spec.ts} | 14 +- test/tsconfig.json | 7 + test/utils.js | 3 + test/{waittask.spec.js => waittask.spec.ts} | 66 ++++--- test/{worker.spec.js => worker.spec.ts} | 23 ++- tsconfig.json | 3 +- utils/testserver/index.js | 7 + 55 files changed, 797 insertions(+), 521 deletions(-) rename test/{CDPSession.spec.js => CDPSession.spec.ts} (90%) rename test/{EventEmitter.spec.js => EventEmitter.spec.ts} (84%) rename test/{accessibility.spec.js => accessibility.spec.ts} (99%) rename test/{browser.spec.js => browser.spec.ts} (96%) rename test/{browsercontext.spec.js => browsercontext.spec.ts} (97%) rename test/{chromiumonly.spec.js => chromiumonly.spec.ts} (98%) rename test/{click.spec.js => click.spec.ts} (86%) rename test/{cookies.spec.js => cookies.spec.ts} (99%) rename test/{coverage.spec.js => coverage.spec.ts} (97%) rename test/{defaultbrowsercontext.spec.js => defaultbrowsercontext.spec.ts} (97%) rename test/{dialog.spec.js => dialog.spec.ts} (95%) rename test/{elementhandle.spec.js => elementhandle.spec.ts} (87%) rename test/{emulation.spec.js => emulation.spec.ts} (90%) rename test/{evaluation.spec.js => evaluation.spec.ts} (92%) rename test/{fixtures.spec.js => fixtures.spec.ts} (92%) rename test/{frame.spec.js => frame.spec.ts} (93%) rename test/{headful.spec.js => headful.spec.ts} (93%) rename test/{ignorehttpserrors.spec.js => ignorehttpserrors.spec.ts} (97%) rename test/{input.spec.js => input.spec.ts} (96%) rename test/{jshandle.spec.js => jshandle.spec.ts} (95%) rename test/{keyboard.spec.js => keyboard.spec.ts} (83%) rename test/{launcher.spec.js => launcher.spec.ts} (96%) rename test/{mocha-utils.js => mocha-utils.ts} (65%) rename test/{mouse.spec.js => mouse.spec.ts} (82%) rename test/{navigation.spec.js => navigation.spec.ts} (96%) rename test/{network.spec.js => network.spec.ts} (97%) rename test/{oopif.spec.js => oopif.spec.ts} (95%) rename test/{page.spec.js => page.spec.ts} (92%) rename test/{queryselector.spec.js => queryselector.spec.ts} (99%) rename test/{requestinterception.spec.js => requestinterception.spec.ts} (98%) rename test/{screenshot.spec.js => screenshot.spec.ts} (88%) rename test/{target.spec.js => target.spec.ts} (94%) rename test/{touchscreen.spec.js => touchscreen.spec.ts} (86%) rename test/{tracing.spec.js => tracing.spec.ts} (93%) create mode 100644 test/tsconfig.json rename test/{waittask.spec.js => waittask.spec.ts} (92%) rename test/{worker.spec.js => worker.spec.ts} (85%) diff --git a/mocha-config/puppeteer-unit-tests.js b/mocha-config/puppeteer-unit-tests.js index 23fb4f8cfa329..fb11fd18d0d4c 100644 --- a/mocha-config/puppeteer-unit-tests.js +++ b/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, diff --git a/package.json b/package.json index 80a1f4a861c45..cf516d67b0723 100644 --- a/package.json +++ b/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": { diff --git a/src/common/Browser.ts b/src/common/Browser.ts index 8aa7d7a291ad6..474b8c93f1213 100644 --- a/src/common/Browser.ts +++ b/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 { return this._browser.waitForTarget( (target) => target.browserContext() === this && predicate(target), diff --git a/src/common/Coverage.ts b/src/common/Coverage.ts index 4b2d5a046d7db..d9537dd0ddbff 100644 --- a/src/common/Coverage.ts +++ b/src/common/Coverage.ts @@ -36,10 +36,12 @@ export class Coverage { this._cssCoverage = new CSSCoverage(client); } - async startJSCoverage(options: { - resetOnNavigation?: boolean; - reportAnonymousScripts?: boolean; - }): Promise { + async startJSCoverage( + options: { + resetOnNavigation?: boolean; + reportAnonymousScripts?: boolean; + } = {} + ): Promise { return await this._jsCoverage.start(options); } @@ -47,9 +49,11 @@ export class Coverage { return await this._jsCoverage.stop(); } - async startCSSCoverage(options: { - resetOnNavigation?: boolean; - }): Promise { + async startCSSCoverage( + options: { + resetOnNavigation?: boolean; + } = {} + ): Promise { return await this._cssCoverage.start(options); } diff --git a/src/common/FrameManager.ts b/src/common/FrameManager.ts index 243c6c3da4d0d..6e56c75332952 100644 --- a/src/common/FrameManager.ts +++ b/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 { + async waitForNavigation( + options: { + timeout?: number; + waitUntil?: PuppeteerLifeCycleEvent | PuppeteerLifeCycleEvent[]; + } = {} + ): Promise { 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 { return this._secondaryWorld.click(selector, options); } @@ -584,7 +590,7 @@ export class Frame { async waitForSelector( selector: string, - options: WaitForSelectorOptions + options: WaitForSelectorOptions = {} ): Promise { const handle = await this._secondaryWorld.waitForSelector( selector, @@ -599,7 +605,7 @@ export class Frame { async waitForXPath( xpath: string, - options: WaitForSelectorOptions + options: WaitForSelectorOptions = {} ): Promise { const handle = await this._secondaryWorld.waitForXPath(xpath, options); if (!handle) return null; diff --git a/src/common/Input.ts b/src/common/Input.ts index 7a49b224b122a..e423808922722 100644 --- a/src/common/Input.ts +++ b/src/common/Input.ts @@ -122,7 +122,7 @@ export class Keyboard { return !!keyDefinitions[char]; } - async type(text: string, options: { delay?: number }): Promise { + async type(text: string, options: { delay?: number } = {}): Promise { const delay = (options && options.delay) || null; for (const char of text) { if (this.charIsKey(char)) { diff --git a/src/common/JSHandle.ts b/src/common/JSHandle.ts index cd88bb662a6ad..4273db864cd90 100644 --- a/src/common/JSHandle.ts +++ b/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 { + async click(options: ClickOptions = {}): Promise { await this._scrollIntoViewIfNeeded(); const { x, y } = await this._clickablePoint(); await this._page.mouse.click(x, y, options); diff --git a/src/common/Page.ts b/src/common/Page.ts index a8d184ea549c2..2ded9343f9f21 100644 --- a/src/common/Page.ts +++ b/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 = { 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 { + async setContent(html: string, options: WaitForOptions = {}): Promise { await this._frameManager.mainFrame().setContent(html, options); } @@ -913,11 +912,11 @@ export class Page extends EventEmitter { ); } - async goBack(options: WaitForOptions): Promise { + async goBack(options: WaitForOptions = {}): Promise { return this._go(-1, options); } - async goForward(options: WaitForOptions): Promise { + async goForward(options: WaitForOptions = {}): Promise { return this._go(+1, options); } @@ -1005,7 +1004,14 @@ export class Page extends EventEmitter { } async emulateVisionDeficiency(type?: VisionDeficiency): Promise { - 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; diff --git a/src/common/Puppeteer.ts b/src/common/Puppeteer.ts index 0a8a0a8c6ad87..e81ca0af9a145 100644 --- a/src/common/Puppeteer.ts +++ b/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); } diff --git a/src/index.ts b/src/index.ts index 3df618f8c3874..6adf9e0b36bf9 100644 --- a/src/index.ts +++ b/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; diff --git a/src/node/BrowserFetcher.ts b/src/node/BrowserFetcher.ts index b4082a771f8fc..4f879da6b0eef 100644 --- a/src/node/BrowserFetcher.ts +++ b/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 { const url = downloadURL( this._product, diff --git a/test/CDPSession.spec.js b/test/CDPSession.spec.ts similarity index 90% rename from test/CDPSession.spec.js rename to test/CDPSession.spec.ts index 43d0aad381e42..299364809dcd4 100644 --- a/test/CDPSession.spec.js +++ b/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'); } }); diff --git a/test/EventEmitter.spec.js b/test/EventEmitter.spec.ts similarity index 84% rename from test/EventEmitter.spec.js rename to test/EventEmitter.spec.ts index b6fcc91952433..9e1d227284f5c 100644 --- a/test/EventEmitter.spec.js +++ b/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); diff --git a/test/accessibility.spec.js b/test/accessibility.spec.ts similarity index 99% rename from test/accessibility.spec.js rename to test/accessibility.spec.ts index 5f223ecfe6393..618be6009126b 100644 --- a/test/accessibility.spec.js +++ b/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(); diff --git a/test/assets/input/textarea.html b/test/assets/input/textarea.html index 6d5be760740f0..6d77f3106d31e 100644 --- a/test/assets/input/textarea.html +++ b/test/assets/input/textarea.html @@ -7,9 +7,9 @@ - \ No newline at end of file + diff --git a/test/assets/shadow.html b/test/assets/shadow.html index 7242e673b562a..3796ca768c44d 100644 --- a/test/assets/shadow.html +++ b/test/assets/shadow.html @@ -1,8 +1,8 @@