fix(cli): stale process (#4367)

kobenguyent · web-flow · commit d3dced129370 · 2024-06-10T06:31:17.000+02:00
diff --git a/lib/cli.js b/lib/cli.js
@@ -186,20 +186,24 @@ class Cli extends Base {
           }
         }
 
-        let stack = err.stack ? err.stack.split('\n') : [];
-        if (stack[0] && stack[0].includes(err.message)) {
-          stack.shift();
-        }
+        try {
+          let stack = err.stack ? err.stack.split('\n') : [];
+          if (stack[0] && stack[0].includes(err.message)) {
+            stack.shift();
+          }
 
-        if (output.level() < 3) {
-          stack = stack.slice(0, 3);
-        }
+          if (output.level() < 3) {
+            stack = stack.slice(0, 3);
+          }
 
-        err.stack = `${stack.join('\n')}\n\n${output.colors.blue(log)}`;
+          err.stack = `${stack.join('\n')}\n\n${output.colors.blue(log)}`;
 
-        // clone err object so stack trace adjustments won't affect test other reports
-        test.err = err;
-        return test;
+          // clone err object so stack trace adjustments won't affect test other reports
+          test.err = err;
+          return test;
+        } catch (e) {
+          throw Error(e);
+        }
       });
 
       const originalLog = Base.consoleLog;
diff --git a/lib/command/gherkin/init.js b/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}`],
   };
 
diff --git a/lib/plugin/retryTo.js b/lib/plugin/retryTo.js
@@ -1,5 +1,4 @@
 const recorder = require('../recorder');
-const store = require('../store');
 const { debug } = require('../output');
 
 const defaultConfig = {
@@ -73,49 +72,58 @@ const defaultConfig = {
  * const retryTo = codeceptjs.container.plugins('retryTo');
  * ```
  *
-*/
+ */
 module.exports = function (config) {
   config = Object.assign(defaultConfig, config);
+  function retryTo(callback, maxTries, pollInterval = config.pollInterval) {
+    return new Promise((done, reject) => {
+      let tries = 1;
 
-  if (config.registerGlobal) {
-    global.retryTo = retryTo;
-  }
-  return retryTo;
+      function handleRetryException(err) {
+        recorder.throw(err);
+        reject(err);
+      }
 
-  function retryTo(callback, maxTries, pollInterval = undefined) {
-    let tries = 1;
-    if (!pollInterval) pollInterval = config.pollInterval;
-
-    let err = null;
-
-    return new Promise((done) => {
       const tryBlock = async () => {
+        tries++;
         recorder.session.start(`retryTo ${tries}`);
-        await callback(tries);
+        try {
+          await callback(tries);
+        } catch (err) {
+          handleRetryException(err);
+        }
+
+        // Call done if no errors
         recorder.add(() => {
           recorder.session.restore(`retryTo ${tries}`);
           done(null);
         });
-        recorder.session.catch((e) => {
-          err = e;
+
+        // Catch errors and retry
+        recorder.session.catch((err) => {
           recorder.session.restore(`retryTo ${tries}`);
-          tries++;
           if (tries <= maxTries) {
             debug(`Error ${err}... Retrying`);
-            err = null;
-
-            recorder.add(`retryTo ${tries}`, () => setTimeout(tryBlock, pollInterval));
+            recorder.add(`retryTo ${tries}`, () =>
+              setTimeout(tryBlock, pollInterval)
+            );
           } else {
-            done(null);
+            // if maxTries reached
+            handleRetryException(err);
           }
         });
       };
 
-      recorder.add('retryTo', async () => {
-        tryBlock();
+      recorder.add('retryTo', tryBlock).catch(err => {
+        console.error('An error occurred:', err);
+        done(null);
       });
-    }).then(() => {
-      if (err) recorder.throw(err);
     });
   }
+
+  if (config.registerGlobal) {
+    global.retryTo = retryTo;
+  }
+
+  return retryTo;
 };
diff --git a/lib/scenario.js b/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));
diff --git a/test/acceptance/retryTo_test.js b/test/acceptance/retryTo_test.js
@@ -14,3 +14,23 @@ Scenario('retryTo works with non await steps @plugin', async () => {
     if (tryNum < 3) I.waitForVisible('.nothing', 1);
   }, 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');
+  }, 3);
+});
+
+Scenario('Should succeed at the third attempt @plugin', async () => {
+  await retryTo(async (tryNum) => {
+    if (tryNum < 2) throw new Error('Custom pluginRetryTo Error');
+  }, 3);
+});
diff --git a/test/data/sandbox/codecept.scenario-stale.js b/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',
+};
diff --git a/test/data/sandbox/test.scenario-stale.js b/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);
diff --git a/test/plugin/plugin_test.js b/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');
@@ -61,4 +70,17 @@ describe('CodeceptJS plugin', function () {
       done();
     });
   });
+
+  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();
+      }
+    );
+  });
 });
diff --git a/test/runner/run_workers_test.js b/test/runner/run_workers_test.js
diff --git a/test/runner/scenario_stale_test.js b/test/runner/scenario_stale_test.js
diff --git a/test/runner/timeout_test.js b/test/runner/timeout_test.js
diff --git a/test/unit/plugin/retryto_test.js b/test/unit/plugin/retryto_test.js

Original file line number	Diff line number	Diff line change
`@@ -70,7 +70,7 @@ module.exports = function (genPath) {`
`70`	`70`	`}`
`71`	`71`
`72`	`72`	`config.gherkin = {`
`73`		`- features: './features/*.feature',`
	`73`	`+ features: "./features/*.feature",`
`74`	`74`	steps: [`./step_definitions/steps.${extension}`],
`75`	`75`	`};`
`76`	`76`