From c90039586d82f918bd329d344e43f001f36cd5c1 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Sat, 15 Aug 2020 13:40:19 -0700 Subject: [PATCH] test: restart worker upon any test failure (#3492) --- test/runner/fixturesUI.js | 21 +++++---- test/runner/index.js | 10 +++-- test/runner/runner.js | 91 ++++++++++++++++++++++----------------- test/runner/testRunner.js | 84 ++++++++++++++++++++++++++++-------- test/runner/worker.js | 2 +- 5 files changed, 133 insertions(+), 75 deletions(-) diff --git a/test/runner/fixturesUI.js b/test/runner/fixturesUI.js index 0f8d55a876..643567449c 100644 --- a/test/runner/fixturesUI.js +++ b/test/runner/fixturesUI.js @@ -69,15 +69,14 @@ function fixturesUI(testRunner, suite) { if (suite.isPending()) fn = null; let wrapper; - if (testRunner.trialRun) { - if (fn) - wrapper = () => {}; - } else { - const wrapped = fixturePool.wrapTestCallback(fn); - wrapper = wrapped ? (done, ...args) => { - wrapped(...args).then(done).catch(done); - } : undefined; - } + const wrapped = fixturePool.wrapTestCallback(fn); + wrapper = wrapped ? (done, ...args) => { + if (!testRunner.shouldRunTest()) { + done(); + return; + } + wrapped(...args).then(done).catch(done); + } : undefined; if (wrapper) { wrapper.toString = () => fn.toString(); wrapper.__original = fn; @@ -114,14 +113,14 @@ function fixturesUI(testRunner, suite) { }); context.beforeEach = (fn) => { - if (testRunner.trialRun) + if (!testRunner.shouldRunTest(true)) return; return common.beforeEach(async () => { return await fixturePool.resolveParametersAndRun(fn); }); }; context.afterEach = (fn) => { - if (testRunner.trialRun) + if (!testRunner.shouldRunTest(true)) return; return common.afterEach(async () => { return await fixturePool.resolveParametersAndRun(fn); diff --git a/test/runner/index.js b/test/runner/index.js index 91f2b38d14..02e68c9e42 100644 --- a/test/runner/index.js +++ b/test/runner/index.js @@ -40,7 +40,7 @@ program let total = 0; // Build the test model, suite per file. for (const file of files) { - const testRunner = new TestRunner(file, { + const testRunner = new TestRunner(file, 0, { forbidOnly: command.forbidOnly || undefined, grep: command.grep, reporter: NullReporter, @@ -63,7 +63,8 @@ program if (!command.reporter) { console.log(); total = Math.min(total, rootSuite.total()); // First accounts for grep, second for only. - console.log(`Running ${total} tests using ${Math.min(command.jobs, total)} workers`); + const workers = Math.min(command.jobs, files.length); + console.log(`Running ${total} test${ total > 1 ? 's' : '' } using ${workers} worker${ workers > 1 ? 's' : ''}`); } // Trial run does not need many workers, use one. @@ -100,9 +101,10 @@ function collectFiles(dir, filters) { files.push(path.join(dir, name)); continue; } + const fullName = path.join(dir, name); for (const filter of filters) { - if (name.includes(filter)) { - files.push(path.join(dir, name)); + if (fullName.includes(filter)) { + files.push(fullName); break; } } diff --git a/test/runner/runner.js b/test/runner/runner.js index 76aec8f8fb..e0741f56ba 100644 --- a/test/runner/runner.js +++ b/test/runner/runner.js @@ -35,7 +35,6 @@ class Runner extends EventEmitter { this._freeWorkers = []; this._workerClaimers = []; this._lastWorkerId = 0; - this._pendingJobs = 0; this.stats = { duration: 0, failures: 0, @@ -49,66 +48,77 @@ class Runner extends EventEmitter { this._tests = new Map(); this._files = new Map(); - this._traverse(suite); - } + let grep; + if (options.grep) { + const match = options.grep.match(/^\/(.*)\/(g|i|)$|.*/); + grep = new RegExp(match[1] || match[0], match[2]); + } - _traverse(suite) { - for (const child of suite.suites) - this._traverse(child); - for (const test of suite.tests) { + suite.eachTest(test => { + if (grep && !grep.test(test.fullTitle())) + return; if (!this._files.has(test.file)) this._files.set(test.file, 0); const counter = this._files.get(test.file); this._files.set(test.file, counter + 1); this._tests.set(`${test.file}::${counter}`, test); - } + }); } _filesSortedByWorkerHash() { const result = []; for (const file of this._files.keys()) - result.push({ file, hash: computeWorkerHash(file) }); + result.push({ file, hash: computeWorkerHash(file), startOrdinal: 0 }); result.sort((a, b) => a.hash < b.hash ? -1 : (a.hash === b.hash ? 0 : 1)); return result; } async run() { this.emit(constants.EVENT_RUN_BEGIN, {}); - const files = this._filesSortedByWorkerHash(); - while (files.length) { - const worker = await this._obtainWorker(); - const requiredHash = files[0].hash; - if (worker.hash && worker.hash !== requiredHash) { - this._restartWorker(worker); - continue; - } - const entry = files.shift(); - worker.hash = requiredHash; - this._runJob(worker, entry.file); - } - await new Promise(f => this._runCompleteCallback = f); + this._queue = this._filesSortedByWorkerHash(); + // Loop in case job schedules more jobs + while (this._queue.length) + await this._dispatchQueue(); this.emit(constants.EVENT_RUN_END, {}); } - _runJob(worker, file) { - ++this._pendingJobs; - worker.run(file); + async _dispatchQueue() { + const jobs = []; + while (this._queue.length) { + const entry = this._queue.shift(); + const requiredHash = entry.hash; + let worker = await this._obtainWorker(); + while (worker.hash && worker.hash !== requiredHash) { + this._restartWorker(worker); + worker = await this._obtainWorker(); + } + jobs.push(this._runJob(worker, entry)); + } + await Promise.all(jobs); + } + + async _runJob(worker, entry) { + worker.run(entry); + let doneCallback; + const result = new Promise(f => doneCallback = f); worker.once('done', params => { - --this._pendingJobs; this.stats.duration += params.stats.duration; this.stats.failures += params.stats.failures; this.stats.passes += params.stats.passes; this.stats.pending += params.stats.pending; - this.stats.tests += params.stats.tests; - if (this._runCompleteCallback && !this._pendingJobs) - this._runCompleteCallback(); - else { - if (params.error) - this._restartWorker(worker); - else - this._workerAvailable(worker); + this.stats.tests += params.stats.passes + params.stats.pending + params.stats.failures; + // When worker encounters error, we will restart it. + 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 }); + } else { + this._workerAvailable(worker); } + doneCallback(); }); + return result; } async _obtainWorker() { @@ -219,8 +229,9 @@ class OopWorker extends EventEmitter { await new Promise(f => this.process.once('message', f)); // Ready ack } - run(file) { - this.process.send({ method: 'run', params: { file, options: this.runner._options } }); + run(entry) { + this.hash = entry.hash; + this.process.send({ method: 'run', params: { file: entry.file, startOrdinal: entry.startOrdinal, options: this.runner._options } }); } stop() { @@ -250,12 +261,12 @@ class InProcessWorker extends EventEmitter { async init() { } - async run(file) { - delete require.cache[file]; + async run(entry) { + delete require.cache[entry.file]; const { TestRunner } = require('./testRunner'); - const testRunner = new TestRunner(file, this.runner._options); + const testRunner = new TestRunner(entry.file, entry.startOrdinal, this.runner._options); for (const event of ['test', 'pending', 'pass', 'fail', 'done']) - testRunner.on(event, this.emit.bind(this, event)); + testRunner.on(event, this.emit.bind(this, event)); testRunner.run(); } diff --git a/test/runner/testRunner.js b/test/runner/testRunner.js index 80725b298f..ca46de001f 100644 --- a/test/runner/testRunner.js +++ b/test/runner/testRunner.js @@ -33,7 +33,7 @@ const GoldenUtils = require('./GoldenUtils'); class NullReporter {} class TestRunner extends EventEmitter { - constructor(file, options) { + constructor(file, startOrdinal, options) { super(); this.mocha = new Mocha({ forbidOnly: options.forbidOnly, @@ -43,47 +43,94 @@ class TestRunner extends EventEmitter { }); if (options.grep) this.mocha.grep(options.grep); + this._currentOrdinal = -1; + this._failedWithError = false; + this._startOrdinal = startOrdinal; + this._trialRun = options.trialRun; + this._passes = 0; + this._failures = 0; + this._pending = 0; + this.mocha.addFile(file); this.mocha.suite.filterOnly(); this.mocha.loadFiles(); this.suite = this.mocha.suite; - this._lastOrdinal = -1; - this._failedWithError = false; - this.trialRun = options.trialRun; } async run() { 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 => { - this.emit('test', { test: serializeTest(test, ++this._lastOrdinal) }); + if (this._failedWithError) { + ++remaining; + return; + } + if (++this._currentOrdinal < this._startOrdinal) + return; + this.emit('test', { test: serializeTest(test, this._currentOrdinal) }); }); runner.on(constants.EVENT_TEST_PENDING, test => { - this.emit('pending', { test: serializeTest(test, ++this._lastOrdinal) }); + if (this._failedWithError) { + ++remaining; + return; + } + if (++this._currentOrdinal < this._startOrdinal) + return; + ++this._pending; + this.emit('pending', { test: serializeTest(test, this._currentOrdinal) }); }); runner.on(constants.EVENT_TEST_PASS, test => { - this.emit('pass', { test: serializeTest(test, this._lastOrdinal) }); + if (this._failedWithError) + return; + + if (this._currentOrdinal < this._startOrdinal) + return; + ++this._passes; + this.emit('pass', { test: serializeTest(test, this._currentOrdinal) }); }); runner.on(constants.EVENT_TEST_FAIL, (test, error) => { + if (this._failedWithError) + return; + ++this._failures; this._failedWithError = error; this.emit('fail', { - test: serializeTest(test, this._lastOrdinal), + test: serializeTest(test, this._currentOrdinal), error: serializeError(error), }); }); runner.once(constants.EVENT_RUN_END, async () => { - this.emit('done', { stats: serializeStats(runner.stats), error: this._failedWithError }); + this.emit('done', { + stats: this._serializeStats(runner.stats), + error: this._failedWithError, + remaining, + total: runner.stats.tests + }); }); await result; } + shouldRunTest(hook) { + if (this._trialRun || this._failedWithError) + return false; + if (hook) { + // Hook starts before we bump the test ordinal. + if (this._currentOrdinal + 1 < this._startOrdinal) + return false; + } else { + if (this._currentOrdinal < this._startOrdinal) + return false; + } + return true; + } + grepTotal() { let total = 0; this.suite.eachTest(test => { @@ -92,6 +139,15 @@ class TestRunner extends EventEmitter { }); return total; } + + _serializeStats(stats) { + return { + passes: this._passes, + failures: this._failures, + pending: this._pending, + duration: stats.duration || 0, + } + } } function createTestSuite() { @@ -105,16 +161,6 @@ function serializeTest(test, origin) { }; } -function serializeStats(stats) { - return { - tests: stats.tests, - passes: stats.passes, - duration: stats.duration, - failures: stats.failures, - pending: stats.pending, - } -} - function trimCycles(obj) { const cache = new Set(); return JSON.parse( diff --git a/test/runner/worker.js b/test/runner/worker.js index 9c1ae81b43..41eb00cb3a 100644 --- a/test/runner/worker.js +++ b/test/runner/worker.js @@ -55,7 +55,7 @@ process.on('message', async message => { await fixturePool.teardownScope('worker'); await gracefullyCloseAndExit(); } if (message.method === 'run') { - const testRunner = new TestRunner(message.params.file, message.params.options); + const testRunner = new TestRunner(message.params.file, message.params.startOrdinal, message.params.options); for (const event of ['test', 'pending', 'pass', 'fail', 'done']) testRunner.on(event, sendMessageToParent.bind(null, event)); await testRunner.run();