Ensure scenario termination

lib/command/gherkin/init.js

@@ -70,7 +70,7 @@ module.exports = function (genPath) {
   }
 
   config.gherkin = {
-    features: './features/*.feature',
+    features: "./features/*.feature",
     steps: [`./step_definitions/steps.${extension}`],
   };
 

lib/plugin/retryTo.js

@@ -104,15 +104,20 @@ module.exports = function (config) {
           recorder.session.restore(`retryTo ${tries}`);
           if (tries <= maxTries) {
             debug(`Error ${err}... Retrying`);
-            recorder.add(`retryTo ${tries}`, () => setTimeout(tryBlock, pollInterval));
+            recorder.add(`retryTo ${tries}`, () =>
+              setTimeout(tryBlock, pollInterval)
+            );
           } else {
             // if maxTries reached
             handleRetryException(err);
           }
         });
       };
 
-      recorder.add('retryTo', tryBlock);
+      recorder.add('retryTo', tryBlock).catch(err => {
+        console.error('An error occurred:', err);
+        done(null);
+      });
     });
   }
 

lib/scenario.js

@@ -18,6 +18,16 @@ const injectHook = function (inject, suite) {
   return recorder.promise();
 };
 
+function makeDoneCallableOnce(done) {
+  let called = false;
+  return function (err) {
+    if (called) {
+      return;
+    }
+    called = true;
+    return done(err);
+  };
+}
 /**
  * Wraps test function, injects support objects from container,
  * starts promise chain with recorder, performs before/after hooks
@@ -34,56 +44,44 @@ module.exports.test = (test) => {
   test.async = true;
 
   test.fn = function (done) {
+    const doneFn = makeDoneCallableOnce(done);
     recorder.errHandler((err) => {
       recorder.session.start('teardown');
       recorder.cleanAsyncErr();
-      if (test.throws) { // check that test should actually fail
+      if (test.throws) {
+        // check that test should actually fail
         try {
           assertThrown(err, test.throws);
           event.emit(event.test.passed, test);
           event.emit(event.test.finished, test);
-          recorder.add(() => done());
+          recorder.add(doneFn);
           return;
         } catch (newErr) {
           err = newErr;
         }
       }
       event.emit(event.test.failed, test, err);
       event.emit(event.test.finished, test);
-      recorder.add(() => done(err));
+      recorder.add(() => doneFn(err));
     });
 
     if (isAsyncFunction(testFn)) {
       event.emit(event.test.started, test);
-
-      const catchError = e => {
-        recorder.throw(e);
-        recorder.catch((e) => {
-          const err = (recorder.getAsyncErr() === null) ? e : recorder.getAsyncErr();
-          recorder.session.start('teardown');
-          recorder.cleanAsyncErr();
-          event.emit(event.test.failed, test, err);
-          event.emit(event.test.finished, test);
-          recorder.add(() => done(err));
+      testFn
+        .call(test, getInjectedArguments(testFn, test))
+        .then(() => {
+          recorder.add('fire test.passed', () => {
+            event.emit(event.test.passed, test);
+            event.emit(event.test.finished, test);
+          });
+          recorder.add('finish test', doneFn);
+        })
+        .catch((err) => {
+          recorder.throw(err);
+        })
+        .finally(() => {
+          recorder.catch();
         });
-      };
-
-      let injectedArguments;
-      try {
-        injectedArguments = getInjectedArguments(testFn, test);
-      } catch (e) {
-        catchError(e);
-        return;
-      }
-
-      testFn.call(test, injectedArguments).then(() => {
-        recorder.add('fire test.passed', () => {
-          event.emit(event.test.passed, test);
-          event.emit(event.test.finished, test);
-        });
-        recorder.add('finish test', () => done());
-        recorder.catch();
-      }).catch(catchError);
       return;
     }
 
@@ -97,7 +95,7 @@ module.exports.test = (test) => {
         event.emit(event.test.passed, test);
         event.emit(event.test.finished, test);
       });
-      recorder.add('finish test', () => done());
+      recorder.add('finish test', doneFn);
       recorder.catch();
     }
   };
@@ -109,13 +107,14 @@ module.exports.test = (test) => {
  */
 module.exports.injected = function (fn, suite, hookName) {
   return function (done) {
+    const doneFn = makeDoneCallableOnce(done);
     const errHandler = (err) => {
       recorder.session.start('teardown');
       recorder.cleanAsyncErr();
       event.emit(event.test.failed, suite, err);
       if (hookName === 'after') event.emit(event.test.after, suite);
       if (hookName === 'afterSuite') event.emit(event.suite.after, suite);
-      recorder.add(() => done(err));
+      recorder.add(() => doneFn(err));
     };
 
     recorder.errHandler((err) => {
@@ -137,28 +136,32 @@ module.exports.injected = function (fn, suite, hookName) {
     const opts = suite.opts || {};
     const retries = opts[`retry${ucfirst(hookName)}`] || 0;
 
-    promiseRetry(async (retry, number) => {
-      try {
-        recorder.startUnlessRunning();
-        await fn.call(this, getInjectedArguments(fn));
-        await recorder.promise().catch(err => retry(err));
-      } catch (err) {
-        retry(err);
-      } finally {
-        if (number < retries) {
-          recorder.stop();
-          recorder.start();
+    promiseRetry(
+      async (retry, number) => {
+        try {
+          recorder.startUnlessRunning();
+          await fn.call(this, getInjectedArguments(fn));
+          await recorder.promise().catch((err) => retry(err));
+        } catch (err) {
+          retry(err);
+        } finally {
+          if (number < retries) {
+            recorder.stop();
+            recorder.start();
+          }
         }
-      }
-    }, { retries })
+      },
+      { retries },
+    )
       .then(() => {
         recorder.add('fire hook.passed', () => event.emit(event.hook.passed, suite));
-        recorder.add(`finish ${hookName} hook`, () => done());
+        recorder.add(`finish ${hookName} hook`, doneFn);
         recorder.catch();
-      }).catch((e) => {
+      })
+      .catch((e) => {
         recorder.throw(e);
         recorder.catch((e) => {
-          const err = (recorder.getAsyncErr() === null) ? e : recorder.getAsyncErr();
+          const err = recorder.getAsyncErr() === null ? e : recorder.getAsyncErr();
           errHandler(err);
         });
         recorder.add('fire hook.failed', () => event.emit(event.hook.failed, suite, e));

test/acceptance/retryTo_test.js

@@ -15,6 +15,14 @@ Scenario('retryTo works with non await steps @plugin', async () => {
   }, 4);
 });
 
+Scenario('Should be succeed', async ({ I }) => {
+  I.amOnPage('http://example.org');
+  I.waitForVisible('.nothing', 1); // should fail here but it won't terminate
+  await retryTo((tryNum) => {
+    I.see('.doesNotMatter');
+  }, 10);
+});
+
 Scenario('Should fail after reached max retries', async () => {
   await retryTo(() => {
     throw new Error('Custom pluginRetryTo Error');
@@ -25,4 +33,4 @@ Scenario('Should succeed at the third attempt @plugin', async () => {
   await retryTo(async (tryNum) => {
     if (tryNum < 2) throw new Error('Custom pluginRetryTo Error');
   }, 3);
-});
+});

test/data/sandbox/codecept.scenario-stale.js

@@ -0,0 +1,10 @@
+exports.config = {
+  tests: './test.scenario-stale.js',
+  timeout: 10000,
+  retry: 2,
+  output: './output',
+  include: {},
+  bootstrap: false,
+  mocha: {},
+  name: 'sandbox',
+};

test/data/sandbox/test.scenario-stale.js

@@ -0,0 +1,22 @@
+Feature('Scenario should not be staling');
+
+const SHOULD_NOT_STALE = 'should not stale scenario error';
+
+Scenario('Rejected promise should not stale the process', async () => {
+  await new Promise((_resolve, reject) => setTimeout(reject(new Error(SHOULD_NOT_STALE)), 500));
+});
+
+Scenario('Should handle throw inside synchronous and terminate gracefully', () => {
+  throw new Error(SHOULD_NOT_STALE);
+});
+Scenario('Should handle throw inside async and terminate gracefully', async () => {
+  throw new Error(SHOULD_NOT_STALE);
+});
+
+Scenario('Should throw, retry and keep failing', async () => {
+  setTimeout(() => {
+    throw new Error(SHOULD_NOT_STALE);
+  }, 500);
+  await new Promise((resolve) => setTimeout(resolve, 300));
+  throw new Error(SHOULD_NOT_STALE);
+}).retry(2);

test/plugin/plugin_test.js

@@ -32,6 +32,15 @@ describe('CodeceptJS plugin', function () {
     });
   });
 
+  it('should failed before the retryTo instruction', (done) => {
+    exec(`${config_run_config('codecept.Playwright.retryTo.js', 'Should be succeed')} --verbose`, (err, stdout) => {
+      expect(stdout).toContain('locator.waitFor: Timeout 1000ms exceeded.'),
+      expect(stdout).toContain('[1] Error | Error: element (.nothing) still not visible after 1 sec'),
+      expect(err).toBeTruthy();
+      done();
+    });
+  });
+
   it('should generate the coverage report', (done) => {
     exec(`${config_run_config('codecept.Playwright.coverage.js', '@coverage')} --debug`, (err, stdout) => {
       const lines = stdout.split('\n');
@@ -63,13 +72,15 @@ describe('CodeceptJS plugin', function () {
   });
 
   it('should retry to failure', (done) => {
-    exec(`${config_run_config('codecept.Playwright.retryTo.js', 'Should fail after reached max retries')} --verbose`, (err, stdout) => {
-      const lines = stdout.split('\n');
-      expect(lines).toEqual(
-        expect.arrayContaining([expect.stringContaining('Custom pluginRetryTo Error')]),
-      );
-      expect(err).toBeTruthy();
-      done();
-    });
+    exec(
+      `${config_run_config('codecept.Playwright.retryTo.js', 'Should fail after reached max retries')} --verbose`, (err, stdout) => {
+        const lines = stdout.split('\n');
+        expect(lines).toEqual(
+          expect.arrayContaining([expect.stringContaining('Custom pluginRetryTo Error')])
+        );
+        expect(err).toBeTruthy();
+        done();
+      }
+    );
   });
 });

test/runner/run_workers_test.js

@@ -19,7 +19,6 @@ describe('CodeceptJS Workers Runner', function () {
     if (!semver.satisfies(process.version, '>=11.7.0')) this.skip('not for node version');
     console.log(`${codecept_run} 3 --debug`);
     exec(`${codecept_run} 3 --debug`, (err, stdout) => {
-      console.log('aaaaaaaaaaaaa', stdout);
       expect(stdout).toContain('CodeceptJS'); // feature
       expect(stdout).toContain('glob current dir');
       expect(stdout).toContain('From worker @1_grep print message 1');

test/runner/scenario_stale_test.js

@@ -0,0 +1,22 @@
+const { expect } = require('expect');
+const path = require('path');
+const { exec } = require('child_process');
+
+const runner = path.join(__dirname, '/../../bin/codecept.js');
+const codecept_dir = path.join(__dirname, '/../data/sandbox');
+const codecept_run = `${runner} run`;
+const config_run_config = config => `${codecept_run} --config ${codecept_dir}/${config}`;
+
+describe('Scenario termination check', () => {
+  before(() => {
+    process.chdir(codecept_dir);
+  });
+
+  it('Should always fail and terminate', (done) => {
+    exec(config_run_config('codecept.scenario-stale.js'), (err, stdout) => {
+      expect(stdout).toContain('should not stale scenario error'); // feature
+      expect(err).toBeTruthy();
+      done();
+    });
+  });
+});

test/runner/timeout_test.js

@@ -11,7 +11,7 @@ describe('CodeceptJS Timeouts', function () {
 
   it('should stop test when timeout exceeded', (done) => {
     exec(config_run_config('codecept.conf.js', 'timed out'), (err, stdout) => {
-      console.log(stdout);
+      debug_this_test && console.log(stdout);
       expect(stdout).toContain('Timeout 2s exceeded');
       expect(stdout).toContain('Timeout 1s exceeded');
       expect(err).toBeTruthy();