Skip to content
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

fix: no error thrown from rerun script #4494

Merged
merged 4 commits into from
Sep 17, 2024

Conversation

lin-brian-l
Copy link
Contributor

@lin-brian-l lin-brian-l commented Sep 13, 2024

Motivation/Description of the PR

  • Description of this PR, which problem it solves

Our team observed that using codeceptjs run-rerun would only throw an exit code of 1 if the last test failed, making it difficult to reliably determine if a test in the suite actually was flaky. After some investigating, we determined that it's because the catch statement in run-rerun.js was never being called due to rerun.js never actually throwing the error.

  • Resolves #issueId (if applicable).

Applicable helpers:

  • Playwright
  • Puppeteer
  • WebDriver
  • REST
  • FileHelper
  • Appium
  • TestCafe

Applicable plugins:

  • allure
  • autoDelay
  • autoLogin
  • customLocator
  • pauseOnFail
  • coverage
  • retryFailedStep
  • screenshotOnFail
  • selenoid
  • stepByStepReport
  • stepTimeout
  • wdio
  • subtitles

Type of change

  • 🔥 Breaking changes
  • 🚀 New functionality
  • 🐛 Bug fix
  • 🧹 Chore
  • 📋 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)

@lin-brian-l lin-brian-l changed the title ENG-1059: Throw error Throw error from rerun script Sep 13, 2024
Comment on lines -20 to -23
function processError(err) {
printError(err)
process.exit(1)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function was unused.

@@ -96,4 +96,18 @@ describe('run-rerun command', () => {
},
)
})

it('should throw exit code 1 if all tests were supposed to pass', (done) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've verified that this test fails without my changes.

@lin-brian-l lin-brian-l marked this pull request as ready for review September 13, 2024 20:45
@lin-brian-l lin-brian-l marked this pull request as draft September 13, 2024 21:56
@lin-brian-l lin-brian-l marked this pull request as ready for review September 13, 2024 22:08
This reverts commit 6cba723.
@kobenguyent kobenguyent changed the title Throw error from rerun script fix: no error thrown from rerun script Sep 15, 2024
@lin-brian-l
Copy link
Contributor Author

@kobenguyent thanks for taking a look at this! Not to rush, but do you know when this would be ready to merge? I've made a similar PR for a fork on my team's repository but we were wondering if we should just wait for this PR to be merged instead. Let me know if you'd like for me to address any feedback!

@kobenguyent
Copy link
Collaborator

@lin-brian-l we are planning to release very soon. Thanks for your contribution!

@kobenguyent kobenguyent merged commit 48a02c4 into codeceptjs:3.x Sep 17, 2024
12 of 14 checks passed
@kobenguyent kobenguyent mentioned this pull request Sep 17, 2024
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.

2 participants