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

doc: improve docs around running javascript tests #49022

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

iloveitaly
Copy link
Contributor

As far as I could tell, this was not documented anywhere. These tips + examples would have been helpful for a first-time contributor like myself.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/tsc

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Aug 4, 2023
Comment on lines +150 to +154
A filter can be applied to only run specific tests:

```bash
make jstest test/parallel/test-debugger-pid.js test/sequential/test-debugger-pid.js test/sequential/test-debugger-launch.mjs
```
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that's true, it runs the full test suite for me.

Suggested change
A filter can be applied to only run specific tests:
```bash
make jstest test/parallel/test-debugger-pid.js test/sequential/test-debugger-pid.js test/sequential/test-debugger-launch.mjs
```
A filter can be applied to only run specific tests:
```bash
./tools/test.py test/parallel/test-debugger-pid.js test/sequential/test-debugger-pid
```

@@ -139,6 +139,28 @@ request. Interesting things to notice:
exit gracefully. Remember that for a test to succeed, it must exit with a
status code of 0.

## Running tests

All tests can be executed by running the `jstest` target:
Copy link
Contributor

Choose a reason for hiding this comment

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

It won't run all the tests, as its name implies it's only a subset of them.

Suggested change
All tests can be executed by running the `jstest` target:
Running the `jstest` target will execute all the tests for the `node` binary:

Copy link
Member

Choose a reason for hiding this comment

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

I think there is still a catch in this...that internet tests aren't run with make jstest 😂

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
All tests can be executed by running the `jstest` target:
Running the `jstest` target will execute all the tests for the `node` binary
except those which require internet access:

All tests can be executed by running the `jstest` target:

```bash
make jstest
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
make jstest
make jstest -j12

@GeoffreyBooth
Copy link
Member

You could also add that filters can be used to run all the tests for a particular subsystem. A command I use a lot is:

tools/test.py --mode=release --timeout=30 es-module

To run all the ESM tests in about 8 seconds on my machine.

Really this is just a filter on the es-module folder; as a long-term goal it would be nice to gradually break up the parallel folder into various folders based on subsystem, so that as you’re working on a particular subsystem you can quickly re-run that system’s set of tests often, and then run the full test suite when you’re ready to commit or push.

@@ -139,6 +139,28 @@ request. Interesting things to notice:
exit gracefully. Remember that for a test to succeed, it must exit with a
status code of 0.

## Running tests
Copy link
Member

Choose a reason for hiding this comment

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

Not blocking this pull request but a suggestion for someone motivated to do it: This PR might contain information that we don't have elsewhere, but we do have "how to run tests" information in both BUILDING.md and doc/contributing/pull-requests.md. Not blocking on this pull request for this, but it would probably be good to consolidate how-to-run-tests info in one place and then put links to the One True Source Of Information in the docs that we remove it from. Part of me wants to suggest putting it all in test/README.md, but any of the places where it exists now would be fine, including the one being created in this PR.

@aduh95
Copy link
Contributor

aduh95 commented Aug 28, 2023

as a long-term goal it would be nice to gradually break up the parallel folder into various folders based on subsystem, so that as you’re working on a particular subsystem you can quickly re-run that system’s set of tests often, and then run the full test suite when you’re ready to commit or push.

You can already run a subset of tests in the parallel folder:

$ tools/test.py test/parallel/test-crypto*
[00:05|% 100|+  60|-   0]: Done                                               

All tests passed.

@GeoffreyBooth
Copy link
Member

You can already run a subset of tests in the parallel folder:

TIL! That's pretty cool. Still though, I think a more sensible organization would avoid having over a thousand files in a single folder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants