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.

Sign up for GitHub

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

Jump to bottom

doc: clarify after and afterEach hooks execution #51523

Merged
merged 5 commits into from
Feb 3, 2024
Merged

doc: clarify after and afterEach hooks execution #51523

merged 5 commits into from
Feb 3, 2024

Conversation

ognjenjevremovic
Copy link
Contributor

Summary

This pull request addresses Issue #50901 , which highlights a documentation clarification regarding the execution of the after and afterEach hooks in the test runner.

Changes Made

after Hook Documentation:

  • Clarified that the after hook is executed once after all tests in a suite, irrespective of test success or failure.
  • Replaced instances of "note that" with more direct language for enhanced clarity.

afterEach Hook Documentation:

  • updated the documentation to explicitly state that the afterEach hook is executed after each individual test, regardless of test outcomes.
  • Improved clarity by replacing occurrences of "note that" with more straightforward language.

Additional Notes

The changes adhere to the Node.js documentation style guide.
Linting warnings related to the phrase "note that" have been addressed for improved documentation clarity.

Related Issues

Fixes: Issue #50901

@ognjenjevremovic
doc: clarify execution of after hook on test suite completion
1a23c68 
The `after` hook now explicitly mentions that it is executed once after
all the tests in a test suite have completed, regardless of whether the
tests passed or failed. This ensures that cleanup tasks or actions
specified in the after hook are guaranteed to run.

Refs: nodejs#50901
@ognjenjevremovic
doc: clarify execution of afterEach hook after each test
19da229 
The `afterEach` hook is now documented to be executed after each
individual test in a test suite, irrespective of whether the test passed
or failed. Developers can rely on this hook for performing cleanup or
resetting state after each test.

Refs: nodejs#50901
@ognjenjevremovic
doc: improve clarity in after and afterEach hook documentation
669ccaa 
Refined the language in the documentation for the `after` and
`afterEach`
hooks to enhance clarity. Replaced instances of "note that" with more
direct and concise language for better readability.

Refs: nodejs#50901
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. test_runner labels Jan 19, 2024
@ognjenjevremovic ognjenjevremovic mentioned this pull request Jan 19, 2024
Closed
marco-ippolito
marco-ippolito reviewed Jan 19, 2024
View reviewed changes
doc/api/test.md Outdated
Comment on lines 1423 to 1427
**Note:** The `after` hook is executed once after all the tests
in a test suite have completed, regardless of whether the tests
passed or failed. This hook is useful for performing cleanup tasks
or actions that should occur after the entire test suite has been run.
The `after` hook is guaranteed to run, even if tests within the suite fail.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
**Note:** The `after` hook is executed once after all the tests
in a test suite have completed, regardless of whether the tests
passed or failed. This hook is useful for performing cleanup tasks
or actions that should occur after the entire test suite has been run.
The `after` hook is guaranteed to run, even if tests within the suite fail.
**Note:** The `after` hook is guaranteed to run, even if tests within the suite fail.

marco-ippolito
marco-ippolito reviewed Jan 19, 2024
View reviewed changes
doc/api/test.md Outdated
Comment on lines 1482 to 1486
**Note:** The `afterEach` hook is executed after each individual test
in a test suite, regardless of whether the test passed or failed.
This hook is commonly used for cleaning up resources or resetting state
between tests. The `afterEach` hook is guaranteed to run after every test,
even if any of the tests fail.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
**Note:** The `afterEach` hook is executed after each individual test
in a test suite, regardless of whether the test passed or failed.
This hook is commonly used for cleaning up resources or resetting state
between tests. The `afterEach` hook is guaranteed to run after every test,
even if any of the tests fail.
**Note:** The `afterEach` hook is guaranteed to run after every test,
even if any of the tests fail.

@ognjenjevremovic
doc: improve clarity in after and afterEach hook documentation
8a1bedd 
Refined the documentation for the `after` and `afterEach` hooks to
enhance clarity. Removed redundant information and retained the
essential details. The revised note now explicitly states that the
`after` and `afterEach` hooks are guaranteed to run, even if tests
within the suite fail.

Refs: nodejs#50901
@ognjenjevremovic
Copy link
Contributor Author

Hey @marco-ippolito thank you for your prompt review! 🚀

I apologize for a slight delay on my end.
I have removed redundant information and retained the essential details in the test runner docs for after and afterEach hooks based on your suggestions 🙂

marco-ippolito
marco-ippolito approved these changes Jan 20, 2024
View reviewed changes
Copy link
Member

@marco-ippolito marco-ippolito left a comment

Choose a reason for hiding this comment

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

lgtm

@ognjenjevremovic
doc: address line length linting error in documentation
4311384 
Adjusted line lengths in the documentation to adhere to the maximum
allowed limit of 80 characters. This change resolves the linting error.

Refs: nodejs#50901
@ognjenjevremovic
Copy link
Contributor Author

Included the commit for adhering to the maximum allowed limit of 80 characters, as per linting configuration (after the change).

@marco-ippolito marco-ippolito changed the title Docs/test runner doc: clarify after and afterEach hooks execution Jan 20, 2024
@marco-ippolito marco-ippolito added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Jan 21, 2024
@lpinca lpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 3, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 3, 2024
@nodejs-github-bot nodejs-github-bot merged commit 9aa211e into nodejs:main Feb 3, 2024
22 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 9aa211e

rdw-msft pushed a commit to rdw-msft/node that referenced this pull request Feb 9, 2024
@ognjenjevremovic @rdw-msft
doc: clarify execution of after hook on test suite completion
08bc1ce 
The `after` hook now explicitly mentions that it is executed once after
all the tests in a test suite have completed, regardless of whether the
tests passed or failed. This ensures that cleanup tasks or actions
specified in the after hook are guaranteed to run.

Refs: nodejs#50901
PR-URL: nodejs#51523
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
targos pushed a commit that referenced this pull request Feb 15, 2024
@ognjenjevremovic @targos
doc: clarify execution of after hook on test suite completion
c9be260 
The `after` hook now explicitly mentions that it is executed once after
all the tests in a test suite have completed, regardless of whether the
tests passed or failed. This ensures that cleanup tasks or actions
specified in the after hook are guaranteed to run.

Refs: #50901
PR-URL: #51523
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Feb 19, 2024
@ognjenjevremovic @marco-ippolito
doc: clarify execution of after hook on test suite completion
5e0857e 
The `after` hook now explicitly mentions that it is executed once after
all the tests in a test suite have completed, regardless of whether the
tests passed or failed. This ensures that cleanup tasks or actions
specified in the after hook are guaranteed to run.

Refs: nodejs#50901
PR-URL: nodejs#51523
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
@marco-ippolito marco-ippolito mentioned this pull request Mar 1, 2024
Merged
@jehon
Copy link

jehon commented Mar 15, 2024

Thanks all!

richardlau pushed a commit that referenced this pull request Mar 25, 2024
@ognjenjevremovic @richardlau
doc: clarify execution of after hook on test suite completion
d405686 
The `after` hook now explicitly mentions that it is executed once after
all the tests in a test suite have completed, regardless of whether the
tests passed or failed. This ensures that cleanup tasks or actions
specified in the after hook are guaranteed to run.

Refs: #50901
PR-URL: #51523
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
richardlau pushed a commit that referenced this pull request Mar 25, 2024
@ognjenjevremovic @richardlau
doc: clarify execution of after hook on test suite completion
b3157a0 
The `after` hook now explicitly mentions that it is executed once after
all the tests in a test suite have completed, regardless of whether the
tests passed or failed. This ensures that cleanup tasks or actions
specified in the after hook are guaranteed to run.

Refs: #50901
PR-URL: #51523
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
@richardlau richardlau mentioned this pull request Mar 25, 2024
Merged
lukins-cz pushed a commit to lukins-cz/OS-Aplet-node that referenced this pull request Jun 1, 2024
@ognjenjevremovic @lukins-cz
doc: clarify execution of after hook on test suite completion
8950f4d 
The `after` hook now explicitly mentions that it is executed once after
all the tests in a test suite have completed, regardless of whether the
tests passed or failed. This ensures that cleanup tasks or actions
specified in the after hook are guaranteed to run.

Refs: nodejs#50901
PR-URL: nodejs#51523
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lpinca lpinca lpinca approved these changes

@MoLow MoLow MoLow approved these changes

@marco-ippolito marco-ippolito marco-ippolito approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. doc Issues and PRs related to the documentations. test_runner
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@ognjenjevremovic @nodejs-github-bot @jehon @lpinca @MoLow @marco-ippolito