feat(testrunner): add new testrunner options (#4718)

This patch adds new TestRunner options:
- `disableTimeoutWhenInspectorIsEnabled` - disable test timeout if
  testrunner detects enabled inspector.
- `breakOnFailure` - if testrunner should terminate test running on
  first test failure

CONTRIBUTING.md

@@ -1,14 +1,14 @@
 <!-- gen:toc -->
 - [How to Contribute](#how-to-contribute)
   * [Contributor License Agreement](#contributor-license-agreement)
-  * [Getting setup](#getting-setup)
+  * [Getting Code](#getting-code)
   * [Code reviews](#code-reviews)
   * [Code Style](#code-style)
   * [API guidelines](#api-guidelines)
   * [Commit Messages](#commit-messages)
   * [Writing Documentation](#writing-documentation)
   * [Adding New Dependencies](#adding-new-dependencies)
-  * [Writing Tests](#writing-tests)
+  * [Running & Writing Tests](#running--writing-tests)
   * [Public API Coverage](#public-api-coverage)
   * [Debugging Puppeteer](#debugging-puppeteer)
 - [For Project Maintainers](#for-project-maintainers)
@@ -33,7 +33,7 @@ You generally only need to submit a CLA once, so if you've already submitted one
 (even if it was for a different project), you probably don't need to do it
 again.
 
-## Getting setup
+## Getting Code
 
 1. Clone this repository
 
@@ -48,6 +48,12 @@ cd puppeteer
 npm install
 ```
 
+3. Run Puppeteer tests locally. For more information about tests, read [Running & Writing Tests](#running--writing-tests).
+
+```bash
+npm run unit
+```
+
 ## Code reviews
 
 All submissions, including submissions by project members, require review. We
@@ -134,7 +140,7 @@ For all dependencies (both installation and development):
 A barrier for introducing new installation dependencies is especially high:
 - **Do not add** installation dependency unless it's critical to project success.
 
-## Writing Tests
+## Running & Writing Tests
 
 - Every feature should be accompanied by a test.
 - Every public api event/method should be accompanied by a test.
@@ -157,6 +163,13 @@ npm run unit
 npm run unit -- -j 4
 ```
 
+- To run tests in "verbose" mode or to stop testrunner on first failure:
+
+```bash
+npm run unit -- --verbose
+npm run unit -- --break-on-failure
+```
+
 - To run a specific test, substitute the `it` with `fit` (mnemonic rule: '*focus it*'):
 
 ```js

test/test.js

@@ -30,7 +30,11 @@ require('events').defaultMaxListeners *= parallel;
 let timeout = process.env.APPVEYOR ? 20 * 1000 : 10 * 1000;
 if (!isNaN(process.env.TIMEOUT))
   timeout = parseInt(process.env.TIMEOUT, 10);
-const testRunner = new TestRunner({timeout, parallel});
+const testRunner = new TestRunner({
+  timeout,
+  parallel,
+  breakOnFailure: process.argv.indexOf('--break-on-failure') !== -1,
+});
 const {describe, fdescribe, beforeAll, afterAll, beforeEach, afterEach} = testRunner;
 
 console.log('Testing on Node', process.version);

utils/testrunner/TestRunner.js

@@ -135,10 +135,11 @@ class Suite {
 }
 
 class TestPass {
-  constructor(runner, rootSuite, tests, parallel) {
+  constructor(runner, rootSuite, tests, parallel, breakOnFailure) {
     this._runner = runner;
     this._parallel = parallel;
     this._runningUserCallbacks = new Multimap();
+    this._breakOnFailure = breakOnFailure;
 
     this._rootSuite = rootSuite;
     this._workerDistribution = new Multimap();
@@ -236,6 +237,8 @@ class TestPass {
     else
       test.result = TestResult.Failed;
     this._runner._didFinishTest(test, workerId);
+    if (this._breakOnFailure && test.result !== TestResult.Ok)
+      this._terminate(`Terminating because a test has failed and |testRunner.breakOnFailure| is enabled`, null);
   }
 
   async _runHook(workerId, suite, hookName, ...args) {
@@ -268,22 +271,27 @@ class TestPass {
 class TestRunner extends EventEmitter {
   constructor(options = {}) {
     super();
+    const {
+      timeout = 10 * 1000, // Default timeout is 10 seconds.
+      parallel = 1,
+      breakOnFailure = false,
+      disableTimeoutWhenInspectorIsEnabled = true,
+    } = options;
     this._rootSuite = new Suite(null, '', TestMode.Run);
     this._currentSuite = this._rootSuite;
     this._tests = [];
-    // Default timeout is 10 seconds.
+    this._timeout = timeout === 0 ? 2147483647 : timeout;
-    this._timeout = options.timeout === 0 ? 2147483647 : options.timeout || 10 * 1000;
+    this._parallel = parallel;
-    this._parallel = options.parallel || 1;
+    this._breakOnFailure = breakOnFailure;
 
     this._hasFocusedTestsOrSuites = false;
 
-    if (MAJOR_NODEJS_VERSION >= 8) {
+    if (MAJOR_NODEJS_VERSION >= 8 && disableTimeoutWhenInspectorIsEnabled) {
       const inspector = require('inspector');
       if (inspector.url()) {
         console.log('TestRunner detected inspector; overriding certain properties to be debugger-friendly');
         console.log('  - timeout = 0 (Infinite)');
         this._timeout = 2147483647;
-        this._parallel = 1;
       }
     }
 
@@ -338,7 +346,7 @@ class TestRunner extends EventEmitter {
   async run() {
     const runnableTests = this._runnableTests();
     this.emit(TestRunner.Events.Started, runnableTests);
-    const pass = new TestPass(this, this._rootSuite, runnableTests, this._parallel);
+    const pass = new TestPass(this, this._rootSuite, runnableTests, this._parallel, this._breakOnFailure);
     const termination = await pass.run();
     if (termination)
       this.emit(TestRunner.Events.Terminated, termination.message, termination.error);