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: allow test.py to use full paths of tests #9694

Closed
wants to merge 1 commit into from

Conversation

reconbot
Copy link
Contributor

@reconbot reconbot commented Nov 19, 2016

Checklist
  • commit message follows commit guidelines
  • ran test suite, no new tests
Affected core subsystem(s)

Fixes: #9684
tools/test.py can now take paths like

$ 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
Description of change

Slices off the test/ and the .js on input files. Wildcards like parallel/test-cluster-* will still work as will defaults.

@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Nov 19, 2016
@Fishrock123
Copy link
Contributor

Copy link
Contributor

@MylesBorins MylesBorins 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 CI is green

Copy link
Member

@gibfahn gibfahn left a comment

Choose a reason for hiding this comment

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

Looking forward to no more No tests to run because you copied the .js as well. Also tab-complete for tests is great.

@reconbot
Copy link
Contributor Author

@thealphanerd freebsd failed but I can't figure out what failed, I see "no failures"

screen shot 2016-11-19 at 10 35 51 am

@MylesBorins
Copy link
Contributor

MylesBorins commented Nov 19, 2016

failure looks infra related /cc @nodejs/build

fixes #9684

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

LGTM

@jbergstroem
Copy link
Member

@thealphanerd what you meant to say was "the failure looks @jbergstroem-related". Should be fixed now.

@MylesBorins
Copy link
Contributor

@jbergstroem you are a hero! Thank you for all the hard infra work you do.

@jbergstroem
Copy link
Member

@thealphanerd thanks! if only the work wasn't so tied to fixing my other work :)

@@ -1461,6 +1461,13 @@ def SplitPath(s):
stripped = [ c.strip() for c in s.split('/') ]
return [ Pattern(s) for s in stripped if len(s) > 0 ]

def NormalizePath(path):
# strip the extra path information of the specified test
if path[:5] == 'test/':
Copy link
Contributor

Choose a reason for hiding this comment

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

Use startswith and endswith.

Allow test.py to run tests with a 'tests/' prefix or a '.js' postfix

Fixes: nodejs#9684
@reconbot
Copy link
Contributor Author

@thefourtheye updated to use startswith and endswith.

Copy link
Contributor

@thefourtheye thefourtheye left a comment

Choose a reason for hiding this comment

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

Nice. LGTM. Thanks :)

@MylesBorins
Copy link
Contributor

landed in 951ba0d

MylesBorins pushed a commit that referenced this pull request Nov 26, 2016
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 pushed a commit that referenced this pull request Nov 26, 2016
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 pushed a commit that referenced this pull request Nov 26, 2016
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 pushed a commit that referenced this pull request Nov 26, 2016
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>
@reconbot reconbot deleted the testpy-paths branch November 27, 2016 16:38
@MylesBorins MylesBorins mentioned this pull request Dec 1, 2016
@Fishrock123 Fishrock123 mentioned this pull request Dec 5, 2016
2 tasks
@MylesBorins MylesBorins mentioned this pull request Dec 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants