search

TritonDataCenter / node-verror

Rich JavaScript errors
MIT License
1.18k stars 61 forks source link

tst.multierror.js failure on Node v8 and later #65

Closed mk-pmb closed 5 years ago

mk-pmb commented 5 years ago

While trying to debug #64:

Test output

deps/catest/catest -a
Configuration:
    SRC:                         /…/node_modules/verror
    Output directory:            /var/tmp/catest.19359
    Temp directory:              /var/tmp/catest.19359_tmpfiles
    Keep successful test output: false
    Found 8 test(s) to run

===================================================

Executing test test/tst.common.js ... success.
Executing test test/tst.context.js ... success.
Executing test test/tst.findcause.js ... success.
Executing test test/tst.info.js ... success.
Executing test test/tst.inherit.js ... success.
Executing test test/tst.multierror.js ... FAILED.
>>> failure details in /var/tmp/catest.19359/failure.0

Executing test test/tst.verror.js ... success.
Executing test test/tst.werror.js ... success.

===================================================

Results:
    Tests passed:    7/ 8
    Tests failed:    1/ 8

===================================================
Cleaning up output from successful tests ... done.
make: *** [test] Error 1

/var/tmp/catest.19359/failure.0/19359.err

assert.js:765
    throw actual;
    ^

AssertionError [ERR_ASSERTION]: errors ([object]) is required
    at new AssertionError (internal/errors.js:102:11)
    at _toss (/…/node_modules/verror/node_modules/assert-plus/assert.js:22:11)
    at Function.out.(anonymous function) [as arrayOfObject] (/…/node_modules/verror/node_modules/assert-plus/assert.js:156:17)
    at VError.errorFromList (/…/node_modules/verror/lib/verror.js:311:17)
    at /…/node_modules/verror/test/tst.multierror.js:62:17
    at getActual (assert.js:715:5)
    at Function.throws (assert.js:793:24)
    at main (/…/node_modules/verror/test/tst.multierror.js:61:13)
    at Object.<anonymous> (/…/node_modules/verror/test/tst.multierror.js:132:1)
    at Module._compile (module.js:653:30)
    at Object.Module._extensions..js (module.js:664:10)
    at Module.load (module.js:566:32)
    at tryModuleLoad (module.js:506:12)
    at Function.Module._load (module.js:498:3)
    at Function.Module.runMain (module.js:694:10)
    at startup (bootstrap_node.js:204:16)
    at bootstrap_node.js:625:3

require('./package.json').version = 1.10.0 Node.js v8.15.0, npm v6.4.1, Ubuntu 14.04.5 LTS trusty

davepacheco commented 5 years ago

Thanks for the report. I see this as well with Node v8.15.0 on OS X 10.14.2, but not on Node v6.14.4 on the same system.

The test is failing here in test/tst.multierror.js:

 61         mod_assert.throws(function () {
 62                 console.error(errorFromList());
 63         }, /^AssertionError: errors \(\[object\]\) is required$/);

It looks to me like the format of the error's message has just changed a bit.

This patch fixes the test for me:

diff --git a/test/tst.multierror.js b/test/tst.multierror.js
index bc8bac7..a63125d 100644
--- a/test/tst.multierror.js
+++ b/test/tst.multierror.js
@@ -60,23 +60,23 @@ function main()
    /* errorFromList */
    mod_assert.throws(function () {
        console.error(errorFromList());
-   }, /^AssertionError: errors \(\[object\]\) is required$/);
+   }, /^AssertionError.*: errors \(\[object\]\) is required$/);

    mod_assert.throws(function () {
        console.error(errorFromList(null));
-   }, /^AssertionError: errors \(\[object\]\) is required$/);
+   }, /^AssertionError.*: errors \(\[object\]\) is required$/);

    mod_assert.throws(function () {
        console.error(errorFromList({}));
-   }, /^AssertionError: errors \(\[object\]\) is required$/);
+   }, /^AssertionError.*: errors \(\[object\]\) is required$/);

    mod_assert.throws(function () {
        console.error(errorFromList('asdf'));
-   }, /^AssertionError: errors \(\[object\]\) is required$/);
+   }, /^AssertionError.*: errors \(\[object\]\) is required$/);

    mod_assert.throws(function () {
        console.error(errorFromList([ new Error(), 17 ]));
-   }, /^AssertionError: errors \(\[object\]\) is required$/);
+   }, /^AssertionError.*: errors \(\[object\]\) is required$/);

    mod_assert.throws(function () {
        console.error(errorFromList([ new Error(), {} ]));
@@ -95,23 +95,23 @@ function main()
    /* errorForEach */
    mod_assert.throws(function () {
        console.error(errorForEach());
-   }, /^AssertionError: err must be an Error$/);
+   }, /^AssertionError.*: err must be an Error$/);

    mod_assert.throws(function () {
        console.error(errorForEach(null));
-   }, /^AssertionError: err must be an Error$/);
+   }, /^AssertionError.*: err must be an Error$/);

    mod_assert.throws(function () {
        console.error(errorForEach(err1));
-   }, /^AssertionError: func \(func\) is required$/);
+   }, /^AssertionError.*: func \(func\) is required$/);

    mod_assert.throws(function () {
        console.error(errorForEach(err1, {}));
-   }, /^AssertionError: func \(func\) is required$/);
+   }, /^AssertionError.*: func \(func\) is required$/);

    mod_assert.throws(function () {
        console.error(errorForEach({}, function () {}));
-   }, /^AssertionError: err must be an Error$/);
+   }, /^AssertionError.*: err must be an Error$/);

    accum = [];
    doAccum = function (e) { accum.push(e); };
davepacheco commented 5 years ago

I've put up this change to fix it: https://cr.joyent.us/#/c/5363/

PS1 passes make prepush on the OS X system with Node v8.15.0:

dap@blinky node-verror $ git clean -nxd
dap@blinky node-verror $ uname -a
Darwin blinky.local 18.2.0 Darwin Kernel Version 18.2.0: Mon Nov 12 20:24:46 PST 2018; root:xnu-4903.231.4~2/RELEASE_X86_64 x86_64
dap@blinky node-verror $ node -v
v8.15.0
dap@blinky node-verror $ make && make prepush
npm install
npm notice created a lockfile as package-lock.json. You should commit this file.
added 3 packages from 7 contributors and audited 3 packages in 1.344s
found 0 vulnerabilities

jsl --nologo --nosummary --conf=jsl.node.conf lib/verror.js examples/levels-verror.js examples/varargs.js examples/levels-werror.js examples/info.js examples/multierror.js examples/fullStack.js examples/nested.js examples/werror.js examples/verror.js test/tst.info.js test/tst.verror.js test/tst.common.js test/tst.werror.js test/tst.inherit.js test/common.js test/tst.multierror.js test/tst.findcause.js test/tst.context.js
/Users/dap/work/node-verror/lib/verror.js
/Users/dap/work/node-verror/examples/levels-verror.js
/Users/dap/work/node-verror/examples/varargs.js
/Users/dap/work/node-verror/examples/levels-werror.js
/Users/dap/work/node-verror/examples/info.js
/Users/dap/work/node-verror/examples/multierror.js
/Users/dap/work/node-verror/examples/fullStack.js
/Users/dap/work/node-verror/examples/nested.js
/Users/dap/work/node-verror/examples/werror.js
/Users/dap/work/node-verror/examples/verror.js
/Users/dap/work/node-verror/test/tst.info.js
/Users/dap/work/node-verror/test/tst.verror.js
/Users/dap/work/node-verror/test/tst.common.js
/Users/dap/work/node-verror/test/tst.werror.js
/Users/dap/work/node-verror/test/tst.inherit.js
/Users/dap/work/node-verror/test/common.js
/Users/dap/work/node-verror/test/tst.multierror.js
/Users/dap/work/node-verror/test/tst.findcause.js
/Users/dap/work/node-verror/test/tst.context.js
jsstyle  lib/verror.js examples/levels-verror.js examples/varargs.js examples/levels-werror.js examples/info.js examples/multierror.js examples/fullStack.js examples/nested.js examples/werror.js examples/verror.js test/tst.info.js test/tst.verror.js test/tst.common.js test/tst.werror.js test/tst.inherit.js test/common.js test/tst.multierror.js test/tst.findcause.js test/tst.context.js
check ok
deps/catest/catest -a
Configuration:
    SRC:                         /Users/dap/work/node-verror
    Output directory:            /var/tmp/catest.27184
    Temp directory:              /var/tmp/catest.27184_tmpfiles
    Keep successful test output: false
    Found 8 test(s) to run

===================================================

Executing test test/tst.common.js ... success.
Executing test test/tst.context.js ... success.
Executing test test/tst.findcause.js ... success.
Executing test test/tst.info.js ... success.
Executing test test/tst.inherit.js ... success.
Executing test test/tst.multierror.js ... success.
Executing test test/tst.verror.js ... success.
Executing test test/tst.werror.js ... success.

===================================================

Results:
    Tests passed:    8/ 8
    Tests failed:    0/ 8

===================================================
Cleaning up output from successful tests ... done.
davepacheco commented 5 years ago

This also looks good on SmartOS with Node v0.10.48:

dap@f0c3d2d4 node-verror $ uname -a
SunOS f0c3d2d4 5.11 joyent_20180329T002515Z i86pc i386 i86pc Solaris
dap@f0c3d2d4 node-verror $ node -v
v0.10.48
dap@f0c3d2d4 node-verror $ git log -1
commit 58e84b74a0364429e72804cb9ef161c85d8ea27e
Author: David Pacheco <dap@joyent.com>
Date:   Mon Jan 14 14:37:37 2019 -0800

    joyent/node-verror#65 tst.multierror.js failure on Node v8 and later
dap@f0c3d2d4 node-verror $ make && make prepush
npm install
assert-plus@1.0.0 node_modules/assert-plus

core-util-is@1.0.2 node_modules/core-util-is

extsprintf@1.4.0 node_modules/extsprintf
jsl --nologo --nosummary --conf=jsl.node.conf lib/verror.js examples/multierror.js examples/info.js examples/varargs.js examples/fullStack.js examples/werror.js examples/levels-werror.js examples/verror.js examples/nested.js examples/levels-verror.js test/common.js test/tst.werror.js test/tst.common.js test/tst.inherit.js test/tst.context.js test/tst.verror.js test/tst.info.js test/tst.findcause.js test/tst.multierror.js
/home/dap/node-verror/lib/verror.js
/home/dap/node-verror/examples/multierror.js
/home/dap/node-verror/examples/info.js
/home/dap/node-verror/examples/varargs.js
/home/dap/node-verror/examples/fullStack.js
/home/dap/node-verror/examples/werror.js
/home/dap/node-verror/examples/levels-werror.js
/home/dap/node-verror/examples/verror.js
/home/dap/node-verror/examples/nested.js
/home/dap/node-verror/examples/levels-verror.js
/home/dap/node-verror/test/common.js
/home/dap/node-verror/test/tst.werror.js
/home/dap/node-verror/test/tst.common.js
/home/dap/node-verror/test/tst.inherit.js
/home/dap/node-verror/test/tst.context.js
/home/dap/node-verror/test/tst.verror.js
/home/dap/node-verror/test/tst.info.js
/home/dap/node-verror/test/tst.findcause.js
/home/dap/node-verror/test/tst.multierror.js
jsstyle  lib/verror.js examples/multierror.js examples/info.js examples/varargs.js examples/fullStack.js examples/werror.js examples/levels-werror.js examples/verror.js examples/nested.js examples/levels-verror.js test/common.js test/tst.werror.js test/tst.common.js test/tst.inherit.js test/tst.context.js test/tst.verror.js test/tst.info.js test/tst.findcause.js test/tst.multierror.js
check ok
deps/catest/catest -a
Configuration:
    SRC:                         /home/dap/node-verror
    Output directory:            /var/tmp/catest.31011
    Temp directory:              /var/tmp/catest.31011_tmpfiles
    Keep successful test output: false
    Found 8 test(s) to run

===================================================

Executing test test/tst.common.js ... success.
Executing test test/tst.context.js ... success.
Executing test test/tst.findcause.js ... success.
Executing test test/tst.info.js ... success.
Executing test test/tst.inherit.js ... success.
Executing test test/tst.multierror.js ... success.
Executing test test/tst.verror.js ... success.
Executing test test/tst.werror.js ... success.

===================================================

Results:
        Tests passed:    8/ 8
        Tests failed:    0/ 8

===================================================
Cleaning up output from successful tests ... done.