GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — test_runner: fixed test object is incorrectly passed to setup() by pulkit-30 · Pull Request #50982 · nodejs/node

Conversation

@pulkit-30

@pulkit-30 pulkit-30 commented Nov 30, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Fix #48809

pass testStream to setup instead testRoot.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
tonivj5 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@nodejs-github-bot

nodejs-github-bot commented Nov 30, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/test_runner
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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 Nov 30, 2023
pulkit-30
pulkit-30 commented Nov 30, 2023
View reviewed changes
Comment thread lib/internal/test_runner/utils.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
@pulkit-30
pulkit-30 marked this pull request as ready for review December 3, 2023 12:36
rluvaton
rluvaton reviewed Dec 3, 2023
View reviewed changes

@rluvaton rluvaton left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Can you please add a test

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
pulkit-30 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@pulkit-30
pulkit-30 requested a review from rluvaton December 4, 2023 04:38
rluvaton
rluvaton reviewed Dec 4, 2023
View reviewed changes
Comment thread test/parallel/test-runner-run.mjs Resolved

Uh oh!

There was an error while loading. Please reload this page.
rluvaton
rluvaton reviewed Dec 4, 2023
View reviewed changes
Comment thread test/parallel/test-runner-run.mjs Resolved

Uh oh!

There was an error while loading. Please reload this page.
@pulkit-30
pulkit-30 force-pushed the fix-48809 branch from 4086899 to 7513467 Compare December 4, 2023 11:08
rluvaton
rluvaton reviewed Dec 4, 2023
View reviewed changes
Comment thread test/parallel/test-runner-run.mjs Resolved

Uh oh!

There was an error while loading. Please reload this page.
rluvaton
rluvaton approved these changes Dec 4, 2023
View reviewed changes

@rluvaton rluvaton left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Well done! thanks

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@rluvaton rluvaton added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Dec 4, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Dec 4, 2023
@nodejs-github-bot

nodejs-github-bot commented Dec 4, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/56073/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@pulkit-30
pulkit-30 requested a review from MoLow December 4, 2023 15:29
MoLow
MoLow requested changes Dec 5, 2023
View reviewed changes

@MoLow MoLow left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

this PR causes a regression:
running a simple test file:

const test = require("node:test");
test("a", () => console.log(1));

on main console is printed with colors
image

but on this branch the colors are discarded:
image

I am working on fixing the test that should have caught this

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@MoLow MoLow mentioned this pull request Dec 5, 2023
test: fix test runner colored output test #51064
Merged
@pulkit-30

pulkit-30 commented Dec 6, 2023

Copy link
Copy Markdown
Contributor Author

this PR causes a regression:
running a simple test file:

I was also suspecting something, but surprised by the tests 😅
MoLow reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@pulkit-30

pulkit-30 commented Dec 6, 2023

Copy link
Copy Markdown
Contributor Author

converting this PR to draft. ref: #51064, still working on it 🚀
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@pulkit-30
pulkit-30 marked this pull request as draft December 6, 2023 01:51
@pulkit-30
pulkit-30 force-pushed the fix-48809 branch from 3c8c335 to 79d22e5 Compare December 6, 2023 17:01
@pulkit-30
pulkit-30 marked this pull request as ready for review December 6, 2023 17:01
@pulkit-30
pulkit-30 requested a review from MoLow December 11, 2023 15:49
@pulkit-30
pulkit-30 force-pushed the fix-48809 branch from 79d22e5 to 6a3ebf7 Compare December 11, 2023 15:51
MoLow
MoLow reviewed Dec 11, 2023
View reviewed changes
Comment thread test/parallel/test-runner-run.mjs Resolved

Uh oh!

There was an error while loading. Please reload this page.
MoLow
MoLow reviewed Dec 11, 2023
View reviewed changes
Comment thread lib/internal/test_runner/utils.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
MoLow
MoLow requested changes Dec 11, 2023
View reviewed changes

@MoLow MoLow left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

createWriteStream is called multiple times for each reporter destination, and in some cases the writeStream is never being closed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@pulkit-30
test_runner: fixed test object is incorrectly passed to setup()
e4eb26b
@pulkit-30
fix suggestions
175af14
@pulkit-30
pulkit-30 force-pushed the fix-48809 branch from 6a3ebf7 to 175af14 Compare December 12, 2023 10:57
pulkit-30 added 2 commits December 12, 2023 16:30
@pulkit-30
fix lint
775ea08
@pulkit-30
call colorizeTestFil fn before abort
9f64e75
MoLow
MoLow reviewed Dec 12, 2023
View reviewed changes
Comment thread lib/internal/test_runner/utils.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
MoLow
MoLow reviewed Dec 12, 2023
View reviewed changes
Comment thread lib/internal/test_runner/harness.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
MoLow
MoLow reviewed Dec 12, 2023
View reviewed changes
Comment thread lib/internal/test_runner/runner.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
@pulkit-30
fix suggestions
ca75b72
@pulkit-30
pulkit-30 requested a review from MoLow December 15, 2023 16:50
@pulkit-30

pulkit-30 commented Dec 23, 2023

Copy link
Copy Markdown
Contributor Author

@MoLow PTAL.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
MoLow
MoLow approved these changes Dec 23, 2023
View reviewed changes
@MoLow MoLow added request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. commit-queue-squash PRs the Commit Queue should land as one squashed commit. labels Dec 23, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Dec 23, 2023
@nodejs-github-bot

nodejs-github-bot commented Dec 23, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/56498/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Dec 24, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/56508/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MoLow MoLow added the commit-queue PRs queued for automated landing through the Commit Queue. label Dec 24, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Dec 24, 2023
@nodejs-github-bot
nodejs-github-bot merged commit dad666a into nodejs:main Dec 24, 2023
@nodejs-github-bot

nodejs-github-bot commented Dec 24, 2023

Copy link
Copy Markdown
Collaborator

Landed in dad666a
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MoLow

MoLow commented Dec 24, 2023

Copy link
Copy Markdown
Member

@pulkit-30 thanks for the hard work 🎉
pulkit-30 reacted with hooray emoji pulkit-30 reacted with heart emoji
All reactions
  • 🎉 1 reaction
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@pulkit-30

pulkit-30 commented Dec 24, 2023

Copy link
Copy Markdown
Contributor Author

@pulkit-30 thanks for the hard work 🎉

Thanks a ton! It means a lot.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
RafaelGSS pushed a commit that referenced this pull request Jan 2, 2024
@pulkit-30 @RafaelGSS
test_runner: fixed test object is incorrectly passed to setup()
979cebc 
PR-URL: #50982
Reviewed-By: Raz Luvaton <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Jan 2, 2024
v21.6.0 proposal #51342
Merged
@koshic

koshic commented Jan 16, 2024

Copy link
Copy Markdown

@MoLow do you plan to backport it to 20.x? Documentation is still incorrect - https://nodejs.org/docs/latest-v20.x/api/test.html#runoptions.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MoLow

MoLow commented Jan 16, 2024

Copy link
Copy Markdown
Member

not every PR needs a manual backport. lets wait for @nodejs/releasers to release a new v20 version and request a backport if needed
koshic reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
richardlau pushed a commit that referenced this pull request Mar 25, 2024
@pulkit-30 @richardlau
test_runner: fixed test object is incorrectly passed to setup()
34ecd1e 
PR-URL: #50982
Reviewed-By: Raz Luvaton <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
@richardlau richardlau mentioned this pull request Mar 25, 2024
v20.12.0 proposal #52212
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ljharb ljharb ljharb left review comments
@MoLow MoLow MoLow approved these changes
@rluvaton rluvaton rluvaton approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. commit-queue-squash PRs the Commit Queue should land as one squashed commit. needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

test_runner: test object is incorrectly passed to setup()

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@pulkit-30 @nodejs-github-bot @MoLow @koshic @ljharb @rluvaton
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.