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

test: fix single test runner regression #15300

Closed

Conversation

apapirovski
Copy link
Member

@apapirovski apapirovski commented Sep 9, 2017

A recent commit c8a389e broke the ability to run single tests without adding * to the end of the name. This fixes it.

You can test by running python tools/test.py -J --mode=release es-module/test-esm-pkg-over-ext before and after this patch.

If there's a cleaner way to accomplish the same, please let me know. Thanks!

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test

Fixes regression in ability to run single tests.

Refs: nodejs@c8a389e
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Sep 9, 2017
@mscdex mscdex added the python PRs and issues that require attention from people who are familiar with Python. label Sep 9, 2017
Copy link
Member

@Trott Trott 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. Thanks for catching/fixing this.

@Trott
Copy link
Member

Trott commented Sep 11, 2017

If we can get some more reviews on this, I'd be +1 on landing it sooner than the usual 48/72 hours.

@Trott
Copy link
Member

Trott commented Sep 11, 2017

@Trott
Copy link
Member

Trott commented Sep 11, 2017

Another reason to expedite this: I don't think the CI single-stress-test job will work again until this lands. @nodejs/testing

@TimothyGu
Copy link
Member

I have an alternative version that's a bit cleaner IMO: #15329

My bad for break it :(

TimothyGu added a commit to TimothyGu/node that referenced this pull request Sep 11, 2017
MylesBorins pushed a commit that referenced this pull request Sep 12, 2017
When ESM support was added it created a regression in the test runner
that broke the ability to run individual tests. This commit
re-introduces the use of `NormalizePath` which fixes the regression
in the test runner

Refs: #15300
PR-URL: #15329
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this pull request Sep 12, 2017
When ESM support was added it created a regression in the test runner
that broke the ability to run individual tests. This commit
re-introduces the use of `NormalizePath` which fixes the regression
in the test runner

Refs: #15300
PR-URL: #15329
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
addaleax pushed a commit to addaleax/node that referenced this pull request Sep 13, 2017
When ESM support was added it created a regression in the test runner
that broke the ability to run individual tests. This commit
re-introduces the use of `NormalizePath` which fixes the regression
in the test runner

Refs: nodejs#15300
PR-URL: nodejs#15329
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python PRs and issues that require attention from people who are familiar with Python. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants