Skip to content

Conversation

@rjonam
Copy link

@rjonam rjonam commented Jan 21, 2021

Motivation/Description of the PR

Currently Codecept don't have the option to rerun the failed scripts and to run the script with different pattern. This option of --rerun-tests will make it easier to run the failed tests alone and selected tests.

Applicable helpers:

  • [ ✓ ] WebDriver

  • Puppeteer

  • Nightmare

  • REST

  • FileHelper

  • Appium

  • Protractor

  • TestCafe

  • Playwright

Applicable plugins:

  • allure
  • autoDelay
  • autoLogin
  • customLocator
  • pauseOnFail
  • puppeteerCoverage
  • retryFailedStep
  • screenshotOnFail
  • selenoid
  • stepByStepReport
  • wdio

Type of change

  • 🔥 Breaking changes
  • [✓ ] 🚀 New functionality
  • 🐛 Bug fix
  • 📋 Documentation changes/updates
  • ♨️ Hot fix
  • 🔨 Markdown files fix - not related to source code
  • 💅 Polish code

Checklist:

  • Tests have been added
  • Documentation has been added (Run npm run docs)
  • [✓ ] Lint checking (Run npm run lint)
  • [ ✓] Local tests are passed (Run npm test)

@rjonam
Copy link
Author

rjonam commented Jan 30, 2021

Team, Can you please have look on this Pull request and advise. @DavertMik @Georgegriff

Copy link
Contributor

@DavertMik DavertMik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Sorry for the delay in reviewing it.

Unfortunately, I can't merge it as it changes too many core classes for the sake of one feature.
Could you try to rewrite this feature as a plugin?

A plugin can do exactly what you need - hook up on events.
a plugin can also load tests from a file and put them into Mocha instance via MochaFactory

And the best part - plugin does not affect the overall behavior of codeceptjs so it can be merged safely. Could you try to rethink the implementation? After all you are on the right path and this seems like a good solution, but it needs to be transformed to not change the core classes of CodeceptJS

this.emit(event.test.after, repackTest(message.data));
break;
case event.all.after:
failedTestRerun.writeFailedTests(failedTests);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, this looks like breaking the abstraction hierarchy.
this class represents only workers and should know nothing about failedTestRerun


event.dispatcher.on(event.all.after, () => {
// Writes the Failed Test Names In The JSON File For Rerun
failedTestRerun.writeFailedTests(failedTestName);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not a correct place to put this logic
this listener only sets the exit code for failed tests

@rjonam
Copy link
Author

rjonam commented Feb 3, 2021

Thanks a Lot for the review @DavertMik . I will check back and resubmit the code as requested.

@Georgegriff
Copy link
Collaborator

Georgegriff commented Feb 3, 2021

I'm not sure if this is what @DavertMik had in mind, but i've addressed this before with some hacky plugin code, which may not work in all cases, and may be dodgy with workers, might need a cross thread way of storing info about retries, potentially.

module.exports = function(config) {
  const event = codeceptjs.event;
  if (config.enabled) {
    const globalRetries =
      typeof config.retries === "undefined" || config.retries === ""
        ? 2
        : config.retries;
    // we dont retry on really slow tests
    const MINUTE = 1000 * 60;
    const maxTestDuration = parseInt(config.maxTestDuration) || 6 * MINUTE;

    event.dispatcher.on(event.test.started, function(test) {
      test.__retry_plugin_started = Date.now();
    });
    // to do make worker safe
    event.dispatcher.on(event.test.failed, function(test) {
      const duration = Date.now() - test.__retry_plugin_started;
      if (duration < maxTestDuration) {
        test.__retries = true
        test.retries(parseInt(globalRetries));
      }
    });
    event.dispatcher.on(event.test.finished, function(test) {
      if(test.__retries) {
        // we use allure  handy to tag  the retries accordingly
        const allure = codeceptjs.container.plugins('allure');
        if(allure) {
          allure.addLabel('tag', "@FLAKEY");
          allure.addLabel('package', "Flakey Tests");
        }

      }

    });
  }
};

@rjonam
Copy link
Author

rjonam commented Feb 18, 2021

Requested changes are implemented in the below PR :

#2809.

Taking back this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants