From 839070341c292e72c7aa0b41b23c27ec23846434 Mon Sep 17 00:00:00 2001 From: Ian Walter Date: Wed, 24 Apr 2019 17:43:11 -0400 Subject: [PATCH 1/4] Working on failFast option --- cli.js | 3 ++- index.js | 72 ++++++++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 56 insertions(+), 19 deletions(-) diff --git a/cli.js b/cli.js index 7acab6dd..a99a940b 100755 --- a/cli.js +++ b/cli.js @@ -13,7 +13,8 @@ async function run () { updateSnapshot: 'u', logLevel: 'l', tags: 't', - timeout: 'T' + timeout: 'T', + failFast: 'f' } } }) diff --git a/index.js b/index.js index 69d4c688..e241cd12 100644 --- a/index.js +++ b/index.js @@ -88,8 +88,17 @@ function run (config) { // Get the snapshot state for the current test file. const snapshotState = getSnapshotState(file, context.updateSnapshot) + // Collect the execution promises in an array so that they can be + // cancelled if need be (e.g. on fastFail before pool termination). + const inProgress = [] + // Iterate through all tests in the test file. const runAllTestsInFile = Promise.all(tests.map(async test => { + // Define the execution promise outside of try-catch-finally so that + // it can be referenced when it needs to be removed from the + // inProgress collection. + let execution + try { // Mark all tests as having been checked for snapshot changes so // that tests that have been removed can have their associated @@ -97,22 +106,30 @@ function run (config) { // test file. snapshotState.markSnapshotsAsCheckedForTest(test.name) - // Don't execute the test if there is a test in the test file marked - // with the only modifier and it's not this test or if the test - // is marked with the skip modifier. - if (test.skip || (hasOnly && !test.only)) { - if (test.skip) { - // Output the test name and increment the skip count to remind - // the user that some tests are being skipped. - print.log('🛌', test.name) - context.skipped++ - } + if (context.hasFastFailure) { + // Don't execute the test if the fastFail option is set and there + // has been a test failure. + print.debug('Skipping test because of fastFail flag:', test.name) + } else if (hasOnly && !test.only) { + // Don't execute the test if there is a test in the current test + // file marked with the only modifier and it's not this test. + print.debug('Skipping test because of only modifier:', test.name) + } else if (test.skip) { + // Output the test name and increment the skip count to remind + // the user that some tests are being explicitly skipped. + print.log('🛌', test.name) + context.skipped++ } else { // Send the test to a worker in the execution pool to be executed. - const result = await executionPool.exec( - 'test', - [file, test, context] - ) + execution = executionPool.exec('test', [file, test, context]) + + // Push the execution promise to the inProgress collection so + // that, if need be, it can be cancelled later (e.g. on fastFail). + inProgress.push(execution) + + // Wait for the execution promise to complete so that the test + // results can be handled. + const result = await execution // Update the snapshot state with the snapshot data received from // the worker. @@ -130,7 +147,11 @@ function run (config) { context.passed++ } } catch (err) { - if (err.name === 'TimeoutError') { + if (context.hasFastFailure) { + // Ignore new thrown errors when a "fast failure" has been + // recorded. + return + } else if (err.name === 'TimeoutError') { print.error( `${test.name}: timeout`, chalk.gray(path.relative(process.cwd(), file)) @@ -138,10 +159,24 @@ function run (config) { } else { print.error(`${test.name}:`, err) } + + // Increment the failure count since the test threw an error + // indicating a test failure. context.failed++ + + // If the failFast option is set, record that there's been a "fast + // failure" and try to cancel any in-progress executions. + if (context.failFast) { + context.hasFastFailure = true + await Promise.all(inProgress.map(async exec => exec.cancel())) + } } finally { // Increment the execution count now that the test has completed. context.executed++ + + // Remove the current execution promise from the inProgress + // collection. + inProgress.splice(inProgress.indexOf(execution), 1) } })) @@ -158,8 +193,9 @@ function run (config) { snapshotState.save() if ( - context.filesRegistered === context.files.length && - context.executed === context.testsRegistered + context.hasFastFailure || + (context.filesRegistered === context.files.length && + context.executed === context.testsRegistered) ) { // Execute each function with the run context exported by the // files configured to be called after a run. @@ -172,7 +208,7 @@ function run (config) { .then(() => print.debug('Execution pool terminated')) // Resolve the run Promise with the run context which contains - // the tests' pass/fail/skip counts. + // the tests' passed/failed/skipped counts. resolve(context) } } catch (err) { From ebdb0aacf46ce72404e3ed7f208280bba28623e0 Mon Sep 17 00:00:00 2001 From: Ian Walter Date: Wed, 24 Apr 2019 17:47:33 -0400 Subject: [PATCH 2/4] Fixing typos --- index.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index e241cd12..3d2840a6 100644 --- a/index.js +++ b/index.js @@ -89,7 +89,7 @@ function run (config) { const snapshotState = getSnapshotState(file, context.updateSnapshot) // Collect the execution promises in an array so that they can be - // cancelled if need be (e.g. on fastFail before pool termination). + // cancelled if need be (e.g. on failFast before pool termination). const inProgress = [] // Iterate through all tests in the test file. @@ -107,9 +107,9 @@ function run (config) { snapshotState.markSnapshotsAsCheckedForTest(test.name) if (context.hasFastFailure) { - // Don't execute the test if the fastFail option is set and there + // Don't execute the test if the failFast option is set and there // has been a test failure. - print.debug('Skipping test because of fastFail flag:', test.name) + print.debug('Skipping test because of failFast flag:', test.name) } else if (hasOnly && !test.only) { // Don't execute the test if there is a test in the current test // file marked with the only modifier and it's not this test. @@ -124,7 +124,7 @@ function run (config) { execution = executionPool.exec('test', [file, test, context]) // Push the execution promise to the inProgress collection so - // that, if need be, it can be cancelled later (e.g. on fastFail). + // that, if need be, it can be cancelled later (e.g. on failFast). inProgress.push(execution) // Wait for the execution promise to complete so that the test From f5a059655f35ae9db913d1d10fb1d5c9b49ce602 Mon Sep 17 00:00:00 2001 From: Ian Walter Date: Wed, 24 Apr 2019 17:59:57 -0400 Subject: [PATCH 3/4] Fixing cancels --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index 3d2840a6..ddf5aeb5 100644 --- a/index.js +++ b/index.js @@ -168,7 +168,7 @@ function run (config) { // failure" and try to cancel any in-progress executions. if (context.failFast) { context.hasFastFailure = true - await Promise.all(inProgress.map(async exec => exec.cancel())) + inProgress.forEach(exec => exec.cancel()) } } finally { // Increment the execution count now that the test has completed. From 421fa596deddb7d3dd2d494ce8f6deb23c71fce0 Mon Sep 17 00:00:00 2001 From: Ian Walter Date: Wed, 24 Apr 2019 19:49:45 -0400 Subject: [PATCH 4/4] Forcing pool termination if hasFastFailure is true --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index ddf5aeb5..8e8a89cd 100644 --- a/index.js +++ b/index.js @@ -204,7 +204,7 @@ function run (config) { } // Terminate the execution pool if all tests have been run. - executionPool.terminate() + executionPool.terminate(context.hasFastFailure) .then(() => print.debug('Execution pool terminated')) // Resolve the run Promise with the run context which contains