Skip to content

test: replaced common.fixturesDir with common.fixtures in test-process-redirect-warnings-env - #15930

Closed
frkat wants to merge 2 commits into
nodejs:masterfrom
frkat:replace-common-fixtures-warnings-env
Closed

test: replaced common.fixturesDir with common.fixtures in test-process-redirect-warnings-env#15930
frkat wants to merge 2 commits into
nodejs:masterfrom
frkat:replace-common-fixtures-warnings-env

Conversation

@frkat

@frkat frkat commented Oct 6, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to Node.js core tests and test infrastructure. label Oct 6, 2017
@frkat frkat changed the title replaced common.fixturesDir with common.fixtures in test-process-redirect-warnings-env Oct 6, 2017
@mscdex mscdex added the process Issues and PRs related to the process subsystem. label Oct 6, 2017
@Trott Trott added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Oct 6, 2017
BridgeAR
BridgeAR reviewed Oct 8, 2017
View reviewed changes
Comment thread test/parallel/test-process-redirect-warnings-env.js Outdated
common.refreshTmpDir();

const warnmod = require.resolve(`${common.fixturesDir}/warnings.js`);
const warnmod = require.resolve(`${fixtures.fixturesDir}/warnings.js`);

@BridgeAR BridgeAR Oct 8, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should probably be replaced with fixtures.path("/warnings.js")

@frkat frkat Oct 9, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@BridgeAR Indeed. Thanks
evanlucas
evanlucas approved these changes Oct 9, 2017
View reviewed changes

@evanlucas evanlucas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

CI: https://ci.nodejs.org/job/node-test-pull-request/10532/
lpinca
lpinca reviewed Oct 13, 2017
View reviewed changes
Comment thread test/parallel/test-process-redirect-warnings-env.js
common.refreshTmpDir();

const warnmod = require.resolve(`${common.fixturesDir}/warnings.js`);
const warnmod = require.resolve(fixtures.path('/warnings.js'));

@lpinca lpinca Oct 13, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This could be just fixtures.path('warnings.js').

@jasnell jasnell Oct 13, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fixing this on landing
@jasnell

jasnell commented Oct 13, 2017

Copy link
Copy Markdown
Member

Failures in CI are unrelated.
jasnell pushed a commit that referenced this pull request Oct 13, 2017
@frkat @jasnell
test: fixtures in test-process-redirect-warnings-env
ab6eed8 
Replaced common.fixturesDir with common.fixtures in
test-process-redirect-warnings-env

PR-URL: #15930
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell

jasnell commented Oct 13, 2017

Copy link
Copy Markdown
Member

Landed in ab6eed8.
Thank you!
@jasnell jasnell closed this Oct 13, 2017
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 15, 2017
@frkat @addaleax
test: fixtures in test-process-redirect-warnings-env
772841e 
Replaced common.fixturesDir with common.fixtures in
test-process-redirect-warnings-env

PR-URL: nodejs/node#15930
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Oct 18, 2017
@frkat @targos
test: fixtures in test-process-redirect-warnings-env
969c87e 
Replaced common.fixturesDir with common.fixtures in
test-process-redirect-warnings-env

PR-URL: #15930
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 16, 2017
@frkat @MylesBorins
test: fixtures in test-process-redirect-warnings-env
5f01272 
Replaced common.fixturesDir with common.fixtures in
test-process-redirect-warnings-env

PR-URL: #15930
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Nov 21, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
@frkat @MylesBorins
test: fixtures in test-process-redirect-warnings-env
cc67141 
Replaced common.fixturesDir with common.fixtures in
test-process-redirect-warnings-env

PR-URL: #15930
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
@frkat @MylesBorins
test: fixtures in test-process-redirect-warnings-env
244bfb3 
Replaced common.fixturesDir with common.fixtures in
test-process-redirect-warnings-env

PR-URL: #15930
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. process Issues and PRs related to the process subsystem. test Issues and PRs related to Node.js core tests and test infrastructure.

9 participants

@frkat @jasnell @evanlucas @lpinca @BridgeAR @mscdex @MylesBorins @Trott @nodejs-github-bot