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

test_runner: remove redundant report call #52089

Merged
merged 1 commit into from
Mar 16, 2024

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Mar 14, 2024

This commit removes a redundant call to reportStarted(). It is redundant because a few lines down, subtest.finalize() is called. finalize() will find the first test that is ready to report its data, and then call report(), which also calls reportStarted(). This will trigger the 'test:start' event as high up the test tree as necessary.

This commit removes a redundant call to `reportStarted()`. It is
redundant because a few lines down, `subtest.finalize()` is
called. `finalize()` will find the first test that is ready to
report its data, and then call `report()`, which also calls
`reportStarted()`. This will trigger the `'test:start'` as high
up the test tree as necessary.
@cjihrig cjihrig requested a review from MoLow March 14, 2024 21:29
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Mar 14, 2024
@MoLow MoLow added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 15, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 15, 2024
@nodejs-github-bot
Copy link
Collaborator

@cjihrig cjihrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 16, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 16, 2024
@nodejs-github-bot nodejs-github-bot merged commit 97b2c53 into nodejs:main Mar 16, 2024
62 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 97b2c53

@cjihrig cjihrig deleted the reporting branch March 17, 2024 13:07
rdw-msft pushed a commit to rdw-msft/node that referenced this pull request Mar 26, 2024
This commit removes a redundant call to `reportStarted()`. It is
redundant because a few lines down, `subtest.finalize()` is
called. `finalize()` will find the first test that is ready to
report its data, and then call `report()`, which also calls
`reportStarted()`. This will trigger the `'test:start'` as high
up the test tree as necessary.

PR-URL: nodejs#52089
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Raz Luvaton <[email protected]>
marco-ippolito pushed a commit that referenced this pull request May 2, 2024
This commit removes a redundant call to `reportStarted()`. It is
redundant because a few lines down, `subtest.finalize()` is
called. `finalize()` will find the first test that is ready to
report its data, and then call `report()`, which also calls
`reportStarted()`. This will trigger the `'test:start'` as high
up the test tree as necessary.

PR-URL: #52089
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Raz Luvaton <[email protected]>
marco-ippolito pushed a commit that referenced this pull request May 3, 2024
This commit removes a redundant call to `reportStarted()`. It is
redundant because a few lines down, `subtest.finalize()` is
called. `finalize()` will find the first test that is ready to
report its data, and then call `report()`, which also calls
`reportStarted()`. This will trigger the `'test:start'` as high
up the test tree as necessary.

PR-URL: #52089
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Raz Luvaton <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants