FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: refactor test_runner tests to change default reporter by cjihrig · Pull Request #54547 · nodejs/node · GitHub

/ node Public

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
ConversationCommits (2)ChecksFiles changed
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 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 commented Aug 24, 2024

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/test_runner

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

avivkeller commented Aug 24, 2024

Copy link
Copy Markdown
Member

Note: this PR is part of #54548

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

codecov Bot 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 approved these changes Aug 25, 2024
View reviewed changes

mcollina 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm

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

cjihrig commented Aug 25, 2024

Copy link
Copy Markdown
Contributor Author

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-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 25, 2024

nodejs-github-bot commented Aug 25, 2024

Copy link
Copy Markdown
Collaborator

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

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 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 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

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
cjihrig added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 26, 2024
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 26, 2024

nodejs-github-bot commented Aug 26, 2024

Copy link
Copy Markdown
Collaborator

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

cjihrig 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-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 26, 2024

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 commented Aug 26, 2024 •
edited by jakecastelli
Loading

Copy link
Copy Markdown
Collaborator

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

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

nodejs-github-bot commented Aug 27, 2024

Copy link
Copy Markdown
Collaborator

Landed in 3d5357a

cjihrig deleted the reporter-tests branch August 27, 2024 01:18
RafaelGSS pushed a commit that referenced this pull request Aug 30, 2024
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>
RafaelGSS mentioned this pull request Aug 30, 2024
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

mcollina mcollina approved these changes
jasnell jasnell approved these changes
benjamingr benjamingr approved these changes
MoLow MoLow approved these changes
jakecastelli jakecastelli approved these changes

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

Back | FazBrowse Home | New Git URL