Skip to content

test: refactor test_runner tests to change default reporter - #54547

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
cjihrig:reporter-tests
Aug 27, 2024
Merged

test: refactor test_runner tests to change default reporter#54547
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
cjihrig:reporter-tests

Conversation

@cjihrig

@cjihrigcjihrig commented Aug 24, 2024

Copy link
Copy Markdown
Contributor

This commit updates the test runner tests in order to switch the default reporter from tap to spec. This commit can be backported, while changing the default reporter cannot.

This will also allow us to more slowly migrate the tests off of using the TAP reporter in some places. We should still test the TAP reporter though. In some cases the TAP reporter is beneficial for testing purposes - for example, verifying that tests are output with the correct test number ordering.

Refs: #54540

@nodejs-github-bot

nodejs-github-bot commented Aug 24, 2024

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Aug 24, 2024
@cjihrigcjihrig mentioned this pull request Aug 24, 2024
Merged
@avivkeller

avivkeller commented Aug 24, 2024

Copy link
Copy Markdown
Member

Note: this PR is part of #54548

@avivkelleravivkeller added the test_runner Issues and PRs related to the test runner subsystem. label Aug 24, 2024
@codecov

codecovBot commented Aug 25, 2024•
edited
Loading

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.32%. Comparing base (05bd3cf) to head (d14e5ca).
Report is 321 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #54547 +/- ##
==========================================
- Coverage 87.33% 87.32% -0.01% 
==========================================
Files 649 649 Lines 182622 182623 +1 Branches 35037 35044 +7 ==========================================
- Hits 159494 159483 -11 - Misses 16393 16401 +8 - Partials 6735 6739 +4

see 32 files with indirect coverage changes

mcollina
mcollina approved these changes Aug 25, 2024
View reviewed changes

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@cjihrigcjihrig added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 25, 2024
@cjihrig

cjihrig commented Aug 25, 2024

Copy link
Copy Markdown
ContributorAuthor

test/sequential/test-net-server-listen-ipv6-link-local.js appears to be failing in GitHub Actions. I don't think it is related to this change though because it's also failing in https://github.com/nodejs/node/actions/runs/10547441912/job/29220162937 (for #54190).

@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 25, 2024
@nodejs-github-bot

nodejs-github-bot commented Aug 25, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/61451/

@avivkeller

avivkeller commented Aug 25, 2024

Copy link
Copy Markdown
Member

test/sequential/test-net-server-listen-ipv6-link-local.js appears to be failing in GitHub Actions. I don't think it is related to this change though because it's also failing in nodejs/node/actions/runs/10547441912/job/29220162937 (for #54190).

It's also failing in a few other places, like https://ci.nodejs.org/job/node-test-binary-windows-js-suites/29545/testReport/

@jakecastelli

jakecastelli commented Aug 25, 2024

Copy link
Copy Markdown
Member

The failure is caused by #54470 (just landed) breaks the test added in an earlier landed PR #54264, I tried to revert 54470 by #54554 🙏

@jakecastelli

jakecastelli commented Aug 26, 2024

Copy link
Copy Markdown
Member

Issue has been resolved, would you mind rebasing 🙏 I will try to follow up with the CI

@cjihrig
test: refactor test_runner tests to change default reporter
9b3fab5
This commit updates the test runner tests in order to switch the
default reporter from tap to spec. This commit can be backported,
while changing the default reporter cannot.
Refs: nodejs#54540
@cjihrigcjihrig added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 26, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 26, 2024
@nodejs-github-bot

nodejs-github-bot commented Aug 26, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/61488/

@cjihrigcjihrig added request-ci Add this label to start a Jenkins CI on a PR. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Aug 26, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 26, 2024
@nodejs-github-bot

nodejs-github-bot commented Aug 26, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/61501/

@nodejs-github-bot

nodejs-github-bot commented Aug 26, 2024•
edited by jakecastelli
Loading

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/61516/ 💚

@jakecastellijakecastelli added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 27, 2024
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 27, 2024
@nodejs-github-bot
nodejs-github-bot merged commit 3d5357a into nodejs:mainAug 27, 2024
@nodejs-github-bot

nodejs-github-bot commented Aug 27, 2024

Copy link
Copy Markdown
Collaborator

Landed in 3d5357a

@cjihrig
cjihrig deleted the reporter-tests branch August 27, 2024 01:18
RafaelGSS pushed a commit that referenced this pull request Aug 30, 2024
@cjihrig@RafaelGSS
test: refactor test_runner tests to change default reporter
846e2b2
This commit updates the test runner tests in order to switch the
default reporter from tap to spec. This commit can be backported,
while changing the default reporter cannot.
Refs: #54540
PR-URL: #54547
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com>
@RafaelGSSRafaelGSS mentioned this pull request Aug 30, 2024
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollinamcollinamcollina approved these changes
@jasnelljasnelljasnell approved these changes
@benjamingrbenjamingrbenjamingr approved these changes
@MoLowMoLowMoLow approved these changes
@jakecastellijakecastellijakecastelli approved these changes

Assignees

No one assigned

Labels

commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.needs-ciPRs that need a full CI run.test_runnerIssues and PRs related to the test runner subsystem.testIssues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@cjihrig@nodejs-github-bot@avivkeller@jakecastelli@mcollina@jasnell@benjamingr@MoLow@targos