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.

Sign up for GitHub

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

Jump to bottom

test: use fixtures module over fixturesDir #15847

Closed
wants to merge 1 commit into from
Closed

test: use fixtures module over fixturesDir #15847

wants to merge 1 commit into from

Conversation

JamesNimlos
Copy link
Contributor

clean up code by using fixtures helper module
instead of fixturesDir directly.
Refs: https://github.com/nodejs/node/tree/master/test/common#fixtures-module

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

N/A

@JamesNimlos
test: use fixtures module over fixturesDir
efdb422 
clean up code by using fixtures helper module
instead of fixturesDir directly.
Refs: https://github.com/nodejs/node/tree/master/test/common#fixtures-module
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Oct 6, 2017
rmg
rmg approved these changes Oct 6, 2017
View reviewed changes
Copy link
Contributor

@rmg rmg left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for the contribution!

@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
@mscdex mscdex added the cli Issues and PRs related to the Node.js command line interface. label Oct 6, 2017
gireeshpunathil
gireeshpunathil approved these changes Oct 10, 2017
View reviewed changes
Copy link
Member

@gireeshpunathil gireeshpunathil left a comment

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/10578/

@gireeshpunathil
Copy link
Member

spurious crash in smart-os. Re-CI: https://ci.nodejs.org/job/node-test-pull-request/10665/

@Trott
Copy link
Member

Trott commented Oct 13, 2017

CI is showing green, but no tests were actually run. Something is not working on CI. Will have to run again after it's fixed.

@jasnell
Copy link
Member

jasnell commented Oct 13, 2017

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

@jasnell
Copy link
Member

jasnell commented Oct 13, 2017

New limited CI on arm: https://ci.nodejs.org/job/node-test-binary-arm/10882/

@joyeecheung
Copy link
Member

Landed in 4e835b3, thanks for the contribution!

joyeecheung pushed a commit that referenced this pull request Oct 14, 2017
@JamesNimlos @joyeecheung
test: use fixtures module over fixturesDir
4e835b3 
clean up code by using fixtures helper module
instead of fixturesDir directly.

PR-URL: #15847
Reviewed-By: Ryan Graham <r.m.graham@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@JamesNimlos
Copy link
Contributor Author

Awesome, thanks everyone!! 🤩

addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 15, 2017
@JamesNimlos @addaleax
test: use fixtures module over fixturesDir
b03faa5 
clean up code by using fixtures helper module
instead of fixturesDir directly.

PR-URL: nodejs/node#15847
Reviewed-By: Ryan Graham <r.m.graham@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this pull request Oct 18, 2017
@JamesNimlos @targos
test: use fixtures module over fixturesDir
438728a 
clean up code by using fixtures helper module
instead of fixturesDir directly.

PR-URL: #15847
Reviewed-By: Ryan Graham <r.m.graham@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 16, 2017
@JamesNimlos @MylesBorins
test: use fixtures module over fixturesDir
5d71477 
clean up code by using fixtures helper module
instead of fixturesDir directly.

PR-URL: #15847
Reviewed-By: Ryan Graham <r.m.graham@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Nov 21, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
@JamesNimlos @MylesBorins
test: use fixtures module over fixturesDir
423a060 
clean up code by using fixtures helper module
instead of fixturesDir directly.

PR-URL: #15847
Reviewed-By: Ryan Graham <r.m.graham@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
@JamesNimlos @MylesBorins
test: use fixtures module over fixturesDir
086d851 
clean up code by using fixtures helper module
instead of fixturesDir directly.

PR-URL: #15847
Reviewed-By: Ryan Graham <r.m.graham@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

@lpinca lpinca lpinca approved these changes

@benjamingr benjamingr benjamingr approved these changes

@jasnell jasnell jasnell approved these changes

@rmg rmg rmg approved these changes

Assignees
No one assigned
Labels
cli Issues and PRs related to the Node.js command line interface. code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.