Skip to content

Fixing to use common.fixtures module - #15869

Closed
suresh-srinivas wants to merge 1 commit into
nodejs:masterfrom
suresh-srinivas:sri-branch
Closed

Fixing to use common.fixtures module#15869
suresh-srinivas wants to merge 1 commit into
nodejs:masterfrom
suresh-srinivas:sri-branch

Conversation

@suresh-srinivas

@suresh-srinivas suresh-srinivas commented Oct 6, 2017

Copy link
Copy Markdown
Contributor

Fixing test to use common.fixtures module in test/sequential/test-process-warnings.js at the Code & Learn in Vancouver 2017.

Checklist
Affected core subsystem(s)

test
@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
@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
lpinca
lpinca approved these changes Oct 10, 2017
View reviewed changes
Comment thread test/sequential/test-process-warnings.js
const assert = require('assert');
const execFile = require('child_process').execFile;
const warnmod = require.resolve(`${common.fixturesDir}/warnings.js`);
const warnmod = require.resolve(fixtures.path('warnings.js'));

@lpinca lpinca Oct 10, 2017 •
edited
Loading

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.

Nit: it should be ok to just use fixtures.path('warnings.js') without passing it to require.resolve().
gireeshpunathil
gireeshpunathil approved these changes Oct 12, 2017
View reviewed changes

@gireeshpunathil gireeshpunathil left a comment

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.

CI: https://ci.nodejs.org/job/node-test-pull-request/10651/
jasnell pushed a commit that referenced this pull request Oct 13, 2017
@suresh-srinivas @jasnell
test: use fixtures in test-process-warnings
cd64b41 
PR-URL: #15869
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
@jasnell

jasnell commented Oct 13, 2017

Copy link
Copy Markdown
Member

Landed in cd64b41
@jasnell jasnell closed this Oct 13, 2017
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 15, 2017
@suresh-srinivas @addaleax
test: use fixtures in test-process-warnings
9104a7e 
PR-URL: nodejs/node#15869
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
targos pushed a commit that referenced this pull request Oct 18, 2017
@suresh-srinivas @targos
test: use fixtures in test-process-warnings
7a210cd 
PR-URL: #15869
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
MylesBorins pushed a commit that referenced this pull request Nov 16, 2017
@suresh-srinivas @MylesBorins
test: use fixtures in test-process-warnings
0b1ad5f 
PR-URL: #15869
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
@MylesBorins MylesBorins mentioned this pull request Nov 21, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
@suresh-srinivas @MylesBorins
test: use fixtures in test-process-warnings
44993bd 
PR-URL: #15869
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
@suresh-srinivas @MylesBorins
test: use fixtures in test-process-warnings
68a2d39 
PR-URL: #15869
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.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.