test: replacing common.fixturesDir in test-require-exceptions.js by tejbirsingh · Pull Request #15860 · nodejs/node

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!

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

test: replacing common.fixturesDir in test-require-exceptions.js#15860
tejbirsingh wants to merge 2 commits intonodejs:masterfrom
tejbirsingh:master

tejbirsingh commented Oct 6, 2017

Uh oh!

gireeshpunathil left a comment

Uh oh!

joyeecheung Oct 11, 2017

Uh oh!

tejbirsingh commented Oct 16, 2017

Uh oh!

joyeecheung commented Oct 16, 2017

Uh oh!

lance commented Oct 16, 2017

Uh oh!

lance commented Oct 16, 2017

Uh oh!

tejbirsingh commented Oct 16, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

Uh oh!

Conversation

tejbirsingh commented Oct 6, 2017

Checklist

Affected core subsystem(s)

Uh oh!

gireeshpunathil left a comment

Choose a reason for hiding this comment

Uh oh!

joyeecheung Oct 11, 2017

Choose a reason for hiding this comment

Uh oh!

tejbirsingh commented Oct 16, 2017

Uh oh!

joyeecheung commented Oct 16, 2017

Uh oh!

lance commented Oct 16, 2017

Uh oh!

lance commented Oct 16, 2017

Uh oh!

tejbirsingh commented Oct 16, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants