chore: cleanup our build system (#4903)

- Consolidate our build and watch to a single build.js file. - Update contributing docs. - Remove unused scripts and package.json script entries.
2024-11-28 09:23:42 +03:00 · 2021-01-06 12:41:17 -08:00 · 2021-01-06 12:41:17 -08:00 · 4ff7e1a419
commit 4ff7e1a419
parent 2311c282d6
11 changed files with 149 additions and 288 deletions
--- a/.gitattributes
+++ b/.gitattributes
@ -1,3 +1,5 @@
 # text files must be lf for golden file tests to work
 *.txt eol=lf
 *.json eol=lf
+*.md eol=lf
+*.yml eol=lf
--- a/.github/workflows/tests.yml
+++ b/.github/workflows/tests.yml
@ -38,7 +38,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 --server-args="-screen 0 1280x960x24" -- bash -c "ulimit -c unlimited && npx folio test/ --workers=1 --forbid-only --global-timeout=5400000 --retries=3 --reporter=dot,json && npm run coverage"
+    - run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" -- bash -c "ulimit -c unlimited && npx folio test/ --workers=1 --forbid-only --global-timeout=5400000 --retries=3 --reporter=dot,json && node test/checkCoverage.js"
      env:
        BROWSER: ${{ matrix.browser }}
        FOLIO_JSON_OUTPUT_NAME: "test-results/report.json"
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@ -106,7 +106,7 @@ Fixes #123, fixes #234

 ### Writing Documentation

-All public API should have a descriptive entry in [`docs/api.md`](https://github.com/microsoft/playwright/blob/master/docs/api.md). There's a [documentation linter](https://github.com/microsoft/playwright/tree/master/utils/doclint) which makes sure documentation is aligned with the codebase.
+All API classes, methods and events should have description in [`docs/src`](https://github.com/microsoft/playwright/blob/master/docs/src). There's a [documentation linter](https://github.com/microsoft/playwright/tree/master/utils/doclint) which makes sure documentation is aligned with the codebase.

 To run the documentation linter, use:

@ -130,8 +130,7 @@ A barrier for introducing new installation dependencies is especially high:
 - Tests should be *hermetic*. Tests should not depend on external services.
 - Tests should work on all three platforms: Mac, Linux and Win. This is especially important for screenshot tests.

-Playwright tests are located in [`test/test.js`](https://github.com/microsoft/playwright/blob/master/test/test.js)
-and are written with a [TestRunner](https://github.com/microsoft/playwright/tree/master/utils/testrunner) framework.
+Playwright tests are located in [`test`](https://github.com/microsoft/playwright/blob/master/test) and use [Folio](https://github.com/microsoft/folio) test runner.
 These are integration tests, making sure public API methods and events work as expected.

 - To run all tests:
@ -145,36 +144,23 @@ npm run test
 npm run ctest # also `ftest` for firefox and `wtest` for WebKit
 ```

- To run tests in parallel, use `-j` flag:
-
-```bash
-npm run wtest -- -j 4
-```
-
- To run tests in "verbose" mode or to stop testrunner on first failure:
-
-```bash
-npm run ftest -- --verbose
-npm run ftest -- --break-on-failure
-```
-
- To run a specific test, substitute the `it` with `fit` (mnemonic rule: '*focus it*'):
+- To run a specific test, substitute `it` with `it.only`:

 ```js
 ...
-// Using "fit" to run specific test
-fit('should work', async ({server, page}) => {
+// Using "it.only" to run a specific test
+it.only('should work', async ({server, page}) => {
  const response = await page.goto(server.EMPTY_PAGE);
  expect(response.ok).toBe(true);
 });
 ```

- To disable a specific test, substitute the `it` with `xit` (mnemonic rule: '*cross it*'):
+- To disable a specific test, substitute `it` with `it.skip`:

 ```js
 ...
-// Using "xit" to skip specific test
-xit('should work', async ({server, page}) => {
+// Using "it.skip" to skip a specific test
+it.skip('should work', async ({server, page}) => {
  const response = await page.goto(server.EMPTY_PAGE);
  expect(response.ok).toBe(true);
 });
@ -198,12 +184,6 @@ CRPATH=<path-to-executable> npm run ctest
 HEADLESS=false SLOW_MO=500 npm run wtest
 ```

- To debug a test, "focus" a test first and then run:
-
-```bash
-BROWSER=chromium node --inspect-brk test/test.js
-```
-
 - When should a test be marked with `skip` or `fail`?

  - **`skip(condition)`**: This test *should ***never*** work* for `condition`
@ -224,18 +204,6 @@ BROWSER=chromium node --inspect-brk test/test.js
    currently diverges from what a user would experience driving a Chromium or
    WebKit.

-### Public API Coverage
-
-Every public API method or event should be called at least once in tests. To ensure this, there's a `coverage` command which tracks calls to public API and reports back if some methods/events were not called.
-
-Run all tests for all browsers with coverage enabled:
-
-```bash
-npm run coverage
-```
-
-There are also per-browser commands:" `npm run ccoverage`, `npm run fcoverage` and `npm run wcoverage`.
-
 ## Contributor License Agreement

 This project welcomes contributions and suggestions.  Most contributions require you to agree to a
--- a/package.json
+++ b/package.json
@ -18,16 +18,13 @@
    "tsc": "tsc -p .",
    "tsc-installer": "tsc -p ./src/install/tsconfig.json",
    "doc": "node utils/doclint/cli.js",
-    "lint": "npm run eslint && npm run tsc && npm run doc && npm run check-deps && npm run generate-channels && node utils/generate_types/ --check-clean && npm run test-types && folio utils/doclint/test/",
+    "lint": "npm run eslint && npm run tsc && npm run doc && npm run check-deps && node utils/generate_channels.js && node utils/generate_types/ --check-clean && npm run test-types && folio utils/doclint/test/",
    "clean": "rimraf lib",
    "prepare": "node install-from-github.js",
-    "build": "node utils/runWebpack.js --mode='development' && tsc -p . && npm run generate-api-json",
-    "watch": "node utils/watch.js",
+    "build": "node utils/build/build.js",
+    "watch": "node utils/build/build.js --watch",
    "test-types": "node utils/generate_types/ && npx -p typescript@3.7.5 tsc -p utils/generate_types/test/tsconfig.json && tsc -p ./test/",
-    "generate-channels": "node utils/generate_channels.js",
-    "generate-api-json": "node utils/doclint/generateApiJson.js",
    "roll-browser": "node utils/roll_browser.js",
-    "coverage": "node test/checkCoverage.js",
    "check-deps": "node utils/check_deps.js",
    "build-android-driver": "./utils/build_android_driver.sh"
  },
--- a/utils/ESTreeWalker.js
+++ b/utils/ESTreeWalker.js
@ -1,136 +0,0 @@
-// Copyright (c) 2014 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-/**
- * @unrestricted
- */
-class ESTreeWalker {
-  /**
-   * @param {function(!ESTree.Node):(!Object|undefined)} beforeVisit
-   * @param {function(!ESTree.Node)=} afterVisit
-   */
-  constructor(beforeVisit, afterVisit) {
-    this._beforeVisit = beforeVisit;
-    this._afterVisit = afterVisit || new Function();
-  }
-
-  /**
-   * @param {!ESTree.Node} ast
-   */
-  walk(ast) {
-    this._innerWalk(ast, null);
-  }
-
-  /**
-   * @param {!ESTree.Node} node
-   * @param {?ESTree.Node} parent
-   */
-  _innerWalk(node, parent) {
-    if (!node)
-      return;
-    node.parent = parent;
-
-    if (this._beforeVisit.call(null, node) === ESTreeWalker.SkipSubtree) {
-      this._afterVisit.call(null, node);
-      return;
-    }
-
-    const walkOrder = ESTreeWalker._walkOrder[node.type];
-    if (!walkOrder)
-      return;
-
-    if (node.type === 'TemplateLiteral') {
-      const templateLiteral = /** @type {!ESTree.TemplateLiteralNode} */ (node);
-      const expressionsLength = templateLiteral.expressions.length;
-      for (let i = 0; i < expressionsLength; ++i) {
-        this._innerWalk(templateLiteral.quasis[i], templateLiteral);
-        this._innerWalk(templateLiteral.expressions[i], templateLiteral);
-      }
-      this._innerWalk(templateLiteral.quasis[expressionsLength], templateLiteral);
-    } else {
-      for (let i = 0; i < walkOrder.length; ++i) {
-        const entity = node[walkOrder[i]];
-        if (Array.isArray(entity))
-          this._walkArray(entity, node);
-        else
-          this._innerWalk(entity, node);
-      }
-    }
-
-    this._afterVisit.call(null, node);
-  }
-
-  /**
-   * @param {!Array.<!ESTree.Node>} nodeArray
-   * @param {?ESTree.Node} parentNode
-   */
-  _walkArray(nodeArray, parentNode) {
-    for (let i = 0; i < nodeArray.length; ++i)
-      this._innerWalk(nodeArray[i], parentNode);
-  }
-}
-
-/** @typedef {!Object} ESTreeWalker.SkipSubtree */
-ESTreeWalker.SkipSubtree = {};
-
-/** @enum {!Array.<string>} */
-ESTreeWalker._walkOrder = {
-  'AwaitExpression': ['argument'],
-  'ArrayExpression': ['elements'],
-  'ArrowFunctionExpression': ['params', 'body'],
-  'AssignmentExpression': ['left', 'right'],
-  'AssignmentPattern': ['left', 'right'],
-  'BinaryExpression': ['left', 'right'],
-  'BlockStatement': ['body'],
-  'BreakStatement': ['label'],
-  'CallExpression': ['callee', 'arguments'],
-  'CatchClause': ['param', 'body'],
-  'ClassBody': ['body'],
-  'ClassDeclaration': ['id', 'superClass', 'body'],
-  'ClassExpression': ['id', 'superClass', 'body'],
-  'ConditionalExpression': ['test', 'consequent', 'alternate'],
-  'ContinueStatement': ['label'],
-  'DebuggerStatement': [],
-  'DoWhileStatement': ['body', 'test'],
-  'EmptyStatement': [],
-  'ExpressionStatement': ['expression'],
-  'ForInStatement': ['left', 'right', 'body'],
-  'ForOfStatement': ['left', 'right', 'body'],
-  'ForStatement': ['init', 'test', 'update', 'body'],
-  'FunctionDeclaration': ['id', 'params', 'body'],
-  'FunctionExpression': ['id', 'params', 'body'],
-  'Identifier': [],
-  'IfStatement': ['test', 'consequent', 'alternate'],
-  'LabeledStatement': ['label', 'body'],
-  'Literal': [],
-  'LogicalExpression': ['left', 'right'],
-  'MemberExpression': ['object', 'property'],
-  'MethodDefinition': ['key', 'value'],
-  'NewExpression': ['callee', 'arguments'],
-  'ObjectExpression': ['properties'],
-  'ObjectPattern': ['properties'],
-  'ParenthesizedExpression': ['expression'],
-  'Program': ['body'],
-  'Property': ['key', 'value'],
-  'ReturnStatement': ['argument'],
-  'SequenceExpression': ['expressions'],
-  'Super': [],
-  'SwitchCase': ['test', 'consequent'],
-  'SwitchStatement': ['discriminant', 'cases'],
-  'TaggedTemplateExpression': ['tag', 'quasi'],
-  'TemplateElement': [],
-  'TemplateLiteral': ['quasis', 'expressions'],
-  'ThisExpression': [],
-  'ThrowStatement': ['argument'],
-  'TryStatement': ['block', 'handler', 'finalizer'],
-  'UnaryExpression': ['argument'],
-  'UpdateExpression': ['argument'],
-  'VariableDeclaration': ['declarations'],
-  'VariableDeclarator': ['id', 'init'],
-  'WhileStatement': ['test', 'body'],
-  'WithStatement': ['object', 'body'],
-  'YieldExpression': ['argument']
-};
-
-module.exports = ESTreeWalker;
--- a/utils/build/build.js
+++ b/utils/build/build.js
@ -0,0 +1,125 @@
+/**
+ * Copyright (c) Microsoft Corporation.
+ *
+ * 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 child_process = require('child_process');
+const path = require('path');
+const fs = require('fs');
+
+const steps = [];
+const onChanges = [];
+
+const watchMode = process.argv.slice(2).includes('--watch');
+const ROOT = path.join(__dirname, '..', '..');
+
+function filePath(relative) {
+  return path.join(ROOT, ...relative.split('/'));
+}
+
+function runWatch() {
+  function runOnChanges(paths, nodeFile) {
+    for (const p of [...paths, nodeFile]) {
+      const file = filePath(p);
+      if (!fs.existsSync(file)) {
+        console.error('could not find file', file);
+        process.exit(1);
+      }
+      fs.watchFile(file, callback);
+    }
+    callback();
+    function callback() {
+      child_process.spawnSync('node', [filePath(nodeFile)], { stdio: 'inherit' });
+    }
+  }
+
+  const spawns = [];
+  for (const step of steps)
+    spawns.push(child_process.spawn(step.command, step.args, { stdio: 'inherit', shell: step.shell }));
+  process.on('exit', () => spawns.forEach(s => s.kill()));
+  for (const onChange of onChanges)
+    runOnChanges(onChange.inputs, onChange.script);
+}
+
+function runBuild() {
+  function runStep(command, args, shell) {
+    const out = child_process.spawnSync(command, args, { stdio: 'inherit', shell });
+    if (out.status)
+      process.exit(out.status);
+  }
+
+  for (const step of steps)
+    runStep(step.command, step.args, step.shell);
+  for (const onChange of onChanges) {
+    if (!onChange.committed)
+      runStep('node', [filePath(onChange.script)], false);
+  }
+}
+
+// Build injected scripts.
+const webPackFiles = [
+  'src/server/injected/injectedScript.webpack.config.js',
+  'src/server/injected/utilityScript.webpack.config.js',
+  'src/debug/injected/consoleApi.webpack.config.js',
+  'src/cli/injected/recorder.webpack.config.js',
+];
+for (const file of webPackFiles) {
+  steps.push({
+    command: 'npx',
+    args: ['webpack', '--config', filePath(file), '--mode', 'development', ...(watchMode ? ['--watch', '--silent'] : [])],
+    shell: true,
+  });
+}
+
+// Run typescript.
+steps.push({
+  command: 'npx',
+  args: ['tsc', ...(watchMode ? ['-w', '--preserveWatchOutput'] : []), '-p', filePath('.')],
+  shell: true,
+});
+
+// Generate api.json.
+onChanges.push({
+  committed: false,
+  inputs: [
+    'docs/src/api-body.md',
+    'docs/src/api-params.md',
+  ],
+  script: 'utils/doclint/generateApiJson.js',
+});
+
+// Generate channels.
+onChanges.push({
+  committed: false,
+  inputs: [
+    'src/protocol/protocol.yml'
+  ],
+  script: 'utils/generate_channels.js',
+});
+
+// Generate types.
+onChanges.push({
+  committed: false,
+  inputs: [
+    'docs/src/api-body.md',
+    'docs/src/api-params.md',
+    'utils/generate_types/overrides.d.ts',
+    'utils/generate_types/exported.json',
+    'src/server/chromium/protocol.ts',
+    'src/trace/traceTypes.ts',
+  ],
+  script: 'utils/generate_types/index.js',
+});
+
+watchMode ? runWatch() : runBuild();
--- a/utils/generate_channels.js
+++ b/utils/generate_channels.js
@ -16,6 +16,7 @@
 */

 const fs = require('fs');
+const os = require('os');
 const path = require('path');
 const yaml = require('yaml');

@ -110,7 +111,7 @@ const channels_ts = [
 * limitations under the License.
 */

-// This file is generated by ${path.basename(__filename)}, do not edit manually.
+// This file is generated by ${path.basename(__filename).split(path.sep).join(path.posix.sep)}, do not edit manually.

 import { EventEmitter } from 'events';

@ -234,6 +235,8 @@ validator_ts.push(`
 let hasChanges = false;

 function writeFile(filePath, content) {
+  if (os.platform() === 'win32')
+    content = content.replace(/\r\n/g, '\n').replace(/\n/g, '\r\n');
  const existing = fs.readFileSync(filePath, 'utf8');
  if (existing === content)
    return;
--- a/utils/generate_types/index.js
+++ b/utils/generate_types/index.js
@ -16,6 +16,7 @@

 //@ts-check
 const path = require('path');
+const os = require('os');
 const {devices} = require('../..');
 const Documentation = require('../doclint/Documentation');
 const PROJECT_DIR = path.join(__dirname, '..', '..');
@ -108,6 +109,8 @@ ${generateDevicesTypes()}
 });

 function writeFile(filePath, content) {
+  if (os.platform() === 'win32')
+    content = content.replace(/\r\n/g, '\n').replace(/\n/g, '\r\n');
  const existing = fs.readFileSync(filePath, 'utf8');
  if (existing === content)
    return;
@ -260,7 +263,7 @@ function parentClass(classDesc) {

 function writeComment(comment, indent = '') {
  const parts = [];
-  
+
  comment = comment.replace(/\[`([^`]+)`\]\(#([^\)]+)\)/g, '[$1](https://github.com/microsoft/playwright/blob/master/docs/api.md#$2)');
  comment = comment.replace(/\[([^\]]+)\]\(#([^\)]+)\)/g, '[$1](https://github.com/microsoft/playwright/blob/master/docs/api.md#$2)');
  comment = comment.replace(/\[`([^`]+)`\]\(\.\/([^\)]+)\)/g, '[$1](https://github.com/microsoft/playwright/blob/master/docs/$2)');
--- a/utils/markdown.js
+++ b/utils/markdown.js
@ -26,7 +26,7 @@
 *  }} MarkdownNode */

 function flattenWrappedLines(content) {
-  const inLines = content.replace(/\r\n/g, '\n').split('\n');
+  const inLines = content.split('\n');
  let inCodeBlock = false;
  const outLines = [];
  let outLineTokens = [];
@ -159,7 +159,7 @@ function buildTree(lines) {
        node.liType = 'ordinal';
      else if (content.startsWith('*'))
        node.liType = 'bullet';
-      else 
+      else
        node.liType = 'default';
    }
    appendNode(indent, node);
--- a/utils/runWebpack.js
+++ b/utils/runWebpack.js
@ -1,41 +0,0 @@
-/**
- * Copyright (c) Microsoft Corporation.
- *
- * 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 child_process = require('child_process');
-const path = require('path');
-
-const files = [
-  path.join('src', 'server', 'injected', 'injectedScript.webpack.config.js'),
-  path.join('src', 'server', 'injected', 'utilityScript.webpack.config.js'),
-  path.join('src', 'debug', 'injected', 'consoleApi.webpack.config.js'),
-  path.join('src', 'cli', 'injected', 'recorder.webpack.config.js'),
-];
-
-function runOne(runner, file) {
-  return runner('npx', ['webpack', '--config', file, ...process.argv.slice(2)], { stdio: 'inherit', shell: true });
-}
-
-const args = process.argv.slice(2);
-if (args.includes('--watch')) {
-  const spawns = files.map(file => runOne(child_process.spawn, file));
-  process.on('exit', () => spawns.forEach(s => s.kill()));
-} else {
-  for (const file of files) {
-    const out = runOne(child_process.spawnSync, file);
-    if (out.status)
-      process.exit(out.status);
-  }
-}
--- a/utils/watch.js
+++ b/utils/watch.js
@ -1,60 +0,0 @@
-/**
- * Copyright (c) Microsoft Corporation.
- *
- * 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 child_process = require('child_process');
-const path = require('path');
-const fs = require('fs');
-
-const spawns = [
-  child_process.spawn('node', [path.join(__dirname, 'runWebpack.js'), '--mode="development"', '--watch', '--silent'], { stdio: 'inherit', shell: true }),
-  child_process.spawn('npx', ['tsc', '-w', '--preserveWatchOutput', '-p', path.join(__dirname, '..')], { stdio: 'inherit', shell: true }),
-];
-process.on('exit', () => spawns.forEach(s => s.kill()));
-
-runOnChanges(['src/protocol/protocol.yml'], 'utils/generate_channels.js');
-runOnChanges([
-  'docs/src/api-body.md',
-  'docs/src/api-params.md',
-  'utils/generate_types/overrides.d.ts',
-  'utils/generate_types/exported.json',
-  'src/server/chromium/protocol.ts',
-  'src/trace/traceTypes.ts',
-], 'utils/generate_types/index.js');
-runOnChanges([
-  'docs/src/api-body.md',
-  'docs/src/api-params.md',
-], 'utils/doclint/generateApiJson.js');
-
-/**
- * @param {string[][]} paths
- * @param {string} nodeFile
- */
-function runOnChanges(paths, nodeFile) {
-  for (const p of [...paths, nodeFile]) {
-    const filePath = path.join(__dirname, '..', ...p.split('/'));
-    if (!fs.existsSync(filePath)) {
-      console.error('could not find file', filePath);
-      process.exit(1);
-    }
-    fs.watchFile(filePath, callback);
-  }
-
-  callback();
-
-  function callback() {
-    child_process.spawnSync('node', [path.join(__dirname, '..', ...nodeFile.split('/'))]);
-  }
-}