feat(testrunner): improvements to testrunner (#2070)

This patch:
- teaches test runner to understand custom argument spelling, e.g. `--file=evalu` and `-j10`
- fixes `--file` filter to actually focus file paths instead of focusing
all tests with given path

test/test.js

@@ -20,13 +20,27 @@ const readline = require('readline');
 const TestRunner = require('../utils/testrunner/');
 const {Environment} = require('../utils/testrunner/Test');
 
+function getCLIArgument(argName) {
+  for (let i = 0; i < process.argv.length; ++i) {
+    // Support `./test.js --foo bar
+    if (process.argv[i] === argName)
+      return process.argv[i + 1];
+    // Support `./test.js --foo=bar
+    if (argName.startsWith('--') && process.argv[i].startsWith(argName + '='))
+      return process.argv[i].substring((argName + '=').length);
+    // Support `./test.js -j4
+    if (!argName.startsWith('--') && argName.startsWith('-') && process.argv[i].startsWith(argName))
+      return process.argv[i].substring(argName.length);
+  }
+  return null;
+}
+
 function collect(browserNames) {
   let parallel = 1;
   if (process.env.PW_PARALLEL_TESTS)
     parallel = parseInt(process.env.PW_PARALLEL_TESTS.trim(), 10);
-  const parallelArgIndex = process.argv.indexOf('-j');
+  if (getCLIArgument('-j'))
-  if (parallelArgIndex !== -1)
+    parallel = parseInt(getCLIArgument('-j'), 10);
-    parallel = parseInt(process.argv[parallelArgIndex + 1], 10);
   require('events').defaultMaxListeners *= parallel;
 
   let timeout = process.env.CI ? 30 * 1000 : 10 * 1000;
@@ -202,30 +216,21 @@ if (require.main === module) {
   });
   const testRunner = collect(browserNames);
 
-  const filterArgIndex = process.argv.indexOf('--filter');
+  const testNameFilter = getCLIArgument('--filter');
-  if (filterArgIndex !== -1) {
+  if (testNameFilter && !testRunner.focusMatchingNameTests(new RegExp(testNameFilter, 'i')).length) {
-    const filter = process.argv[filterArgIndex + 1];
+    console.log('ERROR: no tests matched given `--filter` regex.');
-    if (!testRunner.focusMatchingNameTests(new RegExp(filter, 'i')).length) {
+    process.exit(1);
-      console.log('ERROR: no tests matched given `--filter` regex.');
-      process.exit(1);
-    }
   }
 
-  const fileArgIndex = process.argv.indexOf('--file');
+  const fileNameFilter = getCLIArgument('--file');
-  if (fileArgIndex !== -1) {
+  if (fileNameFilter && !testRunner.focusMatchingFileName(new RegExp(fileNameFilter, 'i')).length) {
-    const filter = process.argv[fileArgIndex + 1];
+    console.log('ERROR: no files matched given `--file` regex.');
-    if (!testRunner.focusMatchingFilePath(new RegExp(filter, 'i')).length) {
+    process.exit(1);
-      console.log('ERROR: no files matched given `--file` regex.');
-      process.exit(1);
-    }
   }
 
-  const repeatArgIndex = process.argv.indexOf('--repeat');
+  const repeat = parseInt(getCLIArgument('--repeat'), 10);
-  if (repeatArgIndex !== -1) {
+  if (!isNaN(repeat))
-    const repeat = parseInt(process.argv[repeatArgIndex + 1], 10);
+    testRunner.repeatAll(repeat);
-    if (!isNaN(repeat))
-      testRunner.repeatAll(repeat);
-  }
 
   testRunner.run().then(() => { delete global.expect; });
 }

utils/testrunner/Reporter.js

@@ -15,6 +15,7 @@
  */
 
 const fs = require('fs');
+const path = require('path');
 const colors = require('colors/safe');
 const {MatchError} = require('./Matchers.js');
 
@@ -38,15 +39,23 @@ class Reporter {
   onStarted(testRuns) {
     this._testCounter = 0;
     this._timestamp = Date.now();
-    if (!this._delegate.hasFocusedTestsOrSuites()) {
+    if (!this._delegate.hasFocusedTestsOrSuitesOrFiles()) {
       console.log(`Running all ${colors.yellow(testRuns.length)} tests on ${colors.yellow(this._delegate.parallel())} worker${this._delegate.parallel() > 1 ? 's' : ''}:\n`);
     } else {
       console.log(`Running ${colors.yellow(testRuns.length)} focused tests out of total ${colors.yellow(this._delegate.testCount())} on ${colors.yellow(this._delegate.parallel())} worker${this._delegate.parallel() > 1 ? 's' : ''}`);
       console.log('');
+      const focusedFilePaths = this._delegate.focusedFilePaths();
+      if (focusedFilePaths.length) {
+        console.log('Focused Files:');
+        for (let i = 0; i < focusedFilePaths.length; ++i)
+          console.log(`  ${i + 1}) ${colors.yellow(path.basename(focusedFilePaths[i]))}`);
+        console.log('');
+      }
       const focusedEntities = [
         ...this._delegate.focusedSuites(),
         ...this._delegate.focusedTests(),
       ];
+
       if (focusedEntities.length) {
         console.log('Focused Suites and Tests:');
         for (let i = 0; i < focusedEntities.length; ++i)

utils/testrunner/TestCollector.js

@@ -21,55 +21,63 @@ const { TestRun } = require('./TestRunner');
 
 class FocusedFilter {
   constructor() {
-    this._focused = new Set();
+    this._focusedTests = new Set();
+    this._focusedSuites = new Set();
+    this._focusedFilePaths = new Set();
   }
 
-  markFocused(testOrSuite) {
+  focusTest(test) { this._focusedTests.add(test); }
-    this._focused.add(testOrSuite);
+  focusSuite(suite) { this._focusedSuites.add(suite); }
-  }
+  focusFilePath(filePath) { this._focusedFilePaths.add(filePath); }
 
-  hasFocusedTestsOrSuites() {
+  hasFocusedTestsOrSuitesOrFiles() {
-    return !!this._focused.size;
+    return !!this._focusedTests.size || !!this._focusedSuites.size || !!this._focusedFilePaths.size;
   }
 
   focusedTests(tests) {
-    return tests.filter(test => this._focused.has(test));
+    return tests.filter(test => this._focusedTests.has(test));
   }
 
   focusedSuites(suites) {
-    return suites.filter(suite => this._focused.has(suite));
+    return suites.filter(suite => this._focusedSuites.has(suite));
+  }
+
+  focusedFilePaths(filePaths) {
+    return filePaths.filter(filePath => this._focusedFilePaths.has(filePath));
   }
 
   filter(tests) {
-    if (!this.hasFocusedTestsOrSuites())
+    if (!this.hasFocusedTestsOrSuitesOrFiles())
       return tests;
 
     const ignoredSuites = new Set();
+    const ignoredFilePaths = new Set();
     for (const test of tests) {
-      if (this._focused.has(test)) {
+      if (this._focusedTests.has(test)) {
         // Focused tests should be run even if skipped.
         test.setSkipped(false);
         // TODO: remove next line once we run failing tests.
         test.setExpectation(test.Expectations.Ok);
+        ignoredFilePaths.add(test.location().filePath());
       }
       for (let suite = test.suite(); suite; suite = suite.parentSuite()) {
-        if (this._focused.has(suite)) {
+        if (this._focusedSuites.has(suite)) {
           // Focused suites should be run even if skipped.
           suite.setSkipped(false);
           // TODO: remove next line once we run failing tests.
           suite.setExpectation(suite.Expectations.Ok);
         }
         // Mark parent suites of focused tests as ignored.
-        if (this._focused.has(test))
+        if (this._focusedTests.has(test))
           ignoredSuites.add(suite);
       }
     }
     // Pick all tests that are focused or belong to focused suites.
     const result = [];
     for (const test of tests) {
-      let focused = this._focused.has(test);
+      let focused = this._focusedTests.has(test) || (this._focusedFilePaths.has(test.location().filePath()) && !ignoredFilePaths.has(test.location().filePath()));
       for (let suite = test.suite(); suite; suite = suite.parentSuite())
-        focused = focused || (this._focused.has(suite) && !ignoredSuites.has(suite));
+        focused = focused || (this._focusedSuites.has(suite) && !ignoredSuites.has(suite));
       if (focused)
         result.push(test);
     }
@@ -225,6 +233,15 @@ class TestCollector {
     return this._suites;
   }
 
+  filePaths() {
+    const filePaths = new Set();
+    for (const test of this._tests)
+      filePaths.add(test.location().filePath());
+    for (const suite of this._suites)
+      filePaths.add(suite.location().filePath());
+    return [...filePaths];
+  }
+
   rootSuite() {
     return this._rootSuite;
   }

utils/testrunner/index.js

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+const path = require('path');
 const { TestRunner, Result, TestResult } = require('./TestRunner');
 const { TestCollector, FocusedFilter, Repeater } = require('./TestCollector');
 const Reporter = require('./Reporter');
@@ -60,10 +61,10 @@ class DefaultTestRunner {
       ...this._collector.api(),
       expect: new Matchers().expect,
     };
-    this._collector.addSuiteAttribute('only', s => this._filter.markFocused(s));
+    this._collector.addSuiteAttribute('only', s => this._filter.focusSuite(s));
     this._collector.addSuiteAttribute('skip', s => s.setSkipped(true));
     this._collector.addSuiteModifier('repeat', (s, count) => this._repeater.repeat(s, count));
-    this._collector.addTestAttribute('only', t => this._filter.markFocused(t));
+    this._collector.addTestAttribute('only', t => this._filter.focusTest(t));
     this._collector.addTestAttribute('skip', t => t.setSkipped(true));
     this._collector.addTestAttribute('todo', t => t.setSkipped(true));
     this._collector.addTestAttribute('slow', t => t.setTimeout(t.timeout() * 3));
@@ -86,22 +87,22 @@ class DefaultTestRunner {
     const focusedTests = [];
     for (const test of this._collector.tests()) {
       if (fullNameRegex.test(test.fullName())) {
-        this._filter.markFocused(test);
+        this._filter.focusTest(test);
         focusedTests.push(test);
       }
     }
     return focusedTests;
   }
 
-  focusMatchingFilePath(filepathRegex) {
+  focusMatchingFileName(filenameRegex) {
-    const focusedTests = [];
+    const focusedFilePaths = [];
-    for (const test of this._collector.tests()) {
+    for (const filePath of this._collector.filePaths()) {
-      if (filepathRegex.test(test.location().filePath())) {
+      if (filenameRegex.test(path.basename(filePath))) {
-        this._filter.markFocused(test);
+        this._filter.focusFilePath(filePath);
-        focusedTests.push(test);
+        focusedFilePaths.push(filePath);
       }
     }
-    return focusedTests;
+    return focusedFilePaths;
   }
 
   repeatAll(repeatCount) {
@@ -113,9 +114,10 @@ class DefaultTestRunner {
 
     if (this._needReporter) {
       const reporterDelegate = {
-        focusedSuites: () => this._filter.focusedTests(this._collector.suites()),
+        focusedSuites: () => this._filter.focusedSuites(this._collector.suites()),
-        focusedTests: () => this._filter.focusedSuites(this._collector.tests()),
+        focusedTests: () => this._filter.focusedTests(this._collector.tests()),
-        hasFocusedTestsOrSuites: () => this._filter.hasFocusedTestsOrSuites(),
+        focusedFilePaths: () => this._filter.focusedFilePaths(this._collector.filePaths()),
+        hasFocusedTestsOrSuitesOrFiles: () => this._filter.hasFocusedTestsOrSuitesOrFiles(),
         parallel: () => this._parallel,
         testCount: () => this._collector.tests().length,
       };
@@ -128,7 +130,7 @@ class DefaultTestRunner {
       reporter = new Reporter(reporterDelegate, reporterOptions);
     }
 
-    if (this._crashIfTestsAreFocusedOnCI && process.env.CI && this._filter.hasFocusedTestsOrSuites()) {
+    if (this._crashIfTestsAreFocusedOnCI && process.env.CI && this._filter.hasFocusedTestsOrSuitesOrFiles()) {
       if (reporter)
         await reporter.onStarted([]);
       const result = new Result();

utils/testrunner/test/testrunner.spec.js

@@ -8,8 +8,8 @@ class Runner {
     this._filter = new FocusedFilter();
     this._repeater = new Repeater();
     this._collector = new TestCollector(options);
-    this._collector.addSuiteAttribute('only', s => this._filter.markFocused(s));
+    this._collector.addSuiteAttribute('only', s => this._filter.focusSuite(s));
-    this._collector.addTestAttribute('only', t => this._filter.markFocused(t));
+    this._collector.addTestAttribute('only', t => this._filter.focusTest(t));
     this._collector.addSuiteAttribute('skip', s => s.setSkipped(true));
     this._collector.addTestAttribute('skip', t => t.setSkipped(true));
     this._collector.addTestAttribute('fail', t => t.setExpectation(t.Expectations.Fail));
@@ -712,16 +712,16 @@ module.exports.addTests = function({describe, fdescribe, xdescribe, it, xit, fit
     });
   });
 
-  describe('TestRunner.hasFocusedTestsOrSuites', () => {
+  describe('TestRunner.hasFocusedTestsOrSuitesOrFiles', () => {
     it('should work', () => {
       const t = new Runner();
       t.it('uno', () => {});
-      expect(t._filter.hasFocusedTestsOrSuites()).toBe(false);
+      expect(t._filter.hasFocusedTestsOrSuitesOrFiles()).toBe(false);
     });
     it('should work #2', () => {
       const t = new Runner();
       t.fit('uno', () => {});
-      expect(t._filter.hasFocusedTestsOrSuites()).toBe(true);
+      expect(t._filter.hasFocusedTestsOrSuitesOrFiles()).toBe(true);
     });
     it('should work #3', () => {
       const t = new Runner();
@@ -732,7 +732,7 @@ module.exports.addTests = function({describe, fdescribe, xdescribe, it, xit, fit
           });
         });
       });
-      expect(t._filter.hasFocusedTestsOrSuites()).toBe(true);
+      expect(t._filter.hasFocusedTestsOrSuitesOrFiles()).toBe(true);
     });
   });