diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index b07cc80928..5ea2f4e013 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -37,7 +37,7 @@ jobs: # XVFB-RUN merges both STDOUT and STDERR, whereas we need only STDERR # Wrap `npm run` in a subshell to redirect STDERR to file. # Enable core dumps in the subshell. - - run: xvfb-run --auto-servernum -- bash -c "ulimit -c unlimited && npm run jest -- --testTimeout=30000" + - run: xvfb-run --auto-servernum -- bash -c "ulimit -c unlimited && npm run jest -- --testTimeout=30000 && npm run coverage" env: BROWSER: ${{ matrix.browser }} DEBUG: "pw:*,-pw:wrapped*" diff --git a/package.json b/package.json index 4cd542a4c3..1c1bcb6b8b 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,8 @@ "generate-types": "node utils/generate_types/", "typecheck-tests": "tsc -p ./test/", "roll-browser": "node utils/roll_browser.js", - "jest": "jest" + "jest": "jest", + "coverage": "node test/jest/checkCoverage.js" }, "author": { "name": "Microsoft Corporation" diff --git a/test/jest/checkCoverage.js b/test/jest/checkCoverage.js new file mode 100644 index 0000000000..6cda377299 --- /dev/null +++ b/test/jest/checkCoverage.js @@ -0,0 +1,67 @@ +/** + * Copyright Microsoft Corporation. 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. + */ +const path = require('path'); +const fs = require('fs'); +const {installCoverageHooks} = require('./coverage'); + +const browserName = process.env.BROWSER || 'chromium'; + +const api = new Set(installCoverageHooks(browserName).coverage.keys()); + +// coverage exceptions + +if (browserName === 'chromium') { + // Sometimes we already have a background page while launching, before adding a listener. + api.delete('chromiumBrowserContext.emit("backgroundpage")'); +} + +if (browserName !== 'chromium') { + // we don't have CDPSession in non-chromium browsers + api.delete('cDPSession.send'); + api.delete('cDPSession.detach'); +} + +// Some permissions tests are disabled in webkit. See permissions.jest.js +if (browserName === 'webkit') + api.delete('browserContext.clearPermissions'); + +const coverageDir = path.join(__dirname, '..', 'output-' + browserName, 'coverage'); + +const coveredMethods = new Set(); +for (const file of getCoverageFiles(coverageDir)) { + for (const method of JSON.parse(fs.readFileSync(file, 'utf8'))) + coveredMethods.add(method); +} + + +let success = true; +for (const method of api) { + if (coveredMethods.has(method)) + continue; + success = false; + console.log(`ERROR: Missing coverage for "${method}"`) +} + +process.exit(success ? 0 : 1); + +function * getCoverageFiles(dir) { + for (const entry of fs.readdirSync(dir, {withFileTypes: true})) { + if (entry.isDirectory()) + yield * getCoverageFiles(path.join(dir, entry.name)) + else + yield path.join(dir, entry.name); + } +} \ No newline at end of file diff --git a/test/apicoverage.spec.js b/test/jest/coverage.js similarity index 58% rename from test/apicoverage.spec.js rename to test/jest/coverage.js index dc509fc41e..49862a776b 100644 --- a/test/apicoverage.spec.js +++ b/test/jest/coverage.js @@ -22,6 +22,7 @@ * @param {!Object} classType */ function traceAPICoverage(apiCoverage, events, className, classType) { + const uninstalls = []; className = className.substring(0, 1).toLowerCase() + className.substring(1); for (const methodName of Reflect.ownKeys(classType.prototype)) { const method = Reflect.get(classType.prototype, methodName); @@ -34,6 +35,7 @@ function traceAPICoverage(apiCoverage, events, className, classType) { }; Object.defineProperty(override, 'name', { writable: false, value: methodName }); Reflect.set(classType.prototype, methodName, override); + uninstalls.push(() => Reflect.set(classType.prototype, methodName, method)); } if (events[classType.name]) { @@ -47,62 +49,67 @@ function traceAPICoverage(apiCoverage, events, className, classType) { apiCoverage.set(`${className}.emit(${JSON.stringify(event)})`, true); return method.call(this, event, ...args); }); + uninstalls.push(() => Reflect.set(classType.prototype, 'emit', method)); } + + return () => uninstalls.forEach(u => u()); } -describe.skip(!process.env.COVERAGE)('**API COVERAGE**', () => { +/** + * @param {string} browserName + */ +function apiForBrowser(browserName) { const BROWSER_CONFIGS = [ { name: 'Firefox', - events: require('../lib/events').Events, - missingCoverage: ['cDPSession.send', 'cDPSession.detach'], + events: require('../../lib/events').Events, }, { name: 'WebKit', - events: require('../lib/events').Events, - missingCoverage: ['browserContext.clearPermissions', 'cDPSession.send', 'cDPSession.detach'], + events: require('../../lib/events').Events, }, { name: 'Chromium', events: { - ...require('../lib/events').Events, - ...require('../lib/chromium/events').Events, - }, - missingCoverage: [], + ...require('../../lib/events').Events, + ...require('../../lib/chromium/events').Events, + } }, ]; - const browserConfig = BROWSER_CONFIGS.find(config => config.name.toLowerCase() === browserType.name()); + const browserConfig = BROWSER_CONFIGS.find(config => config.name.toLowerCase() === browserName); const events = browserConfig.events; // TODO: we should rethink our api.ts approach to ensure coverage and async stacks. const api = { - ...require('../lib/api'), - Browser: require('../lib/browser').BrowserBase, - BrowserContext: require('../lib/browserContext').BrowserContextBase, + ...require('../../lib/api'), + Browser: require('../../lib/browser').BrowserBase, + BrowserContext: require('../../lib/browserContext').BrowserContextBase, }; - const coverage = new Map(); - Object.keys(api).forEach(apiName => { - if (BROWSER_CONFIGS.some(config => apiName.startsWith(config.name)) && !apiName.startsWith(browserConfig.name)) - return; - traceAPICoverage(coverage, events, apiName, api[apiName]); + const filteredKeys = Object.keys(api).filter(apiName => { + return !BROWSER_CONFIGS.some(config => apiName.startsWith(config.name)) || apiName.startsWith(browserConfig.name); }); + const filteredAPI = {}; + for (const key of filteredKeys) + filteredAPI[key] = api[key]; + return { + api: filteredAPI, + events + } +} - it('should call all API methods', () => { - const ignoredMethods = new Set(browserConfig.missingCoverage); - const missingMethods = []; - const extraIgnoredMethods = []; - for (const method of coverage.keys()) { - // Sometimes we already have a background page while launching, before adding a listener. - if (method === 'chromiumBrowserContext.emit("backgroundpage")') - continue; - if (!coverage.get(method) && !ignoredMethods.has(method)) - missingMethods.push(method); - else if (coverage.get(method) && ignoredMethods.has(method)) - extraIgnoredMethods.push(method); - } - if (extraIgnoredMethods.length) - throw new Error('Certain API Methods are called and should not be ignored: ' + extraIgnoredMethods.join(', ')); - if (missingMethods.length) - throw new Error('Certain API Methods are not called: ' + missingMethods.join(', ')); - }); -}); +/** + * @param {string} browserName + */ +function installCoverageHooks(browserName) { + const uninstalls = []; + const {api, events} = apiForBrowser(browserName); + const coverage = new Map(); + for (const [name, value] of Object.entries(api)) + uninstalls.push(traceAPICoverage(coverage, events, name, value)); + const uninstall = () => uninstalls.map(u => u()); + return {coverage, uninstall}; +} + +module.exports = { + installCoverageHooks +}; diff --git a/test/jest/playwrightEnvironment.js b/test/jest/playwrightEnvironment.js index 1f5dbd7ddc..92312ebe88 100644 --- a/test/jest/playwrightEnvironment.js +++ b/test/jest/playwrightEnvironment.js @@ -18,9 +18,10 @@ const NodeEnvironment = require('jest-environment-node'); const registerFixtures = require('./fixtures'); const os = require('os'); const path = require('path'); +const fs = require('fs'); const platform = os.platform(); const GoldenUtils = require('../../utils/testrunner/GoldenUtils'); - +const {installCoverageHooks} = require('./coverage'); const browserName = process.env.BROWSER || 'chromium'; class PlaywrightEnvironment extends NodeEnvironment { @@ -41,6 +42,7 @@ class PlaywrightEnvironment extends NodeEnvironment { testOptions.GOLDEN_DIR = path.join(__dirname, '..', 'golden-' + browserName); testOptions.OUTPUT_DIR = path.join(__dirname, '..', 'output-' + browserName); this.global.testOptions = testOptions; + this.testPath = context.testPath; this.global.registerFixture = (name, fn) => { this.fixturePool.registerFixture(name, 'test', fn); @@ -53,11 +55,27 @@ class PlaywrightEnvironment extends NodeEnvironment { async setup() { await super.setup(); + const {coverage, uninstall} = installCoverageHooks(browserName); + this.coverage = coverage; + this.uninstallCoverage = uninstall; } async teardown() { await this.fixturePool.teardownScope('worker'); await super.teardown(); + // If the setup throws an error, we don't want to override it + // with a useless error about this.coverage not existing. + if (!this.coverage) + return; + this.uninstallCoverage(); + const testRoot = path.join(__dirname, '..'); + const relativeTestPath = path.relative(testRoot, this.testPath); + const coveragePath = path.join(this.global.testOptions.OUTPUT_DIR, 'coverage', relativeTestPath + '.json'); + const coverageJSON = [...this.coverage.keys()].filter(key => this.coverage.get(key)); + await fs.promises.mkdir(path.dirname(coveragePath), { recursive: true }); + await fs.promises.writeFile(coveragePath, JSON.stringify(coverageJSON, undefined, 2), 'utf8'); + delete this.coverage; + delete this.uninstallCoverage; } runScript(script) { diff --git a/test/test.config.js b/test/test.config.js index 70853d5705..55e9e57d30 100644 --- a/test/test.config.js +++ b/test/test.config.js @@ -76,7 +76,6 @@ module.exports = { }, { files: [ - './apicoverage.spec.js', ], environments: [], },