Skip to content

fix: hook exit code #5058

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

Open
wants to merge 6 commits into
base: 3.x
Choose a base branch
from
Open

fix: hook exit code #5058

wants to merge 6 commits into from

Conversation

kobenguyent
Copy link
Collaborator

@kobenguyent kobenguyent commented Aug 5, 2025

Motivation/Description of the PR

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)

@kobenguyent kobenguyent requested a review from Copilot August 5, 2025 16:26
Copilot

This comment was marked as outdated.

Copilot

This comment was marked as outdated.

kobenguyent and others added 2 commits August 5, 2025 21:08
@kobenguyent kobenguyent requested a review from Copilot August 5, 2025 19:08
Copilot

This comment was marked as outdated.

@kobenguyent kobenguyent requested a review from Copilot August 5, 2025 19:13
Copilot

This comment was marked as outdated.

@kobenguyent kobenguyent requested a review from Copilot August 5, 2025 19:17
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes hook exit code handling by ensuring that error reporting is consistent across all hook types. The change ensures that suiteTestFailedHookError is called for 'after' and 'afterSuite' hooks when errors occur, which was previously missing for these hook types.

  • Adds suiteTestFailedHookError calls for 'after' and 'afterSuite' hooks
  • Restructures conditional blocks for better readability with proper bracing
  • Maintains existing event emission behavior for each hook type

if (hookName == 'before' || hookName == 'beforeSuite') suiteTestFailedHookError(suite, err, hookName)
if (hookName === 'after') suite.eachTest(test => event.emit(event.test.after, test))
if (hookName === 'afterSuite') event.emit(event.suite.after, suite)
if (hookName === 'before' || hookName === 'beforeSuite') {
Copy link
Preview

Copilot AI Aug 5, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider using a switch statement or array includes for better readability when checking multiple hook types, especially as this pattern is repeated throughout the function.

Suggested change
if (hookName === 'before' || hookName === 'beforeSuite') {
if (['before', 'beforeSuite'].includes(hookName)) {

Copilot uses AI. Check for mistakes.

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.

After and AfterSuite hooks return exit code 0 on failure instead of 1, incorrectly indicating success
1 participant