Skip to content

Conversation

@MoLow
Copy link
Member

@MoLow MoLow commented Oct 27, 2022

TODO:

  • tests
  • documentation?
  • call clearFileFilters for specific child process
  • if it doesn't take too long - wait for TAP parser to land to avoid conflicts

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Oct 27, 2022
@MoLow MoLow added test_runner Issues and PRs related to the test runner subsystem. watch-mode Issues and PRs related to watch mode labels Oct 27, 2022
@MoLow MoLow force-pushed the test_runner_watch_mode branch from 91df6e5 to 49ad862 Compare November 6, 2022 21:41
@cjihrig
Copy link
Contributor

cjihrig commented Nov 9, 2022

@MoLow is this ready for review? At a minimum, it looks like there are linter issues.

@MoLow
Copy link
Member Author

MoLow commented Nov 9, 2022

is this ready for review? At a minimum, it looks like there are linter issues.

not yet, will work on it today/tomorrow

@MoLow MoLow force-pushed the test_runner_watch_mode branch from 49ad862 to 9be722d Compare November 10, 2022 18:01
@MoLow
Copy link
Member Author

MoLow commented Nov 10, 2022

@nodejs/test_runner ready for review

Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

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

We should probably update the doc of --watch-path and --test to clarify they can't be used together

node/doc/api/cli.md

Lines 1214 to 1216 in f1e9382

Starts the Node.js command line test runner. This flag cannot be combined with
`--check`, `--eval`, `--interactive`, or the inspector. See the documentation
on [running tests from the command line][] for more details.

node/doc/api/cli.md

Lines 1610 to 1611 in f1e9382

This flag cannot be combined with
`--check`, `--eval`, `--interactive`, or the REPL.

We could add an entry to the history of --watch and --test to say that now they can be used together.

Could we add a test to ensure watching ESM works? And maybe even ESM+CJS?

@MoLow
Copy link
Member Author

MoLow commented Nov 11, 2022

@ErickWendel you were asking about this

Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

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

I don't know if you missed it in my previous review, I was asking if it would be possible to add tests for ESM (i.e. does dependencies loaded though import() from a CJS files are picked up)

@MoLow MoLow added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Nov 12, 2022
@MoLow
Copy link
Member Author

MoLow commented Nov 13, 2022

Going to rebase on top of #45348 to avoid conflicts

@MoLow MoLow force-pushed the test_runner_watch_mode branch from bf37536 to 28f8da3 Compare November 13, 2022 15:21
@MoLow MoLow removed the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Nov 13, 2022
@MoLow MoLow requested a review from cjihrig November 13, 2022 15:23
Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

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

LGTM if the CI is happy

@MoLow MoLow added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Nov 13, 2022
@MoLow MoLow added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 13, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 13, 2022
@nodejs-github-bot
Copy link
Collaborator

@MoLow MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 13, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 13, 2022
@nodejs-github-bot nodejs-github-bot merged commit 97355c7 into nodejs:main Nov 13, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in 97355c7

@MoLow MoLow deleted the test_runner_watch_mode branch November 13, 2022 21:41
ruyadorno pushed a commit that referenced this pull request Nov 21, 2022
PR-URL: #45214
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
MoLow added a commit to MoLow/node that referenced this pull request Nov 23, 2022
PR-URL: nodejs#45214
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
MoLow added a commit to MoLow/node that referenced this pull request Nov 23, 2022
PR-URL: nodejs#45214
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
MoLow added a commit to MoLow/node that referenced this pull request Nov 24, 2022
PR-URL: nodejs#45214
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
MoLow added a commit to MoLow/node that referenced this pull request Nov 24, 2022
PR-URL: nodejs#45214
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
@ruyadorno ruyadorno mentioned this pull request Nov 24, 2022
MoLow added a commit to MoLow/node that referenced this pull request Dec 9, 2022
PR-URL: nodejs#45214
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
PR-URL: #45214
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
PR-URL: #45214
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
PR-URL: #45214
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
danielleadams pushed a commit that referenced this pull request Jan 4, 2023
PR-URL: #45214
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
@MoLow MoLow mentioned this pull request Apr 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. watch-mode Issues and PRs related to watch mode

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants