Repository: Codeception/CodeceptJS
Pull request #1897 (2019-09-23 20:31:44)

Improved retryFailedStep plugin
Got some improvements to retryFailedStep plugin.

* is activated only for tests
* contains blacklist of steps to ignore
* can be turned off for a specific test 

codacy-bot: 
![Codacy](https://app.codacy.com/assets/images/favicon.png) Here is an overview of what got changed by this pull request:


```diff

Issues
======
- Added 1
           

Complexity increasing per file
==============================
- test/data/sandbox/flaky_test.retryFailed.js  1
- lib/plugin/retryFailedStep.js  1
         

Clones added
============
- test/unit/plugin/retryFailedStep_test.js  2
- test/runner/interface_test.js  2
         
```


See the complete overview on [Codacy](https://app.codacy.com/app/DavertMik/CodeceptJS/pullRequest?prid=4205519&bid=14383025)
        
codacy-bot: 
![Codacy](https://app.codacy.com/assets/images/favicon.png) Here is an overview of what got changed by this pull request:


```diff

Issues
======
- Added 1
           

Complexity increasing per file
==============================
- lib/plugin/retryFailedStep.js  4
- test/data/sandbox/flaky_test.retryFailed.js  1
         

Clones added
============
- test/unit/plugin/retryFailedStep_test.js  8
- test/runner/interface_test.js  2
         
```


See the complete overview on [Codacy](https://app.codacy.com/app/DavertMik/CodeceptJS/pullRequest?prid=4205519&bid=14383025)
        

Patch:
diff --git a/bin/codecept.js b/bin/codecept.js
index 2616e6f1..4bb63c37 100755
--- a/bin/codecept.js
+++ b/bin/codecept.js
@@ -125,6 +125,7 @@ program.command('run-workers <workers>')
   .option('-c, --config [file]', 'configuration file to be used')
   .option('-g, --grep <pattern>', 'only run tests matching <pattern>')
   .option('-o, --override [value]', 'override current config options')
+  .option('--suites', 'parallel execution of suites not single tests')
   .option('--debug', 'output additional information')
   .option('--verbose', 'output internal logging information')
   .option('--features', 'run only *.feature files and skip tests')
diff --git a/docs/parallel.md b/docs/parallel.md
index 7340bd0b..71acb499 100644
--- a/docs/parallel.md
+++ b/docs/parallel.md
@@ -24,6 +24,12 @@ This command is similar to `run`, however, steps output can't be shown in worker
 
 Each worker spins an instance of CodeceptJS, executes a group of tests, and sends back report to the main process.
 
+By default the tests are assigned one by one to the avaible workers this may lead to multiple execution of `BeforeSuite()`. Use the option `--suites` to assigne the suites one by one to the workers.
+
+```sh
+npx codeceptjs run-workers --suites 2
+```
+
 
 ## Multiple Browsers Execution
 
diff --git a/lib/command/run-workers.js b/lib/command/run-workers.js
index cfd9b83d..2309c803 100644
--- a/lib/command/run-workers.js
+++ b/lib/command/run-workers.js
@@ -50,7 +50,7 @@ module.exports = function (workers, options) {
   codecept.init(testRoot);
   codecept.loadTests();
 
-  const groups = createGroupsOfTests(codecept, numberOfWorkers);
+  const groups = createGroupsOfTests(codecept, numberOfWorkers, options.suites);
 
   stats.start = new Date();
 
@@ -60,17 +60,15 @@ module.exports = function (workers, options) {
 
   // run bootstrap all
   runHook(config.bootstrapAll, () => {
-    const workerList = createWorkers(groups, config, options, testRoot);
+    const workerList = createWorkers(groups, options, testRoot);
     workerList.forEach(worker => assignWorkerMethods(worker, groups.length));
   }, 'bootstrapAll');
 
-  function createWorkers(groups, config, options, testRoot) {
-    delete config.mocha;
+  function createWorkers(groups, options, testRoot) {
     const workers = groups.map((tests, workerIndex) => {
       workerIndex++;
       return new Worker(pathToWorker, {
         workerData: {
-          config: simplifyObject(config, { objects: false, underscores: false }),
           options: simplifyObject(options),
           tests,
           testRoot,
@@ -121,7 +119,7 @@ function printResults() {
   stats.duration = stats.end - stats.start;
   output.print();
   if (stats.passes && !errors.length) {
-    output.result(stats.passes, stats.failures, 0, stats.duration);
+    output.result(stats.passes, stats.failures, 0, `${stats.duration / 1000}s`);
   }
   if (stats.failures) {
     output.print();
@@ -130,28 +128,59 @@ function printResults() {
   }
 }
 
-function createGroupsOfTests(codecept, numberOfGroups) {
+function createGroupsOfTests(codecept, numberOfGroups, preserveSuites) {
   const files = codecept.testFiles;
   const mocha = Container.mocha();
   mocha.files = files;
   mocha.loadFiles();
 
   const groups = [];
+  for (let i = 0; i < numberOfGroups; i++) {
+    groups[i] = [];
+  }
+  if (preserveSuites) {
+    return groupSuites(mocha.suite, groups);
+  }
+  return groupTests(mocha.suite, groups);
+}
+
+function groupTests(suite, groups) {
   let groupCounter = 0;
 
-  mocha.suite.eachTest((test) => {
-    const i = groupCounter % numberOfGroups;
-    if (groups[i] === undefined) groups[i] = [];
+  suite.eachTest((test) => {
+    const i = groupCounter % groups.length;
     if (test) {
       const { id } = test;
       groups[i].push(id);
       groupCounter++;
     }
   });
+  return groups;
+}
 
+function groupSuites(suite, groups) {
+  suite.suites.forEach((suite) => {
+    const i = indexOfSmallestElement(groups);
+    suite.tests.forEach((test) => {
+      if (test) {
+        const { id } = test;
+        groups[i].push(id);
+      }
+    });
+  });
   return groups;
 }
 
+function indexOfSmallestElement(groups) {
+  let i = 0;
+  for (let j = 1; j < groups.length; j++) {
+    if (groups[j - 1].length > groups[j].length) {
+      i = j;
+    }
+  }
+  return i;
+}
+
 function appendStats(newStats) {
   stats.passes += newStats.passes;
   stats.failures += newStats.failures;
@@ -159,7 +188,7 @@ function appendStats(newStats) {
 }
 
 function repackTest(test) {
-  test = Object.assign(new Test(test.title || '', () => {}), test);
+  test = Object.assign(new Test(test.title || '', () => { }), test);
   test.parent = Object.assign(new Suite(test.parent.title), test.parent);
   return test;
 }
@@ -169,22 +198,13 @@ function repackSuite(suite) {
 }
 
 
-function simplifyObject(object, remove = {}) {
-  const defaultRemove = {
-    objects: true,
-    functions: true,
-    underscores: true,
-  };
-
-  remove = Object.assign(defaultRemove, remove);
-
-  let tempObj = Object.keys(object);
-  if (remove.underscores) tempObj = tempObj.filter(k => k.indexOf('_') !== 0);
-  if (remove.functions) tempObj = tempObj.filter(k => typeof object[k] !== 'function');
-  if (remove.objects) tempObj = tempObj.filter(k => typeof object[k] !== 'object');
-
-  return tempObj.reduce((obj, key) => {
-    obj[key] = object[key];
-    return obj;
-  }, {});
+function simplifyObject(object) {
+  return Object.keys(object)
+    .filter(k => k.indexOf('_') !== 0)
+    .filter(k => typeof object[k] !== 'function')
+    .filter(k => typeof object[k] !== 'object')
+    .reduce((obj, key) => {
+      obj[key] = object[key];
+      return obj;
+    }, {});
 }
diff --git a/lib/helper/Puppeteer.js b/lib/helper/Puppeteer.js
index a54280ce..9464f344 100644
--- a/lib/helper/Puppeteer.js
+++ b/lib/helper/Puppeteer.js
@@ -218,7 +218,7 @@ class Puppeteer extends Helper {
 
   async _before() {
     recorder.retry({
-      retries: 3,
+      retries: 5,
       when: err => err.message.indexOf('context') > -1, // ignore context errors
     });
     if (this.options.restart && !this.options.manualStart) return this._startBrowser();
diff --git a/lib/plugin/retryFailedStep.js b/lib/plugin/retryFailedStep.js
index 7c3dd207..40c56fa9 100644
--- a/lib/plugin/retryFailedStep.js
+++ b/lib/plugin/retryFailedStep.js
@@ -3,6 +3,15 @@ const recorder = require('../recorder');
 
 const defaultConfig = {
   retries: 5,
+  defaultIgnoredSteps: [
+    'amOnPage',
+    'wait*',
+    'send*',
+    'execute*',
+    'run*',
+    'have*',
+  ],
+  ignoredSteps: [],
 };
 
 /**
@@ -22,7 +31,7 @@ const defaultConfig = {
  * Run tests with plugin enabled:
  *
  * ```
- * codeceptjs run --plugins retryFailedStep
+ * npx codeceptjs run --plugins retryFailedStep
  * ```
  *
  * ##### Configuration:
@@ -33,15 +42,53 @@ const defaultConfig = {
  * * `minTimeout` - The number of milliseconds before starting the first retry. Default is 1000.
  * * `maxTimeout` - The maximum number of milliseconds between two retries. Default is Infinity.
  * * `randomize` - Randomizes the timeouts by multiplying with a factor between 1 to 2. Default is false.
+ * * `defaultIgnoredSteps` - an array of steps to be ignored for retry. Includes:
+ *     * `amOnPage`
+ *     * `wait*`
+ *     * `send*`
+ *     * `execute*`
+ *     * `run*`
+ *     * `have*`
+ * * `ignoredSteps` - an array for custom steps to ignore on retry. Use it to append custom steps to ignored list.
+ * You can use step names or step prefixes ending with `*`. As such, `wait*` will match all steps starting with `wait`.
+ * To append your own steps to ignore list - copy and paste a default steps list. Regexp values are accepted as well.
  *
- * This plugin is very basic so it's recommended to improve it to match your custom needs.
+ * ##### Example
+ *
+ * ```js
+ * plugins: {
+ *     retryFailedStep: {
+ *         enabled: true,
+ *         ignoreSteps: [
+ *           'scroll*', // ignore all scroll steps
+ *           /Cookie/, // ignore all steps with a Cookie in it (by regexp)
+ *         ]
+ *     }
+ * }
+ * ```
+ *
+ * ##### Disable Per Test
+ *
+ * This plugin can be disabled per test. In this case you will need to stet `I.retry()` to all flaky steps:
+ *
+ * Use scenario configuration to disable plugin for a test
+ *
+ * ```js
+ * Scenario('scenario tite', () => {
+ *    // test goes here
+ * }).config(test => test.disableRetryFailedStep = true)
+ * ```
  *
  */
 module.exports = (config) => {
   config = Object.assign(defaultConfig, config);
+  config.ignoredSteps = config.ignoredSteps.concat(config.defaultIgnoredSteps);
   const customWhen = config.when;
 
+  let enableRetry = false;
+
   const when = (err) => {
+    if (!enableRetry) return;
     const store = require('../store');
     if (store.debugMode) return false;
     if (customWhen) return customWhen(err);
@@ -49,7 +96,22 @@ module.exports = (config) => {
   };
   config.when = when;
 
+  event.dispatcher.on(event.step.started, (step) => {
+    // if a step is ignored - return
+    for (const ignored of config.ignoredSteps) {
+      if (step.name === ignored) return;
+      if (ignored instanceof RegExp && step.name.match(ignored)) return;
+      if (ignored.indexOf('*') && step.name.startsWith(ignored.slice(0, -1))) return;
+    }
+    enableRetry = true; // enable retry for a step
+  });
+
+  event.dispatcher.on(event.step.finished, (step) => {
+    enableRetry = false;
+  });
+
   event.dispatcher.on(event.test.before, (test) => {
+    if (test && test.disableRetryFailedStep) return; // disable retry when a test is not active
     recorder.retry(config);
   });
 };
diff --git a/lib/plugin/screenshotOnFail.js b/lib/plugin/screenshotOnFail.js
index b8792a9a..f8fa45a6 100644
--- a/lib/plugin/screenshotOnFail.js
+++ b/lib/plugin/screenshotOnFail.js
@@ -88,7 +88,7 @@ module.exports = function (config) {
         fileName = fileName.substr(0, (fileName.indexOf('{') - 3)).trim();
       }
       if (test.ctx && test.ctx.test && test.ctx.test.type === 'hook') fileName = clearString(`${test.title}_${test.ctx.test.title}`);
-      if (options.uniqueScreenshotNames) {
+      if (options.uniqueScreenshotNames && test) {
         const uuid = test.uuid || test.ctx.test.uuid || Math.floor(new Date().getTime() / 1000);
         fileName = `${fileName.substring(0, 10)}_${uuid}.failed.png`;
       } else {
diff --git a/lib/utils.js b/lib/utils.js
index 921897ea..bf3a93d0 100644
--- a/lib/utils.js
+++ b/lib/utils.js
@@ -140,6 +140,7 @@ module.exports.chunkArray = function (arr, chunk) {
 };
 
 module.exports.clearString = function (str) {
+  if (!str) return '';
   /* Replace forbidden symbols in string
    */
   return str


Test patch:
diff --git a/test/data/sandbox/codecept.retryFailed.json b/test/data/sandbox/codecept.retryFailed.json
new file mode 100644
index 00000000..2b3e78ba
--- /dev/null
+++ b/test/data/sandbox/codecept.retryFailed.json
@@ -0,0 +1,19 @@
+{
+  "tests": "./*_test.retryFailed.js",
+  "timeout": 10000,
+  "output": "./output",
+  "helpers": {
+    "Retry": {
+      "require": "./retry_helper.js"
+    }
+  },
+  "plugins": {
+    "retryFailedStep": {
+      "enabled": true,
+      "retries": 5
+    }
+  },
+  "bootstrap": false,
+  "mocha": {},
+  "name": "sandbox"
+}
\ No newline at end of file
diff --git a/test/data/sandbox/flaky_test.retryFailed.js b/test/data/sandbox/flaky_test.retryFailed.js
new file mode 100644
index 00000000..d605d2e9
--- /dev/null
+++ b/test/data/sandbox/flaky_test.retryFailed.js
@@ -0,0 +1,35 @@
+const assert = require('assert');
+
+let tries = 0;
+
+Feature('Retry');
+
+Scenario('auto repeat failing step @test1', async (I) => {
+  tries++;
+  await I.failWhen(() => {
+    tries++;
+    return tries < 5;
+  });
+  assert.equal(tries, 5);
+  console.log(`[T] Retries: ${tries}`);
+});
+
+Scenario('no repeat for waiter @test2', async (I) => {
+  await I.waitForFail(() => {
+    tries++;
+    return tries < 5;
+  });
+  assert.equal(tries, 1);
+});
+
+Scenario('no retries if disabled per test @test3', async (I) => {
+  await I.failWhen(() => {
+    tries++;
+    return tries < 5;
+  });
+  assert.equal(tries, 1);
+}).config(test => test.disableRetryFailedStep = true);
+
+After(() => {
+  console.log(`[T] Retries: ${tries}`);
+});
diff --git a/test/data/sandbox/retry_helper.js b/test/data/sandbox/retry_helper.js
index 1150550b..b6081c86 100644
--- a/test/data/sandbox/retry_helper.js
+++ b/test/data/sandbox/retry_helper.js
@@ -4,6 +4,10 @@ class Retry extends Helper {
   failWhen(fn) {
     if (fn()) throw new Error('ups, error');
   }
+
+  waitForFail(fn) {
+    if (fn()) throw new Error('ups, error');
+  }
 }
 
 module.exports = Retry;
diff --git a/test/runner/interface_test.js b/test/runner/interface_test.js
index 38dc706d..f2408fef 100644
--- a/test/runner/interface_test.js
+++ b/test/runner/interface_test.js
@@ -36,6 +36,35 @@ describe('CodeceptJS Interface', () => {
   });
 
 
+  it('should use retryFailedStep plugin for failed steps', (done) => {
+    exec(`${config_run_config('codecept.retryFailed.json')} --grep @test1`, (err, stdout, stderr) => {
+      stdout.should.include('Retry'); // feature
+      stdout.should.include('Retries: 5'); // test name
+      assert(!err);
+      done();
+    });
+  });
+
+  it('should not retry wait* steps in retryFailedStep plugin', (done) => {
+    exec(`${config_run_config('codecept.retryFailed.json')} --grep @test2`, (err, stdout, stderr) => {
+      stdout.should.include('Retry'); // feature
+      stdout.should.not.include('Retries: 5');
+      stdout.should.include('Retries: 1');
+      assert(err);
+      done();
+    });
+  });
+
+  it('should not retry steps if retryFailedStep plugin disabled', (done) => {
+    exec(`${config_run_config('codecept.retryFailed.json')} --grep @test3`, (err, stdout, stderr) => {
+      stdout.should.include('Retry'); // feature
+      stdout.should.not.include('Retries: 5');
+      stdout.should.include('Retries: 1');
+      assert(err);
+      done();
+    });
+  });
+
   it('should include grep option tests', (done) => {
     exec(config_run_config('codecept.grep.json'), (err, stdout, stderr) => {
       stdout.should.include('Got login davert and password'); // feature
diff --git a/test/unit/plugin/retryFailedStep_test.js b/test/unit/plugin/retryFailedStep_test.js
index 9ef0fc6b..a5e0335f 100644
--- a/test/unit/plugin/retryFailedStep_test.js
+++ b/test/unit/plugin/retryFailedStep_test.js
@@ -16,9 +16,15 @@ describe('retryFailedStep', () => {
     });
     recorder.start();
   });
+
+  afterEach(() => {
+    event.dispatcher.emit(event.step.finished, { });
+  });
+
   it('should retry failed step', async () => {
     retryFailedStep({ retries: 2, minTimeout: 1 });
-    event.dispatcher.emit(event.test.before);
+    event.dispatcher.emit(event.test.before, {});
+    event.dispatcher.emit(event.step.started, { name: 'click' });
 
     let counter = 0;
     recorder.add(() => {
@@ -31,10 +37,10 @@ describe('retryFailedStep', () => {
   });
   it('should not retry within', async () => {
     retryFailedStep({ retries: 1, minTimeout: 1 });
-    event.dispatcher.emit(event.test.before);
+    event.dispatcher.emit(event.test.before, {});
 
     let counter = 0;
-
+    event.dispatcher.emit(event.step.started, { name: 'click' });
     try {
       within('foo', () => {
         recorder.add(() => {
@@ -50,10 +56,81 @@ describe('retryFailedStep', () => {
     // expects to retry only once
     counter.should.equal(2);
   });
+
+  it('should not retry steps with wait*', async () => {
+    retryFailedStep({ retries: 2, minTimeout: 1 });
+    event.dispatcher.emit(event.test.before, {});
+
+    let counter = 0;
+    event.dispatcher.emit(event.step.started, { name: 'waitForElement' });
+    try {
+      recorder.add(() => {
+        counter++;
+        if (counter < 3) {
+          throw new Error();
+        }
+      });
+      await recorder.promise();
+    } catch (e) {
+      recorder.catchWithoutStop((err) => {
+      });
+    }
+
+    counter.should.equal(1);
+    // expects to retry only once
+  });
+
+  it('should not retry steps with amOnPage', async () => {
+    retryFailedStep({ retries: 2, minTimeout: 1 });
+    event.dispatcher.emit(event.test.before, {});
+
+    let counter = 0;
+    event.dispatcher.emit(event.step.started, { name: 'amOnPage' });
+    try {
+      recorder.add(() => {
+        counter++;
+        if (counter < 3) {
+          throw new Error();
+        }
+      });
+      await recorder.promise();
+    } catch (e) {
+      recorder.catchWithoutStop((err) => {
+      });
+    }
+
+    counter.should.equal(1);
+    // expects to retry only once
+  });
+
+
+  it('should add custom steps to ignore', async () => {
+    retryFailedStep({ retries: 2, minTimeout: 1, ignoredSteps: ['somethingNew*'] });
+    event.dispatcher.emit(event.test.before, {});
+
+    let counter = 0;
+    event.dispatcher.emit(event.step.started, { name: 'somethingNew' });
+    try {
+      recorder.add(() => {
+        counter++;
+        if (counter < 3) {
+          throw new Error();
+        }
+      });
+      await recorder.promise();
+    } catch (e) {
+      recorder.catchWithoutStop((err) => {
+      });
+    }
+
+    counter.should.equal(1);
+    // expects to retry only once
+  });
+
   it('should not retry session', async () => {
     retryFailedStep({ retries: 1, minTimeout: 1 });
-    event.dispatcher.emit(event.test.before);
-
+    event.dispatcher.emit(event.test.before, {});
+    event.dispatcher.emit(event.step.started, { name: 'click' });
     let counter = 0;
 
     try {