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

Update CONTRIBUTING.md to clarify single-test running #11231

Closed
Trott opened this issue Feb 8, 2017 · 8 comments
Closed

Update CONTRIBUTING.md to clarify single-test running #11231

Trott opened this issue Feb 8, 2017 · 8 comments
Labels
discuss Issues opened for discussions and feedbacks. doc Issues and PRs related to the documentations. test Issues and PRs related to the tests.

Comments

@Trott
Copy link
Member

Trott commented Feb 8, 2017

In CONTRIBUTING.md:

You can run tests directly with node:

```text
$ ./node ./test/parallel/test-stream2-transform.js
```

This bit is a little problematic and can trip up new folks.

  • You can run tests in some directories (such as parallel and sequential) but not others (such as message).
  • If the test has a --FLAGS comment, then that needs to be taken into account on the command line or else the test won't work.

We could clarify all this, or we could just remove that bit of text entirely and instruct people to always use test.py. That second option seems like the better one to me. Discuss.

@nodejs/testing

@Trott Trott added discuss Issues opened for discussions and feedbacks. doc Issues and PRs related to the documentations. test Issues and PRs related to the tests. labels Feb 8, 2017
@Trott
Copy link
Member Author

Trott commented Feb 8, 2017

By the way, this definitely comes up for real users.

@bnoordhuis
Copy link
Member

I don't know, it's still useful info for people trying to debug segfaults. The test runner only gets in the way then.

@gibfahn
Copy link
Member

gibfahn commented Feb 9, 2017

Given that the first method suggested (directly above) is to run the test with tools/test.py, I think we could just say that this alternative method doesn't always work without going into extensive detail.

If we are going to have more documentation about what the test runner does, it might be worth putting that somewhere else (like test/README.md) and just linking to said info from CONTRIBUTING.md.

@joyeecheung
Copy link
Member

Maybe extend doc/guides/writing-tests.md a little?

@joaocgreis
Copy link
Member

The way to run a single test using test.py was not obvious to me at first (test.py parallel/test-stream2-transform), I would welcome this in the documentation.

@Trott
Copy link
Member Author

Trott commented Feb 9, 2017

@joaocgreis It's immediately above the text we're talking about in this PR:

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:

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

There's probably room to improve that text and/or make it more noticeable.

@gibfahn
Copy link
Member

gibfahn commented Feb 9, 2017

@joaocgreis FYI, since #9694, you can use the full path, e.g. tools/test.py test/parallel/test-stream2-transform.js, which should make it more obvious.

@joaocgreis
Copy link
Member

@Trott my bad, thanks for pointing it out.

I agree with @gibfahn above, we could remove this from CONTRIBUTING.md, leaving only instructions to run with test.py and a link to test/README.md. There, add some more information (a note about --FLAGS, a a note about message and https://github.com/nodejs/node/blob/master/test/message/testcfg.py, ...).

@Trott Trott closed this as completed Jul 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Issues opened for discussions and feedbacks. doc Issues and PRs related to the documentations. test Issues and PRs related to the tests.
Projects
None yet
Development

No branches or pull requests

5 participants