test: replacing common.fixturesDir in test-require-exceptions.js #15860

tejbirsingh · 2017-10-06T17:02:46Z

Using common.fixtures module instead of common.fixturesDir based on task at Nodejs code and learn at Node.js Interactive 2017 in Vancouver.

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)

Using common.fixtures module instead of common.fixturesDir based on task at Nodejs code and learn at Node.js Interactive 2017 in Vancouver.

gireeshpunathil

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

joyeecheung · 2017-10-11T14:48:53Z

test/parallel/test-require-exceptions.js

 'use strict';
 const common = require('../common');
 const assert = require('assert');
+const fixtures = require ('../common/fixtures');


The linter complained:

not ok 25 - /usr/home/iojs/build/workspace/node-test-linter/test/parallel/test-require-exceptions.js --- message: Unexpected space between function name and paren. severity: error data: line: 25 column: 18 ruleId: func-call-spacing ...

Can you remove the space after require? Thanks.

Removing space after require to appease linter.

tejbirsingh · 2017-10-16T01:46:35Z

@joyeecheung I have updated the PR based on your request. Thank you for letting me know.

joyeecheung · 2017-10-16T04:52:37Z

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

The nit was corrected :-)

lance · 2017-10-16T13:51:41Z

CI Results with unrelated failures.

6,pi1-raspbian-wheezy

+ git clean -fdx warning: failed to remove out/Release/.nfs00000000000829ed000000b7 Removing config.gypi Removing icu_config.gypi Removing node Removing out/Release/openssl-cli Removing out/Release/node Removing test.tap Removing test/abort/testcfg.pyc Removing test/addons-napi/testcfg.pyc Removing test/addons/testcfg.pyc Removing test/async-hooks/testcfg.pyc Removing test/doctool/testcfg.pyc Removing test/es-module/testcfg.pyc Removing test/gc/testcfg.pyc Removing test/inspector/testcfg.pyc Removing test/internet/testcfg.pyc Removing test/known_issues/testcfg.pyc Removing test/message/testcfg.pyc Removing test/parallel/testcfg.pyc Removing test/pseudo-tty/testcfg.pyc Removing test/pummel/testcfg.pyc Removing test/sequential/testcfg.pyc Removing test/testpy/__init__.pyc Removing test/tick-processor/testcfg.pyc Removing test/timers/testcfg.pyc Removing test/tmp.0/ Removing tools/test.pyc Removing tools/utils.pyc Build step 'Execute shell' marked build as failure TAP Reports Processing: START Looking for TAP results report in workspace using pattern: *.tap Did not find any matching files. Setting build result to FAILURE. Checking ^not ok

aix61-ppc64

not ok 1896 inspector/test-stop-profile-after-done # TODO : Fix flaky test --- duration_ms: 0.906 severity: flaky stack: |- [test] Connecting to a child Node process [test] Testing /json/list [err] Debugger listening on ws://127.0.0.1:34272/ded86544-6a2e-4776-aa90-fc9593cf608f [err] For help see https://nodejs.org/en/docs/inspector [err] [out] {} [out] [out] {} [out] [out] {} [out] [out] {} [out] [out] {} [out] [out] {} [out] [out] {} [out] [out] {} [out] [out] {} [out] [out] {} [out] [out] {} [out] { Error: connect ECONNREFUSED 127.0.0.1:34272 at Object._errnoException (util.js:1023:13) at _exceptionWithHostPort (util.js:1044:20) at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1188:14) errno: 'ECONNREFUSED', code: 'ECONNREFUSED', syscall: 'connect', address: '127.0.0.1', port: 34272 } 1 ...

vs2017,win2016,0

not ok 468 inspector/test-bindings # TODO : Fix flaky test --- duration_ms: 0.184 severity: crashed stack: |- oh no! exit code: CRASHED ... not ok 469 inspector/test-debug-end # TODO : Fix flaky test --- duration_ms: 0.615 severity: flaky stack: |- Test there's no crash stopping server that was not started Test there's no crash stopping server without connecting [err] Debugger listening on ws://127.0.0.1:50876/a065bb47-97c7-420e-be6f-ecb8498ebf0f [err] For help see https://nodejs.org/en/docs/inspector [err] Test there's no crash stopping server after connecting [test] Connecting to a child Node process [test] Testing /json/list [err] Debugger listening on ws://127.0.0.1:50877/fa46e087-4253-435d-ab33-7e7d9a231f2c [err] For help see https://nodejs.org/en/docs/inspector [err] [err] Debugger attached. [err] Debugger listening on ws://127.0.0.1:50880/8937680d-1980-47a5-8c15-8397c89f605c [err] For help see https://nodejs.org/en/docs/inspector [err] { AssertionError [ERR_ASSERTION]: 42 === 3221225477 at testSessionNoCrash (c:\workspace\node-test-binary-windows\test\inspector\test-debug-end.js:35:3) at at process._tickCallback (internal/process/next_tick.js:188:7) generatedMessage: true, name: 'AssertionError [ERR_ASSERTION]', code: 'ERR_ASSERTION', actual: 42, expected: 3221225477, operator: '===' } 1 ...

---

This can be landed.

lance · 2017-10-16T14:01:54Z

Landed in: 9b3d6a0

@tejbirsingh thanks for the contribution!

Use common.fixtures module instead of common.fixturesDir based on task at Nodejs code and learn at Node.js Interactive 2017 in Vancouver. PR-URL: #15860 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

tejbirsingh · 2017-10-16T16:53:46Z

@lance Yaay my first contribution!

Use common.fixtures module instead of common.fixturesDir based on task at Nodejs code and learn at Node.js Interactive 2017 in Vancouver. PR-URL: #15860 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

Use common.fixtures module instead of common.fixturesDir based on task at Nodejs code and learn at Node.js Interactive 2017 in Vancouver. PR-URL: nodejs/node#15860 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

Use common.fixtures module instead of common.fixturesDir based on task at Nodejs code and learn at Node.js Interactive 2017 in Vancouver. PR-URL: #15860 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

test: replacing common.fixturesDir in test-require-exceptions.js

cfa33c2

Using common.fixtures module instead of common.fixturesDir based on task at Nodejs code and learn at Node.js Interactive 2017 in Vancouver.

nodejs-github-bot added the test Issues and PRs related to the tests. label Oct 6, 2017

Trott added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Oct 6, 2017

cjihrig approved these changes Oct 7, 2017

View reviewed changes

richardlau approved these changes Oct 7, 2017

View reviewed changes

benjamingr approved these changes Oct 7, 2017

View reviewed changes

jasnell approved these changes Oct 10, 2017

View reviewed changes

lpinca approved these changes Oct 10, 2017

View reviewed changes

gireeshpunathil reviewed Oct 11, 2017

View reviewed changes

gireeshpunathil approved these changes Oct 11, 2017

View reviewed changes

joyeecheung previously requested changes Oct 11, 2017

View reviewed changes

test: removing space after require in test-require-exceptions.js

085b28f

Removing space after require to appease linter.

lance self-assigned this Oct 16, 2017

lance closed this Oct 16, 2017

MylesBorins added the land-on-v6.x label Nov 16, 2017

MylesBorins mentioned this pull request Nov 21, 2017

v6.12.1 proposal #17180

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

test: replacing common.fixturesDir in test-require-exceptions.js #15860

test: replacing common.fixturesDir in test-require-exceptions.js #15860

Uh oh!

tejbirsingh commented Oct 6, 2017

gireeshpunathil left a comment

joyeecheung Oct 11, 2017

tejbirsingh commented Oct 16, 2017

joyeecheung commented Oct 16, 2017

lance commented Oct 16, 2017

lance commented Oct 16, 2017

tejbirsingh commented Oct 16, 2017

Labels

12 participants

Uh oh!

test: replacing common.fixturesDir in test-require-exceptions.js #15860

test: replacing common.fixturesDir in test-require-exceptions.js #15860

Uh oh!

Conversation

tejbirsingh commented Oct 6, 2017

Checklist

Affected core subsystem(s)

gireeshpunathil left a comment

Choose a reason for hiding this comment

joyeecheung Oct 11, 2017

Choose a reason for hiding this comment

tejbirsingh commented Oct 16, 2017

joyeecheung commented Oct 16, 2017

lance commented Oct 16, 2017

lance commented Oct 16, 2017

tejbirsingh commented Oct 16, 2017

Labels

12 participants