-
-
Notifications
You must be signed in to change notification settings - Fork 752
Implementation of reRunFailedTest #2809
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Team, Can you please have look on this Pull request and advise. @DavertMik @Georgegriff |
|
|
Theres a lot to review here i'm not best placed to understand it. I do notice that you are still providing a cli option which seems a little wrong for a plugin? @DavertMik might have thoughts here |
| | failed | Only executes the failed/custom(selective) scripts present in failedCases.json | | ||
| ```js | ||
| npx codeceptjs run --failed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should be able to read the Plugin values from the config plugins object. You might not need to provide CLI option unless it's a core codeceptjs feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gkushang, This option is more over to run the failed tests from the previous run to support both runner options sequential and workers. this would be a core feature as many of the developers were looking for this feature similar option in testng.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@senthillkumar The reason I asked because I see you also have a plugin reRunFailedTest.js defined so I was wondering the use case of the Plugin. If this becomes the Core feature then you wont' need a plugin or vice-versa (if plugin is enabled then it'd run failed tests automatically)
** This is indeed a much needed feature though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would the user need to execute one more command to run
failedtests or thefailedtests will run automatically if plugin is enabled? e.g.
codeceptjs run>> run all tests and re-run failed tests automatically at the end of execution
OR
codeceptjs run && codeceptjs run -p <this-plugin>>> execute two commands?
There are 2 features in the rerun test plugin.
- Manually run the failed test cases using run/run-workers command with --failed option as mentioned below.
codeceptjs run --failed or codeceptjs run-workers --failed - Enabling the Rerunfailed test Plugin would auto retry the failed test after the execution without passing any argument.
reRunFailedTest: { enabled: true, autoRetry: true, require: './plugins/reRunFailedTest', },
Advantages:
As i have 50 test cases running in parallel out of 50 , 10 were failed. i want to autoretry / manual retry the failed test case in the same/different environment. As rerun functionality is available as core feature in the codeceptjs, but that will rerun the scripts sequentially and if the case is failed in between it will retry the failed one then continue the rest of the scripts. rerun mode is more time consuming.
Let me know your thoughts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gkushang/@DavertMik , can you please review the changes
|
@PeterNgTr/ @DavertMik, Could you please help reviewing the changes for failed test case rerun functionality from the previous run and also auto retry the failed case post execution. |
|
Would the user need to execute one more command to run
|
DavertMik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I still can't merge this.
This breaks abstraction principle.
the core should know nothing about the plugin.
so you can't import plugin into runWorkers or run
What I recommend is the following. Change the current test list by running
codecept.loadTests() and passing in a list of failed tests as parameter. This can be done from a plugin as well.
Ideally, this PR should include only changes to plugin file and a test.
| .option('--tests', 'run only JS test files and skip features') | ||
| .option('-p, --plugins <k=v,k2=v2,...>', 'enable plugins, comma-separated') | ||
|
|
||
| .option('--failed', 'to run failed/custom Tests') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this option (as it affects core) and use environment variable instead:
RUN_FAILED=true npx codeceptjs run
| await workers.run(); | ||
| } finally { | ||
| await workers.teardownAll(); | ||
| if (Config.plugins && Config.plugins.reRunFailedTest && Config.plugins.reRunFailedTest.enabled === true) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah, this is unacceptable. It is a plugin so run-workers should not be affected by it.
| } = require('./utils'); | ||
| const Config = require('../config'); | ||
| const Codecept = require('../codecept'); | ||
| const reRunFailedTest = require('../plugin/reRunFailedTest'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here. You can't import a plugin into a core.
|
@DavertMik I know this is closed but still wants to say this could have been a great feature of CodeceptJS! I hope authors act on the suggestions and repen … |

Motivation/Description of the PR
Applicable helpers:
Applicable plugins:
Type of change
Checklist:
npm run docs)npm run lint)npm test)