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, test: question about test.py -v key in CONTRIBUTING.md #12771

Closed
vsemozhetbyt opened this issue May 1, 2017 · 13 comments
Closed

doc, test: question about test.py -v key in CONTRIBUTING.md #12771

vsemozhetbyt opened this issue May 1, 2017 · 13 comments
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. test Issues and PRs related to the tests.

Comments

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented May 1, 2017

  • Version: 8.0.0-rc.0
  • Platform: Windows 7 x64
  • Subsystem: test

CONTRIBUTING.md states:

If you are updating tests and just want to run a single test to check it, you can use this syntax to run it exactly as the test harness would:

$ python tools/test.py -v --mode=release parallel/test-stream2-transform

Is -v key intended here? I can't find this key in Makefile and vcbuild.bat. It seems it just adds more debug info:

j:\temp\node-master> python tools/test.py --mode=release parallel/test-stream2-transform
[00:00|% 100|+   1|-   0]: Done

j:\temp\node-master> python tools/test.py -v --mode=release parallel/test-stream2-transform
# j:\temp\node-master\Release\node.exe -p process.arch
[00:00|%   0|+   0|-   0]: release test-stream2-transform # j:\temp\node-master\Release\node.exe j:\temp\node-master\test\parallel\test-stream2-transform.js
[00:00|% 100|+   1|-   0]: Done
@vsemozhetbyt vsemozhetbyt added doc Issues and PRs related to the documentations. test Issues and PRs related to the tests. labels May 1, 2017
@gibfahn
Copy link
Member

gibfahn commented May 1, 2017

It should probably be removed to match Makefile#L198-199 exactly. The -v was originally added in 8a6c36d, and has never been in the Makefile AFAICT.

@gibfahn gibfahn added good first issue Issues that are suitable for first-time contributors. mentor-available labels May 1, 2017
@gibfahn
Copy link
Member

gibfahn commented May 1, 2017

Might as well add the -J option while we're at it, as that's what the Makefile uses.

-$ python tools/test.py -v --mode=release parallel/test-stream2-transform
+$ python tools/test.py -J --mode=release parallel/test-stream2-transform

@vsemozhetbyt
Copy link
Contributor Author

@gibfahn What these flags change, BTW?

@gibfahn
Copy link
Member

gibfahn commented May 1, 2017

@vsemozhetbyt if you run tools/test.py --help it'll show you. You can also read the options in the source (e.g. here for -J.

  • -j4 means run 4 tests at a time (only does it for the parallel suite).
  • -J means calculate the number of cores and run that many tests at a time
  • -v is verbose (as you mentioned)

@vsemozhetbyt
Copy link
Contributor Author

@gibfahn Thank you!

@richardlau
Copy link
Member

Might as well add the -J option while we're at it, as that's what the Makefile uses.

It's extra characters to type though for no actual effect in the original use case (running a single test through the harness).

@vsemozhetbyt
Copy link
Contributor Author

@richardlau @gibfahn So maybe it is useful also to add an example how to run a test suite (section, folder).

@gibfahn
Copy link
Member

gibfahn commented May 2, 2017

It's extra characters to type though for no actual effect in the original use case (running a single test through the harness).

In the original case yes, but people won't be running the original case (and if they are they'll be copy-pasting right?) It's useful to know about -J because it makes the tests run faster, and also because it's possible the parallelism is triggering a bug in the test (which would be really hard to diagnose otherwise).

So maybe it is useful also to add an example how to run a test suite (section, folder).

Maybe, the problem with the test runner is that there are loads of options to go into. Maybe what we should do is note that you can do tools/test.py --help to see the available options, and then add some examples into that --help section (not sure how easy that is to do).

@vsemozhetbyt
Copy link
Contributor Author

@gibfahn BTW: #12786

@kysnm
Copy link
Contributor

kysnm commented May 2, 2017

I would like to try this issue tonight or tomorrow, okay?

@andreihincu
Copy link

hello.... I am kind of new here... I would like to help.

@gibfahn
Copy link
Member

gibfahn commented May 8, 2017

@avhincu there's already a PR open for this (@kysnm has opened #12830). If you want to help there should be some other good first contribution issues that haven't already been taken by someone, if you aren't finding any let us know and we'll find you one!

@gibfahn
Copy link
Member

gibfahn commented May 18, 2017

#12830 landed.

@gibfahn gibfahn closed this as completed May 18, 2017
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. good first issue Issues that are suitable for first-time contributors. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants