test: add deprecation code to expectWarning #19474

danbev · 2018-03-20T12:17:34Z

This commit adds a deprecation code to expectWarning and updates the
function to check the passed code against the code property on the
warning object.

Not all warnings have a deprecation code so for those that don't an
explicit code of common.noWarnCode is required. Passing this skips the
assertion of the code.

This PR follows up on #19317.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
documentation is changed or added
commit message follows commit guidelines

danbev · 2018-03-20T12:18:09Z

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

vsemozhetbyt · 2018-03-20T12:25:02Z

test/common/README.md


-Tests whether `name` and `expected` are part of a raised warning.
+Tests whether `name`, `expected`, and 'code' are part of a raised warning. If


Nit: 'code' -> `code`?

Thanks, will fix this.

vsemozhetbyt · 2018-03-20T12:26:36Z

test/common/README.md


-Tests whether `name` and `expected` are part of a raised warning.
+Tests whether `name`, `expected`, and 'code' are part of a raised warning. If
+an expected warning does not have a code then common.noWarnCode can be used


Nit: common.noWarnCode -> `common.noWarnCode`?

Will fix this too, thanks.

targos · 2018-03-20T12:25:27Z

test/common/README.md

@@ -111,8 +111,11 @@ Indicates if there is more than 1gb of total memory.
 ### expectWarning(name, expected)


code argument is missing here

targos · 2018-03-20T12:25:54Z

test/common/README.md


-Tests whether `name` and `expected` are part of a raised warning.
+Tests whether `name`, `expected`, and 'code' are part of a raised warning. If
+an expected warning does not have a code then common.noWarnCode can be used


wrap common.noWarnCode in backticks?

targos · 2018-03-20T12:26:23Z

test/common/README.md


-Tests whether `name` and `expected` are part of a raised warning.
+Tests whether `name`, `expected`, and 'code' are part of a raised warning. If


backticks for code instead of quotes

targos · 2018-03-20T12:28:03Z

test/common/index.js

@@ -633,8 +645,12 @@ function expectWarningByMap(warningMap) {
  const catchWarning = {};
  Object.keys(warningMap).forEach((name) => {
    let expected = warningMap[name];
-    if (typeof expected === 'string') {
-      expected = [expected];
+    if (!(expected instanceof Array)) {


suggestion: Array.isArray(expected)

Thanks for that, will update shortly.

targos · 2018-03-20T12:28:20Z

test/common/index.js

+      throw new Error('warningMap entries must be arrays consisting of two ' +
+      'entries: [message, warningCode]');
+    }
+    if (!(expected[0] instanceof Array)) {


danbev · 2018-03-20T12:57:28Z

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

Trott · 2018-03-20T13:16:28Z

Should common.noWarnCode be documented, even if it's just to say "See common.expectWarning()`?

danbev · 2018-03-20T13:21:21Z

Should common.noWarnCode be documented, even if it's just to say "See common.expectWarning()`?

I did but then removed it as I was not sure if only functions should be documented. Happy to add it and refer to common.expectWarning like you suggested though.

danbev · 2018-03-22T11:41:12Z

CI after latest updates: https://ci.nodejs.org/job/node-test-pull-request/13822/

danbev · 2018-03-22T13:12:25Z

I've currently looking into

node-test-linux-linked-fips20 failure

console output:

07:52:27 not ok 328 parallel/test-crypto-authenticated
07:52:27   ---
07:52:27   duration_ms: 0.215
07:52:27   severity: fail
07:52:27   stack: |-
07:52:27     1..0 # Skipped: IV len < 12 bytes unsupported in FIPS mode
07:52:27     1..0 # Skipped: IV len < 12 bytes unsupported in FIPS mode
07:52:27     1..0 # Skipped: IV len < 12 bytes unsupported in FIPS mode
07:52:27     Mismatched <anonymous> function calls. Expected exactly 1, actual 0.
07:52:27         at Object.exports.mustCall (/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_fips20_x64/test/common/index.js:427:10)
07:52:27         at expectWarning (/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_fips20_x64/test/common/index.js:618:18)
07:52:27         at Object.keys.forEach (/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_fips20_x64/test/common/index.js:655:26)
07:52:27         at Array.forEach (<anonymous>)
07:52:27         at expectWarningByMap (/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_fips20_x64/test/common/index.js:646:27)
07:52:27         at Object.exports.expectWarning (/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_fips20_x64/test/common/index.js:667:5)
07:52:27         at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_fips20_x64/test/parallel/test-crypto-authenticated.js:350:8)
07:52:27         at Module._compile (internal/modules/cjs/loader.js:677:30)
07:52:27         at Object.Module._extensions..js (internal/modules/cjs/loader.js:688:10)
07:52:27     (node:16009) [DEP0091] DeprecationWarning: crypto.DEFAULT_ENCODING is deprecated.
07:52:27     (node:16009) [DEP0090] DeprecationWarning: Permitting authentication tag lengths of 0 bytes is deprecated. Valid GCM tag lengths are 4, 8, 12, 13, 14, 15, 16.
07:52:27     (node:16009) [DEP0090] DeprecationWarning: Permitting authentication tag lengths of 1 bytes is deprecated. Valid GCM tag lengths are 4, 8, 12, 13, 14, 15, 16.
07:52:27     (node:16009) [DEP0090] DeprecationWarning: Permitting authentication tag lengths of 2 bytes is deprecated. Valid GCM tag lengths are 4, 8, 12, 13, 14, 15, 16.
07:52:27     (node:16009) [DEP0090] DeprecationWarning: Permitting authentication tag lengths of 6 bytes is deprecated. Valid GCM tag lengths are 4, 8, 12, 13, 14, 15, 16.
07:52:27     (node:16009) [DEP0090] DeprecationWarning: Permitting authentication tag lengths of 9 bytes is deprecated. Valid GCM tag lengths are 4, 8, 12, 13, 14, 15, 16.
07:52:27     (node:16009) [DEP0090] DeprecationWarning: Permitting authentication tag lengths of 10 bytes is deprecated. Valid GCM tag lengths are 4, 8, 12, 13, 14, 15, 16.
07:52:27     (node:16009) [DEP0090] DeprecationWarning: Permitting authentication tag lengths of 11 bytes is deprecated. Valid GCM tag lengths are 4, 8, 12, 13, 14, 15, 16.
07:52:27     (node:16009) [DEP0090] DeprecationWarning: Permitting authentication tag lengths of 17 bytes is deprecated. Valid GCM tag lengths are 4, 8, 12, 13, 14, 15, 16.
07:52:27   ...

danbev · 2018-03-23T06:30:26Z

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

danbev · 2018-03-23T07:27:28Z

node-test-commit-aix failure looks unrelated

console output:

not ok 706 parallel/test-http-client-timeout-agent
  ---
  duration_ms: 1.819
  severity: fail
  stack: |-
    res#0 data:0
    res#0 end
    res#2 data:2
    res#2 end
    res#4 data:4
    res#4 end
    res#6 data:6
    res#6 end
    res#8 data:8
    res#8 end
    res#10 data:10
    res#10 end
    res#12 data:12
    res#12 end
    res#14 data:14
    res#14 end
    res#16 data:16
    res#16 end
    res#18 data:18
    res#18 end
    res#20 data:20
    res#20 end
    res#22 data:22
    res#22 end
    res#24 data:24
    res#24 end
    res#26 data:26
    res#26 end
    res#28 data:28
    res#28 end
    req#1 timeout
    req#3 timeout
    req#5 timeout
    req#7 timeout
    req#9 timeout
    req#11 timeout
    req#13 timeout
    req#15 timeout
    req#17 timeout
    req#19 timeout
    req#21 timeout
    req#23 timeout
    req#25 timeout
    req#27 timeout
    req#29 timeout
    req#0 timeout
    req#2 timeout
    req#4 timeout
    req#6 timeout
    req#8 timeout
    req#10 timeout
    req#12 timeout
    req#14 timeout
    req#16 timeout
    req#28 close
    req#26 close
    req#24 close
    req#22 close
    req#20 close
    req#18 close
    req#16 close
    req#14 close
    req#12 close
    req#10 close
    req#8 close
    req#6 close
    req#4 close
    req#2 close
    req#0 close
    req#29 error
    req#29 close
    req#27 error
    req#27 close
    req#25 error
    req#25 close
    req#23 error
    req#23 close
    req#21 error
    req#21 close
    req#19 error
    req#19 close
    req#17 error
    req#17 close
    req#15 error
    req#15 close
    req#13 error
    req#13 close
    req#11 error
    req#11 close
    req#9 error
    req#9 close
    req#7 error
    req#7 close
    req#5 error
    req#5 close
    req#3 error
    req#3 close
    req#1 error
    req#1 close
    done=39 sent=30
    assert.js:80
      throw new AssertionError(obj);
      ^
    
    AssertionError [ERR_ASSERTION]: timeout on http request called too much
        at process.<anonymous> (/home/iojs/build/workspace/node-test-commit-aix/nodes/aix61-ppc64/test/parallel/test-http-client-timeout-agent.js:94:10)
        at process.emit (events.js:187:15)

node-test-commit-arm-fanned failure looks unrelated

console output:

03:17:11 Checking ^not ok
03:28:30 Agent went offline during the build
03:28:30 ERROR: Connection was broken: java.util.concurrent.TimeoutException: Ping started at 1521789510661 hasn't completed by 1521790110665
03:28:30 	at hudson.remoting.PingThread.ping(PingThread.java:134)
03:28:30 	at hudson.remoting.PingThread.run(PingThread.java:90)
03:28:30

This commit adds a deprecation code to expectWarning and updates the function to check the passed code against the code property on the warning object. Not all warnings have a deprecation code so for those that don't an explicit code of common.noWarnCode is required. Passing this skips the assertion of the code.

danbev · 2018-03-26T05:43:27Z

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

danbev · 2018-03-26T08:28:59Z

freebsd11-x64 failure looks unrelated

console output:

not ok 42 async-hooks/test-promise.promise-before-init-hooks
  ---
  duration_ms: 0.356
  severity: crashed
  stack: |-
    oh no!
    exit code: CRASHED (Signal: 11)

danbev · 2018-03-26T08:45:32Z

Landed in 8fb4ea9.

This commit adds a deprecation code to expectWarning and updates the function to check the passed code against the code property on the warning object. Not all warnings have a deprecation code so for those that don't an explicit code of common.noWarnCode is required. Passing this skips the assertion of the code. PR-URL: nodejs#19474 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>

nodejs-github-bot added the test Issues and PRs related to the tests. label Mar 20, 2018

vsemozhetbyt reviewed Mar 20, 2018

View reviewed changes

targos reviewed Mar 20, 2018

View reviewed changes

targos approved these changes Mar 20, 2018

View reviewed changes

jasnell approved these changes Mar 20, 2018

View reviewed changes

tniessen approved these changes Mar 20, 2018

View reviewed changes

danbev added the wip Issues and PRs that are still a work in progress. label Mar 22, 2018

danbev removed the wip Issues and PRs that are still a work in progress. label Mar 23, 2018

danbev force-pushed the tests_deprectation_code branch from dcad716 to 55a7d6a Compare March 26, 2018 05:42

danbev closed this Mar 26, 2018

danbev deleted the tests_deprectation_code branch March 26, 2018 08:45

tniessen mentioned this pull request Mar 30, 2018

crypto: add support for AES-CCM #18138

Closed

4 tasks

targos added the dont-land-on-v9.x label Apr 2, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: add deprecation code to expectWarning #19474

test: add deprecation code to expectWarning #19474

danbev commented Mar 20, 2018

danbev commented Mar 20, 2018

vsemozhetbyt Mar 20, 2018

danbev Mar 20, 2018

vsemozhetbyt Mar 20, 2018

danbev Mar 20, 2018

targos Mar 20, 2018

targos Mar 20, 2018

targos Mar 20, 2018

targos Mar 20, 2018

danbev Mar 20, 2018

targos Mar 20, 2018

danbev commented Mar 20, 2018

Trott commented Mar 20, 2018

danbev commented Mar 20, 2018

danbev commented Mar 22, 2018

danbev commented Mar 22, 2018

danbev commented Mar 23, 2018

danbev commented Mar 23, 2018 •

edited

Loading

danbev commented Mar 26, 2018

danbev commented Mar 26, 2018

danbev commented Mar 26, 2018


		Tests whether `name` and `expected` are part of a raised warning.
		Tests whether `name`, `expected`, and 'code' are part of a raised warning. If

		@@ -111,8 +111,11 @@ Indicates if there is more than 1gb of total memory.
		### expectWarning(name, expected)

test: add deprecation code to expectWarning #19474

test: add deprecation code to expectWarning #19474

Conversation

danbev commented Mar 20, 2018

Checklist

danbev commented Mar 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danbev commented Mar 20, 2018

Trott commented Mar 20, 2018

danbev commented Mar 20, 2018

danbev commented Mar 22, 2018

danbev commented Mar 22, 2018

danbev commented Mar 23, 2018

danbev commented Mar 23, 2018 • edited Loading

danbev commented Mar 26, 2018

danbev commented Mar 26, 2018

danbev commented Mar 26, 2018

danbev commented Mar 23, 2018 •

edited

Loading