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

tools/test.py should be able to get a full path to a test to manually run it #9684

Closed
MylesBorins opened this Issue Nov 18, 2016 · 7 comments

Comments

Projects
None yet
5 participants
@MylesBorins
Copy link
Member

MylesBorins commented Nov 18, 2016

Currently if you want to manually run a test you have to remove test/ from the beginning of the path and not include the js prefix.

For example the following commands will not work

$ python tools/test.py test/parallel/test-cluster-worker-init.js
$ python tools/test.py test/parallel/test-cluster-worker-init
$ python tools/test.py parallel/test-cluster-worker-init.js

but the following will

$ python tools/test.py parallel/test-cluster-worker-init

I find this counter intuitive and would like the ability to pass the full path to a test

/cc @nodejs/build @jbergstroem do you oppose to this being done?

@bnoordhuis

This comment has been minimized.

Copy link
Member

bnoordhuis commented Nov 18, 2016

Personally I'm fine with that as long as the current syntax (including wildcards like parallel/test-cluster-*) keeps working.

@MylesBorins

This comment has been minimized.

Copy link
Member Author

MylesBorins commented Nov 19, 2016

I was imagining it could likely be a regex/replace for test/ and .js

Example implemented with sed

$ echo "test/parallel/test-cluster-worker-init.js" | sed 's+test/++; s/.js//'
parallel/test-cluster-worker-init
@reconbot

This comment has been minimized.

Copy link
Contributor

reconbot commented Nov 19, 2016

@addaleax

This comment has been minimized.

Copy link
Member

addaleax commented Nov 19, 2016

@reconbot It works, so I guess there’s nothing speaking against opening a PR and seeing where it goes? (You’ll want to add a tools: prefix to your commit subject line, btw; EDIT: and a Fixes: https://github.com/nodejs/node/issues/9684 line, too? :))

@reconbot

This comment has been minimized.

Copy link
Contributor

reconbot commented Nov 19, 2016

I found the handy contributors guide link when opening the PR. I think I got it right

@jbergstroem

This comment has been minimized.

Copy link
Member

jbergstroem commented Nov 19, 2016

I'm with @bnoordhuis; would accept such a PR.

@reconbot

This comment has been minimized.

Copy link
Contributor

reconbot commented Nov 19, 2016

@addaleax thanks for the subsection btw, it wasn't clear to me what that should be

reconbot added a commit to reconbot/node that referenced this issue Nov 20, 2016

tools: allow test.py to use full paths of tests
Allow test.py to run tests with a 'tests/' prefix or a '.js' postfix

Fixes: nodejs#9684

MylesBorins added a commit that referenced this issue Nov 26, 2016

tools: allow test.py to use full paths of tests
Allow test.py to run tests with a 'tests/' prefix or a '.js' postfix

PR-URL: #9694
Fixes: #9684
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

MylesBorins added a commit that referenced this issue Nov 26, 2016

tools: allow test.py to use full paths of tests
Allow test.py to run tests with a 'tests/' prefix or a '.js' postfix

PR-URL: #9694
Fixes: #9684
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

MylesBorins added a commit that referenced this issue Nov 26, 2016

tools: allow test.py to use full paths of tests
Allow test.py to run tests with a 'tests/' prefix or a '.js' postfix

PR-URL: #9694
Fixes: #9684
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.