test: run tests by ordinals, not ranges (#3497)

test/base.fixture.ts

@@ -25,7 +25,7 @@ import { Transport } from '../lib/rpc/transport';
 import { setUnderTest } from '../lib/helper';
 import { installCoverageHooks } from './runner/coverage';
 import { valueFromEnv } from './runner/utils';
-import { registerFixture, registerWorkerFixture} from './runner/fixtures';
+import { registerFixture, registerWorkerFixture } from './runner/fixtures';
 import './runner/builtin.fixtures';
 
 import {mkdtempAsync, removeFolderAsync} from './utils';
@@ -42,6 +42,7 @@ declare global {
     golden: (path: string) => string;
     playwright: typeof import('../index');
     browserType: BrowserType<Browser>;
+    browserName: string;
     browser: Browser;
   }
   interface FixtureState {
@@ -81,7 +82,7 @@ registerWorkerFixture('httpService', async ({parallelIndex}, test) => {
   ]);
 });
 
-const getExecutablePath = () => {
+const getExecutablePath = (browserName) => {
   if (browserName === 'chromium' && process.env.CRPATH)
     return process.env.CRPATH;
   if (browserName === 'firefox' && process.env.FFPATH)
@@ -91,8 +92,8 @@ const getExecutablePath = () => {
   return
 }
 
-registerWorkerFixture('defaultBrowserOptions', async({}, test) => {
-  let executablePath = getExecutablePath();
+registerWorkerFixture('defaultBrowserOptions', async({browserName}, test) => {
+  let executablePath = getExecutablePath(browserName);
 
   if (executablePath)
     console.error(`Using executable at ${executablePath}`);
@@ -104,7 +105,7 @@ registerWorkerFixture('defaultBrowserOptions', async({}, test) => {
   });
 });
 
-registerWorkerFixture('playwright', async({parallelIndex}, test) => {
+registerWorkerFixture('playwright', async({parallelIndex, browserName}, test) => {
   const {coverage, uninstall} = installCoverageHooks(browserName);
   if (process.env.PWWIRE) {
     const connection = new Connection();
@@ -146,9 +147,9 @@ registerFixture('toImpl', async ({playwright}, test) => {
   await test((playwright as any)._toImpl);
 });
 
-registerWorkerFixture('browserType', async ({playwright}, test) => {
+registerWorkerFixture('browserType', async ({playwright, browserName}, test) => {
   const browserType = playwright[process.env.BROWSER || 'chromium']
-  const executablePath = getExecutablePath()
+  const executablePath = getExecutablePath(browserName)
   if (executablePath)
     browserType._executablePath = executablePath
   await test(browserType);
@@ -180,7 +181,7 @@ registerFixture('server', async ({httpService}, test) => {
   await test(httpService.server);
 });
 
-registerFixture('browserName', async ({}, test) => {
+registerWorkerFixture('browserName', async ({}, test) => {
   await test(browserName);
 });
 

test/focus.spec.ts

@@ -74,6 +74,7 @@ it('should traverse focus in all directions', async function({page}) {
   await page.keyboard.press('Shift+Tab');
   expect(await page.evaluate(() => (document.activeElement as HTMLInputElement).value)).toBe('1');
 });
+
 // Chromium and WebKit both have settings for tab traversing all links, but 
 // it is only on by default in WebKit.
 it.skip(!MAC || !WEBKIT)('should traverse only form elements', async function({page}) {

test/page-goto.spec.ts

@@ -173,7 +173,7 @@ it('should fail when navigating to bad url', async({page, server}) => {
     expect(error.message).toContain('Invalid url');
 });
 
-it('should fail when navigating to bad SSL', async({page, httpsServer}) => {
+it('should fail when navigating to bad SSL', async({page, httpsServer, browserName}) => {
   // Make sure that network events do not emit 'undefined'.
   // @see https://crbug.com/750469
   page.on('request', request => expect(request).toBeTruthy());
@@ -181,15 +181,15 @@ it('should fail when navigating to bad SSL', async({page, httpsServer}) => {
   page.on('requestfailed', request => expect(request).toBeTruthy());
   let error = null;
   await page.goto(httpsServer.EMPTY_PAGE).catch(e => error = e);
-  utils.expectSSLError(error.message, );
+  utils.expectSSLError(browserName, error.message, );
 });
 
-it('should fail when navigating to bad SSL after redirects', async({page, server, httpsServer}) => {
+it('should fail when navigating to bad SSL after redirects', async({page, server, httpsServer, browserName}) => {
   server.setRedirect('/redirect/1.html', '/redirect/2.html');
   server.setRedirect('/redirect/2.html', '/empty.html');
   let error = null;
   await page.goto(httpsServer.PREFIX + '/redirect/1.html').catch(e => error = e);
-  utils.expectSSLError(error.message);
+  utils.expectSSLError(browserName, error.message);
 });
 
 it('should not crash when navigating to bad SSL after a cross origin navigation', async({page, server, httpsServer}) => {

test/page-wait-for-navigation.spec.ts

@@ -71,14 +71,14 @@ it('should work with clicking on anchor links', async({page, server}) => {
   expect(page.url()).toBe(server.EMPTY_PAGE + '#foobar');
 });
 
-it('should work with clicking on links which do not commit navigation', async({page, server, httpsServer}) => {
+it('should work with clicking on links which do not commit navigation', async({page, server, httpsServer, browserName}) => {
   await page.goto(server.EMPTY_PAGE);
   await page.setContent(`<a href='${httpsServer.EMPTY_PAGE}'>foobar</a>`);
   const [error] = await Promise.all([
     page.waitForNavigation().catch(e => e),
     page.click('a'),
   ]);
-  utils.expectSSLError(error.message);
+  utils.expectSSLError(browserName, error.message);
 });
 
 it('should work with history.pushState()', async({page, server}) => {

test/runner/fixtures.js

@@ -120,6 +120,21 @@ class FixturePool {
       }
     };
   }
+
+  fixtures(callback) {
+    const result = new Set();
+    const visit  = (callback) => {
+      for (const name of fixtureParameterNames(callback)) {
+        if (name in result)
+          continue;
+        result.add(name);
+        const { fn } = registrations.get(name)
+        visit(fn);
+      }
+    };
+    visit(callback);
+    return result;
+  }
 }
 
 function fixtureParameterNames(fn) {
@@ -146,7 +161,7 @@ function registerFixture(name, fn) {
   innerRegisterFixture(name, 'test', fn);
 };
 
-function registerWorkerFixture (name, fn) {
+function registerWorkerFixture(name, fn) {
   innerRegisterFixture(name, 'worker', fn);
 };
 

test/runner/fixturesUI.js

@@ -81,6 +81,7 @@ function fixturesUI(testRunner, suite) {
         wrapper.__original = fn;
       }
       const test = new Test(title, wrapper);
+      test.__fixtures = fixturePool.fixtures(fn);
       test.file = file;
       suite.addTest(test);
       const only = specs.only && specs.only[0];

test/runner/index.js

@@ -43,7 +43,7 @@ program
     let total = 0;
     // Build the test model, suite per file.
     for (const file of files) {
-      const testRunner = new TestRunner(file, 0, {
+      const testRunner = new TestRunner(file, [], {
         forbidOnly: command.forbidOnly || undefined,
         grep: command.grep,
         reporter: NullReporter,

test/runner/runner.js

@@ -67,8 +67,8 @@ class Runner extends EventEmitter {
 
   _filesSortedByWorkerHash() {
     const result = [];
-    for (const file of this._files.keys())
-      result.push({ file, hash: computeWorkerHash(file), startOrdinal: 0 });
+    for (const [file, count] of this._files.entries())
+      result.push({ file, hash: computeWorkerHash(file), ordinals: new Array(count).fill(0).map((_, i) => i) });
     result.sort((a, b) => a.hash < b.hash ? -1 : (a.hash === b.hash ? 0 : 1));
     return result;
   }
@@ -111,8 +111,8 @@ class Runner extends EventEmitter {
       if (params.error) {
         this._restartWorker(worker);
         // If there are remaining tests, we will queue them.
-        if (params.remaining)
-          this._queue.unshift({ ...entry, startOrdinal: params.total - params.remaining });
+        if (params.remaining.length)
+          this._queue.unshift({ ...entry, ordinals: params.remaining });
       } else {
         this._workerAvailable(worker);
       }
@@ -231,7 +231,7 @@ class OopWorker extends EventEmitter {
 
   run(entry) {
     this.hash = entry.hash;
-    this.process.send({ method: 'run', params: { file: entry.file, startOrdinal: entry.startOrdinal, options: this.runner._options } });
+    this.process.send({ method: 'run', params: { file: entry.file, ordinals: entry.ordinals, options: this.runner._options } });
   }
 
   stop() {
@@ -268,7 +268,7 @@ class InProcessWorker extends EventEmitter {
   async run(entry) {
     delete require.cache[entry.file];
     const { TestRunner } = require('./testRunner');
-    const testRunner = new TestRunner(entry.file, entry.startOrdinal, this.runner._options);
+    const testRunner = new TestRunner(entry.file, entry.ordinals, this.runner._options);
     for (const event of ['test', 'pending', 'pass', 'fail', 'done'])
       testRunner.on(event, this.emit.bind(this, event));
     testRunner.run();

test/runner/testRunner.js

@@ -26,7 +26,7 @@ const GoldenUtils = require('./GoldenUtils');
 class NullReporter {}
 
 class TestRunner extends EventEmitter {
-  constructor(file, startOrdinal, options) {
+  constructor(file, ordinals, options) {
     super();
     this.mocha = new Mocha({
       forbidOnly: options.forbidOnly,
@@ -38,7 +38,8 @@ class TestRunner extends EventEmitter {
       this.mocha.grep(options.grep);
     this._currentOrdinal = -1;
     this._failedWithError = false;
-    this._startOrdinal = startOrdinal;
+    this._ordinals = new Set(ordinals);
+    this._remaining = new Set(ordinals);
     this._trialRun = options.trialRun;
     this._passes = 0;
     this._failures = 0;
@@ -54,39 +55,39 @@ class TestRunner extends EventEmitter {
     let callback;
     const result = new Promise(f => callback = f);
     const runner = this.mocha.run(callback);
-    let remaining = 0;
 
     const constants = Mocha.Runner.constants;
     runner.on(constants.EVENT_TEST_BEGIN, test => {
       relativeTestFile = this._relativeTestFile;
-      if (this._failedWithError) {
-        ++remaining;
+      if (this._failedWithError)
         return;
-      }
-      if (++this._currentOrdinal < this._startOrdinal)
+      const ordinal = ++this._currentOrdinal;
+      if (this._ordinals.size && !this._ordinals.has(ordinal))
         return;
-      this.emit('test', { test: serializeTest(test, this._currentOrdinal) });
+      this._remaining.delete(ordinal);
+      this.emit('test', { test: serializeTest(test, ordinal) });
     });
 
     runner.on(constants.EVENT_TEST_PENDING, test => {
-      if (this._failedWithError) {
-        ++remaining;
+      if (this._failedWithError)
         return;
-      }
-      if (++this._currentOrdinal < this._startOrdinal)
+      const ordinal = ++this._currentOrdinal;
+      if (this._ordinals.size && !this._ordinals.has(ordinal))
         return;
+      this._remaining.delete(ordinal);
       ++this._pending;
-      this.emit('pending', { test: serializeTest(test, this._currentOrdinal) });
+      this.emit('pending', { test: serializeTest(test, ordinal) });
     });
 
     runner.on(constants.EVENT_TEST_PASS, test => {
       if (this._failedWithError)
         return;
 
-      if (this._currentOrdinal < this._startOrdinal)
+      const ordinal = this._currentOrdinal;
+      if (this._ordinals.size && !this._ordinals.has(ordinal))
         return;
       ++this._passes;
-      this.emit('pass', { test: serializeTest(test, this._currentOrdinal) });
+      this.emit('pass', { test: serializeTest(test, ordinal) });
     });
 
     runner.on(constants.EVENT_TEST_FAIL, (test, error) => {
@@ -104,7 +105,7 @@ class TestRunner extends EventEmitter {
       this.emit('done', {
         stats: this._serializeStats(runner.stats),
         error: this._failedWithError,
-        remaining,
+        remaining: [...this._remaining],
         total: runner.stats.tests
       });
     });
@@ -116,10 +117,10 @@ class TestRunner extends EventEmitter {
       return false;
     if (hook) {
       // Hook starts before we bump the test ordinal.
-      if (this._currentOrdinal + 1 < this._startOrdinal)
+      if (!this._ordinals.has(this._currentOrdinal + 1))
         return false;
     } else {
-      if (this._currentOrdinal < this._startOrdinal)
+      if (!this._ordinals.has(this._currentOrdinal))
         return false;
     }
     return true;

test/runner/worker.js

@@ -62,7 +62,7 @@ process.on('message', async message => {
     return;
   }
   if (message.method === 'run') {
-    const testRunner = new TestRunner(message.params.file, message.params.startOrdinal, message.params.options);
+    const testRunner = new TestRunner(message.params.file, message.params.ordinals, message.params.options);
     for (const event of ['test', 'pending', 'pass', 'fail', 'done'])
       testRunner.on(event, sendMessageToParent.bind(null, event));
     await testRunner.run();

test/utils.js

@@ -23,7 +23,6 @@ const removeFolder = require('rimraf');
 
 const {FlakinessDashboard} = require('../utils/flakiness-dashboard');
 const PROJECT_ROOT = fs.existsSync(path.join(__dirname, '..', 'package.json')) ? path.join(__dirname, '..') : path.join(__dirname, '..', '..');
-const browserName = process.env.BROWSER || 'chromium';
 
 let platform = os.platform();
 
@@ -227,7 +226,7 @@ const utils = module.exports = {
     return logger;
   },
 
-  expectSSLError(errorMessage) {
+  expectSSLError(browserName, errorMessage) {
     if (browserName === 'chromium') {
       expect(errorMessage).toContain('net::ERR_CERT_AUTHORITY_INVALID');
     } else if (browserName === 'webkit') {