GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — tools: allow test.py to use full paths of tests by reconbot · Pull Request #9694 · nodejs/node

Conversation

@reconbot

@reconbot reconbot commented Nov 19, 2016 •
edited by MylesBorins
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor
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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

MylesBorins, gibfahn, and thefourtheye reacted with thumbs up emoji gibfahn reacted with hooray emoji
All reactions
  • 👍 3 reactions
  • 🎉 1 reaction
@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Nov 19, 2016
princejwesley
princejwesley approved these changes Nov 19, 2016
View reviewed changes
@Fishrock123

Fishrock123 commented Nov 19, 2016

Copy link
Copy Markdown
Contributor

CI: https://ci.nodejs.org/job/node-test-pull-request/4907/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

MylesBorins
MylesBorins approved these changes Nov 19, 2016
View reviewed changes

@MylesBorins MylesBorins left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this 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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
gibfahn
gibfahn approved these changes Nov 19, 2016
View reviewed changes

@gibfahn gibfahn left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@reconbot

reconbot commented Nov 19, 2016

Copy link
Copy Markdown
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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@MylesBorins

MylesBorins commented Nov 19, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

failure looks infra related /cc @nodejs/build

fixes #9684

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

addaleax
addaleax approved these changes Nov 19, 2016
View reviewed changes

@addaleax addaleax left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@jbergstroem

jbergstroem commented Nov 19, 2016

Copy link
Copy Markdown
Member

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

MylesBorins, reconbot, and thefourtheye reacted with thumbs up emoji
All reactions
  • 👍 3 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@MylesBorins

MylesBorins commented Nov 19, 2016

Copy link
Copy Markdown
Contributor

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

reconbot reacted with hooray emoji addaleax, jbergstroem, gibfahn, and thefourtheye reacted with heart emoji
All reactions
  • 🎉 1 reaction
  • ❤️ 4 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jbergstroem

jbergstroem commented Nov 19, 2016

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

thefourtheye
thefourtheye suggested changes Nov 20, 2016
View reviewed changes
Comment thread tools/test.py Outdated

@thefourtheye thefourtheye Nov 20, 2016

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Use startswith and endswith.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

Fixes: nodejs#9684
@reconbot
reconbot force-pushed the testpy-paths branch from b5cd231 to ddcbcf1 Compare November 20, 2016 23:17
@reconbot

reconbot commented Nov 20, 2016

Copy link
Copy Markdown
Contributor Author

@thefourtheye updated to use startswith and endswith.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

thefourtheye
thefourtheye approved these changes Nov 21, 2016
View reviewed changes

@thefourtheye thefourtheye left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Nice. LGTM. Thanks :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
gibfahn
gibfahn approved these changes Nov 21, 2016
View reviewed changes
@MylesBorins

MylesBorins commented Nov 26, 2016

Copy link
Copy Markdown
Contributor

landed in 951ba0d

addaleax reacted with hooray emoji
All reactions
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@MylesBorins MylesBorins closed this Nov 26, 2016
MylesBorins pushed a commit that referenced this pull request Nov 26, 2016
@reconbot @MylesBorins
tools: allow test.py to use full paths of tests
951ba0d
Allow test.py to run tests with a 'tests/' prefix or a '.js' postfix

PR-URL: #9694
Fixes: #9684
Reviewed-By: Prince John Wesley <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 26, 2016
@reconbot @MylesBorins
tools: allow test.py to use full paths of tests
561b149
Allow test.py to run tests with a 'tests/' prefix or a '.js' postfix

PR-URL: #9694
Fixes: #9684
Reviewed-By: Prince John Wesley <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 26, 2016
@reconbot @MylesBorins
tools: allow test.py to use full paths of tests
bd7c1e7
Allow test.py to run tests with a 'tests/' prefix or a '.js' postfix

PR-URL: #9694
Fixes: #9684
Reviewed-By: Prince John Wesley <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 26, 2016
@reconbot @MylesBorins
tools: allow test.py to use full paths of tests
b1bd1c4
Allow test.py to run tests with a 'tests/' prefix or a '.js' postfix

PR-URL: #9694
Fixes: #9684
Reviewed-By: Prince John Wesley <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@MylesBorins MylesBorins added land-on-v4.x labels Nov 26, 2016
@reconbot
reconbot deleted the testpy-paths branch November 27, 2016 16:38
@MylesBorins MylesBorins mentioned this pull request Dec 1, 2016
v6.9.2 proposal #9735
Merged
@Fishrock123 Fishrock123 mentioned this pull request Dec 5, 2016
v7.2.1 proposal #10127
Merged
2 tasks done
@MylesBorins MylesBorins mentioned this pull request Dec 6, 2016
v4.7.0 proposal #9736
Merged
@gibfahn gibfahn mentioned this pull request Feb 9, 2017
Update CONTRIBUTING.md to clarify single-test running #11231
Closed
@richardlau richardlau mentioned this pull request Aug 16, 2019
doc: fix example single-test command #29171
Closed
2 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@addaleax addaleax addaleax approved these changes
+4 more reviewers
@MylesBorins MylesBorins MylesBorins approved these changes
@thefourtheye thefourtheye thefourtheye approved these changes
@princejwesley princejwesley princejwesley approved these changes
@gibfahn gibfahn gibfahn approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

tools Issues and PRs related to the tools directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

9 participants

@reconbot @Fishrock123 @MylesBorins @jbergstroem @thefourtheye @addaleax @princejwesley @gibfahn @nodejs-github-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.