From c35084dd2c0ef73150b75725b38c812a9e61e66e Mon Sep 17 00:00:00 2001 From: Alex Rudenko Date: Mon, 26 Jun 2023 10:57:48 +0200 Subject: [PATCH] ci: separate unit tests (#10436) --- .eslintrc.js | 2 +- .github/workflows/ci.yml | 21 +++++++++++++ docs/contributing.md | 10 +++++++ package.json | 5 ++-- packages/puppeteer-core/package.json | 10 ++++++- .../src/common/NetworkManager.test.ts | 30 +++++++++++-------- packages/puppeteer/package.json | 1 + test/TestExpectations.json | 18 ++++------- 8 files changed, 68 insertions(+), 29 deletions(-) rename test/src/NetworkManager.spec.ts => packages/puppeteer-core/src/common/NetworkManager.test.ts (98%) diff --git a/.eslintrc.js b/.eslintrc.js index 549d58ae..26b64263 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -98,7 +98,7 @@ module.exports = { 'no-restricted-imports': [ 'error', { - patterns: ['*Events'], + patterns: ['*Events', '*.test.js'], paths: [ { name: 'mitt', diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ba51432b..3e7ce837 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -372,6 +372,27 @@ jobs: run: | docker run -i --init --cap-add=SYS_ADMIN --rm puppeteer-test-image node -e "`cat test/smoke-test.js`" + unit-tests: + name: '[Required] Unit tests' + runs-on: ubuntu-latest + needs: check-changes + if: ${{ contains(fromJSON(needs.check-changes.outputs.changes), 'puppeteer') }} + steps: + - name: Check out repository + uses: actions/checkout@8e5e7e5ab8b370d6c329ec480221332ada57f0ab # v3.5.2 + - name: Set up Node.js + uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0 + with: + cache: npm + node-version: lts/* + - name: Install dependencies + run: npm ci + env: + PUPPETEER_SKIP_DOWNLOAD: true + - name: Run unit tests + run: | + npm run unit --w puppeteer-core -w puppeteer --if-present + ng-schematics-tests: name: '[Required] Test Angular Schematics' runs-on: ubuntu-latest diff --git a/docs/contributing.md b/docs/contributing.md index f5675d9d..4bddec35 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -98,6 +98,7 @@ usually check through CI: [`tsd`](https://github.com/SamVerschueren/tsd). - `test:chrome:**` - Tests `puppeteer` on Chromium. - `test:firefox:**` - Tests `puppeteer` on Firefox. +- `unit` - Runs unit tests. The default `npm test` runs `test:{chrome,firefox}:headless` which is generally sufficient. @@ -108,6 +109,15 @@ to see if a given test result is expected or not. See more info about the test runner in [`tools/mochaRunner`](https://github.com/puppeteer/puppeteer/tree/main/tools/mochaRunner). +### Unit tests + +Tests that only test code (without the running browser) are put next to the classes they test +and run using the Node test runner (requires Node 20+): + +```bash +npm run unit +``` + ## Code reviews All submissions, including submissions by project members, require review. We diff --git a/package.json b/package.json index 644afb8f..568bb8cc 100644 --- a/package.json +++ b/package.json @@ -34,8 +34,9 @@ "test:firefox:headful": "wireit", "test:firefox:headless": "wireit", "test:firefox": "wireit", - "test": "cross-env PUPPETEER_DEFERRED_PROMISE_DEBUG_TIMEOUT=20000 node tools/mochaRunner/lib/main.js --min-tests 1052", - "validate-licenses": "tsx tools/third_party/validate-licenses.ts" + "test": "cross-env PUPPETEER_DEFERRED_PROMISE_DEBUG_TIMEOUT=20000 node tools/mochaRunner/lib/main.js --min-tests 1048", + "validate-licenses": "tsx tools/third_party/validate-licenses.ts", + "unit": "npm run unit --workspaces --if-present" }, "wireit": { "build": { diff --git a/packages/puppeteer-core/package.json b/packages/puppeteer-core/package.json index 03bbd710..1cdba68b 100644 --- a/packages/puppeteer-core/package.json +++ b/packages/puppeteer-core/package.json @@ -42,7 +42,8 @@ "clean": "tsc -b --clean && rm -rf lib src/generated", "generate:package-json": "wireit", "generate:sources": "wireit", - "prepack": "wireit" + "prepack": "wireit", + "unit": "wireit" }, "wireit": { "prepack": { @@ -123,11 +124,18 @@ "dependencies": [ "build:tsc" ] + }, + "unit": { + "command": "node --test --test-reporter spec lib/cjs", + "dependencies": [ + "build" + ] } }, "files": [ "lib", "src", + "!*.test.js", "!*.tsbuildinfo" ], "author": "The Chromium Authors", diff --git a/test/src/NetworkManager.spec.ts b/packages/puppeteer-core/src/common/NetworkManager.test.ts similarity index 98% rename from test/src/NetworkManager.spec.ts rename to packages/puppeteer-core/src/common/NetworkManager.test.ts index db73f6e6..8a098f87 100644 --- a/test/src/NetworkManager.spec.ts +++ b/packages/puppeteer-core/src/common/NetworkManager.test.ts @@ -14,15 +14,16 @@ * limitations under the License. */ +import {describe, it} from 'node:test'; + import expect from 'expect'; -import {HTTPRequest} from 'puppeteer-core/internal/api/HTTPRequest.js'; -import {HTTPResponse} from 'puppeteer-core/internal/api/HTTPResponse.js'; -import {EventEmitter} from 'puppeteer-core/internal/common/EventEmitter.js'; -import {Frame} from 'puppeteer-core/internal/common/Frame.js'; -import { - NetworkManager, - NetworkManagerEmittedEvents, -} from 'puppeteer-core/internal/common/NetworkManager.js'; + +import {HTTPRequest} from '../api/HTTPRequest.js'; +import {HTTPResponse} from '../api/HTTPResponse.js'; + +import {EventEmitter} from './EventEmitter.js'; +import {Frame} from './Frame.js'; +import {NetworkManager, NetworkManagerEmittedEvents} from './NetworkManager.js'; // TODO: develop a helper to generate fake network events for attributes that // are not relevant for the network manager to make tests shorter. @@ -477,13 +478,16 @@ describe('NetworkManager', () => { return null; }, }); - manager.setRequestInterception(true); + await manager.setRequestInterception(true); const requests: HTTPRequest[] = []; - manager.on(NetworkManagerEmittedEvents.Request, (request: HTTPRequest) => { - request.continue(); - requests.push(request); - }); + manager.on( + NetworkManagerEmittedEvents.Request, + async (request: HTTPRequest) => { + requests.push(request); + await request.continue(); + } + ); /** * This sequence was taken from an actual CDP session produced by the following diff --git a/packages/puppeteer/package.json b/packages/puppeteer/package.json index 1816b1b3..302c4612 100644 --- a/packages/puppeteer/package.json +++ b/packages/puppeteer/package.json @@ -115,6 +115,7 @@ "lib", "src", "install.js", + "!*.test.js", "!*.tsbuildinfo" ], "author": "The Chromium Authors", diff --git a/test/TestExpectations.json b/test/TestExpectations.json index e1bae865..65b0368b 100644 --- a/test/TestExpectations.json +++ b/test/TestExpectations.json @@ -11,6 +11,12 @@ "parameters": ["webDriverBiDi"], "expectations": ["PASS"] }, + { + "testIdPattern": "[Deferred.spec] *", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["webDriverBiDi"], + "expectations": ["PASS"] + }, { "testIdPattern": "[DeviceRequestPrompt.spec] DeviceRequestPrompt *", "platforms": ["darwin", "linux", "win32"], @@ -155,12 +161,6 @@ "parameters": ["webDriverBiDi"], "expectations": ["FAIL", "TIMEOUT"] }, - { - "testIdPattern": "[NetworkManager.spec] NetworkManager *", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["webDriverBiDi"], - "expectations": ["PASS"] - }, { "testIdPattern": "[page.spec] Page Page.browser *", "platforms": ["darwin", "linux", "win32"], @@ -473,12 +473,6 @@ "parameters": ["chrome", "webDriverBiDi"], "expectations": ["PASS"] }, - { - "testIdPattern": "[Deferred.spec] *", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["webDriverBiDi"], - "expectations": ["PASS"] - }, { "testIdPattern": "[drag-and-drop.spec] *", "platforms": ["darwin", "linux", "win32"],