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.

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

errors, buffer: Migrate buffer errors to use internal/errors #13976

Merged
merged 1 commit into from
Jul 12, 2017

Conversation

starkwang
Copy link
Contributor

Migrate buffer errors to use internal/errors.

Ref: #11273

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)

buffer, errors

@nodejs-github-bot nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core. labels Jun 29, 2017
lib/buffer.js Outdated
'If encoding is specified then the first argument must be a string'
);
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', {
msg: 'The first argument must be of type string.'
Copy link
Member

Choose a reason for hiding this comment

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

I believe these should be passed parameters which provide info about the arg and expected type as opposed to a fixed string. See invalidArgType(name, expected, actual) in internal/errors.js

Copy link
Contributor Author

@starkwang starkwang Jun 30, 2017

Choose a reason for hiding this comment

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

Agreed.
I think invalidArgType(name, expected, actual) is unsuitable for all situations. Because "Invalid argument type" can caused by many reasons:

  • Argument must be one or more specified type, but recieve other.
  • Argument must NOT be one or more specified type, but recieve.
  • In some specified condition, the above two happen.

But invalidArgType(name, expected, actual) in internal/errors.js can only handle the first one, without concerning about the rest.

Should I extend the invalidArgType method in order to make it suitable for all situations?

lib/buffer.js Outdated

if (typeof value === 'number')
throw new TypeError('"value" argument must not be a number');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', {
Copy link
Member

Choose a reason for hiding this comment

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

same comment as above

lib/buffer.js Outdated
@@ -196,7 +200,9 @@ Buffer.from = function(value, encodingOrOffset, length) {
length);
}

throw new TypeError(kFromErrorMsg);
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', {
Copy link
Member

Choose a reason for hiding this comment

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

same comment as above

lib/buffer.js Outdated
@@ -208,12 +214,14 @@ function assertSize(size) {
let err = null;

if (typeof size !== 'number') {
err = new TypeError('"size" argument must be a number');
// err = new TypeError('"size" argument must be a number');
err = new errors.TypeError('ERR_INVALID_ARG_TYPE', 'size', 'number', size);
Copy link
Member

Choose a reason for hiding this comment

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

same comment as above

lib/buffer.js Outdated
throw new Error(
'If encoding is specified then the first argument must be a string'
);
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', {
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be passed arguments that provide info about the expected argument as opposed to a specific string. Look at the function associated with ERR_INVALID_ARG_TYPE in internal/errors.js. Same comment applies to everywhere ERR_INVALID_ARG_TYPE is used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

lib/buffer.js Outdated
} else if (size > binding.kMaxLength) {
err = new RangeError('"size" argument must not be larger ' +
'than ' + binding.kMaxLength);
err = new errors.RangeError('ERR_VALUE_OUT_OF_RANGE',
Copy link
Member

Choose a reason for hiding this comment

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

This is probably a good candidate for adding a function that takes info about the arguments and the expected range as opposed to using a fixed string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

@@ -586,6 +586,10 @@ Used when `Console` is instantiated without `stdout` stream or when `stdout` or

Used when the native call from `process.cpuUsage` cannot be processed properly.

<a id="ERR_DEPRECATED_METHOD"></a>
### ERR_DEPRECATED_METHOD
Copy link
Member

Choose a reason for hiding this comment

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

To avoid confusion, perhaps ERR_NO_LONGER_SUPPORTED would be better here. The method itself is not deprecated, only one particular way of calling it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

@starkwang starkwang force-pushed the buffer-internal-errors branch 5 times, most recently from 07afe26 to 1778514 Compare July 1, 2017 15:58
refack
refack previously requested changes Jul 5, 2017
@@ -34,8 +34,10 @@ deepStrictEqual(Buffer.from(
runInNewContext('new String(checkString)', {checkString})),
check);

const err = new RegExp('^TypeError: First argument must be a string, Buffer, ' +
'ArrayBuffer, Array, or array-like object\\.$');
const err = new RegExp(
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be updated to be a common.expectsError()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -56,7 +56,11 @@ exports.isOSX = process.platform === 'darwin';

exports.enoughTestMem = os.totalmem() > 0x40000000; /* 1 Gb */
exports.bufferMaxSizeMsg = new RegExp(
`^RangeError: "size" argument must not be larger than ${buffer.kMaxLength}$`);
'RangeError \\[ERR_VALUE_OUT_OF_RANGE\\]: ' +
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this work is these were just strings?

Copy link
Contributor Author

@starkwang starkwang Jul 5, 2017

Choose a reason for hiding this comment

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

It works, but I'm updating it to use common.expectsError.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -234,3 +248,22 @@ function oneOf(expected, thing) {
return `of ${thing} ${String(expected)}`;
}
}

function valueOutOfRange(name, compareWord, target, actual) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not convinced this is not overkill... For example https://github.com/nodejs/node/blob/master/lib/readline.js#L109
Can you try and find any more places where it's useful?
Otherwise a simpler error message should be enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

<a id="ERR_NO_LONGER_SUPPORTED"></a>
### ERR_NO_LONGER_SUPPORTED

Used when call a method which is no longer supported.
Copy link
Contributor

Choose a reason for hiding this comment

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

Used when a Node.js API in called in an unsupported manner.

For example: `Buffer.write(string, encoding, offset[, length])`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@starkwang starkwang force-pushed the buffer-internal-errors branch 3 times, most recently from ed482e1 to 512cb57 Compare July 5, 2017 08:14
lib/buffer.js Outdated
@@ -1178,7 +1180,7 @@ Buffer.prototype.readDoubleBE = function readDoubleBE(offset, noAssert) {

function checkInt(buffer, value, offset, ext, max, min) {
if (value > max || value < min)
throw new TypeError('"value" argument is out of bounds');
throw new errors.TypeError('ERR_INVALID_OPT_VALUE', 'value', value);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know if this error code is suitable.
Should we have a new error code for it?

Copy link
Member

Choose a reason for hiding this comment

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

It should probably be a RangeError

@starkwang
Copy link
Contributor Author

starkwang commented Jul 5, 2017

Now all the errors in buffer module have been migrated into internal/errors.

lib/buffer.js Outdated
throw new Error(
'If encoding is specified then the first argument must be a string'
);
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'first argument',
Copy link
Member

Choose a reason for hiding this comment

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

This should be the name of the argument as opposed to 'first argument'

Copy link
Member

Choose a reason for hiding this comment

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

Sames goes for other similar instances.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is checking for the https://nodejs.org/api/buffer.html#buffer_new_buffer_string_encoding case first argument -> string


if (typeof value === 'number')
throw new TypeError('"value" argument must not be a number');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'value', 'not number',
Copy link
Member

Choose a reason for hiding this comment

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

What does the string end up being here ? I think this may be the first case were we have the 'not' cases versus listing out what it should be.

Copy link
Contributor Author

@starkwang starkwang Jul 7, 2017

Choose a reason for hiding this comment

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

I extended the invalidArgType(name, expected, actual) in internal/errors.js to make it adapt to the 'not' cases. See here.

The case here will end up being:

TypeError [ERR_INVALID_ARG_TYPE]: The "value" argument must not be of type number. Received type number

The tests for it is also changed.

lib/buffer.js Outdated
@@ -300,7 +299,7 @@ function fromString(string, encoding) {
} else {
length = byteLength(string, encoding, true);
if (length === -1)
throw new TypeError('"encoding" must be a valid string encoding');
throw new errors.TypeError('ERR_INVALID_OPT_VALUE', 'encoding', encoding);
Copy link
Member

Choose a reason for hiding this comment

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

I think it should likely be 'string encoding'

Copy link
Member

Choose a reason for hiding this comment

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

In other places UNKNOWN_ENCODING so that should be used here as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

but IMHO name should stay encoding like in the doc - https://nodejs.org/api/buffer.html#buffer_new_buffer_string_encoding

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think UNKNOWN_ENCODING is more suitable for this.

lib/buffer.js Outdated
@@ -401,7 +400,8 @@ Buffer.isBuffer = function isBuffer(b) {

Buffer.compare = function compare(a, b) {
if (!isUint8Array(a) || !isUint8Array(b)) {
throw new TypeError('Arguments must be Buffers or Uint8Arrays');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'arguments',
Copy link
Member

Choose a reason for hiding this comment

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

This does not quite fit with the new approach. It really should be one of 'a' or 'b' instead of 'arguments'. I think we may need to different cases one that throws an error for 'a' and one for 'b' to be consistent with how we through errors everywhere else. To match that the second string passed in should be the name of the argument defined in the function definition that is wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO it should be buf1 and buf2 alike in the API docs https://nodejs.org/api/buffer.html#buffer_class_method_buffer_compare_buf1_buf2

lib/buffer.js Outdated
@@ -635,8 +636,8 @@ Buffer.prototype.toString = function(encoding, start, end) {

Buffer.prototype.equals = function equals(b) {
if (!isUint8Array(b))
throw new TypeError('Argument must be a Buffer or Uint8Array');

throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'argument',
Copy link
Member

Choose a reason for hiding this comment

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

same comment here as before, probably need to check for all cases that the second string matches the name of the argument that was invalid.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mhdawson
Copy link
Member

mhdawson commented Jul 6, 2017

This turns out to be one of the more challenging ones to convert, thanks for the work so far. A few more comments.

<a id="ERR_NO_LONGER_SUPPORTED"></a>
### ERR_NO_LONGER_SUPPORTED

Used when a Node.js API in called in an unsupported manner.
Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, in -> is

lib/buffer.js Outdated
throw new Error(
'If encoding is specified then the first argument must be a string'
);
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'first argument',
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is checking for the https://nodejs.org/api/buffer.html#buffer_new_buffer_string_encoding case first argument -> string

lib/buffer.js Outdated
@@ -300,7 +299,7 @@ function fromString(string, encoding) {
} else {
length = byteLength(string, encoding, true);
if (length === -1)
throw new TypeError('"encoding" must be a valid string encoding');
throw new errors.TypeError('ERR_INVALID_OPT_VALUE', 'encoding', encoding);
Copy link
Contributor

Choose a reason for hiding this comment

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

but IMHO name should stay encoding like in the doc - https://nodejs.org/api/buffer.html#buffer_new_buffer_string_encoding

exports.bufferMaxSizeMsg = expectsError({
code: 'ERR_INVALID_OPT_VALUE',
type: RangeError,
message: /^The value ".*" is invalid for option "size"$/
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you replace .* with [^"]* just to be more explicit
(and at L65 too)

Since common is becoming a Winnebago, I'm actually not in love with these two. The old one is reused 3 times, and the new one 2 times.
Maybe for your next PR you could inline them 😉

@refack refack dismissed their stale review July 6, 2017 20:28

CR addressed

@refack
Copy link
Contributor

refack commented Jul 6, 2017

@starkwang this is really good work. I would approve after you address the comments.

Optionally if you have some more patience to replace all the RegExps with common.expectsError that would be fantastic. But it could also wait for a future PR.

@starkwang starkwang force-pushed the buffer-internal-errors branch 2 times, most recently from dd2bdc4 to 7c28ac2 Compare July 7, 2017 07:28
@starkwang
Copy link
Contributor Author

Pushed commit to address comments.

@starkwang
Copy link
Contributor Author

It seems like the CI failed after merged to the master.
I've fixed it. Shall we restart the CI?

@refack
Copy link
Contributor

refack commented Jul 7, 2017

@@ -717,7 +714,7 @@ exports.expectsError = function expectsError({code, type, message}) {
}
return true;
};
};
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't lint:

not ok 3 - /usr/home/iojs/build/workspace/node-test-linter/test/common/index.js
  ---
  message: Missing semicolon.
  severity: error
  data:
    line: 717
    column: 2
    ruleId: semi
  ...

Simplest thing is to restore the previous code

Copy link
Contributor Author

@starkwang starkwang Jul 7, 2017

Choose a reason for hiding this comment

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

Oops, I forget make lint to check it.
Stupid mistake☹️

Copy link
Contributor

Choose a reason for hiding this comment

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

That's what the CI is for

@refack
Copy link
Contributor

refack commented Jul 7, 2017

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM

@jasnell
Copy link
Member

jasnell commented Jul 12, 2017

@refack
Copy link
Contributor

refack commented Jul 12, 2017

Killed https://ci.nodejs.org/job/node-test-commit-arm/10847/nodes=armv7-wheezy/ after it had a few test fails, and there are others as well (12 in total):

not ok 86 parallel/test-buffer-bytelength
  ---
  duration_ms: 0.411
  severity: fail
  stack: |-
    Mismatched <anonymous> function calls. Expected exactly 1, actual 4.
        at Object.exports.mustCall (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:481:10)
        at Object.expectsError (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:707:27)
        at Object.<anonymous> (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-buffer-bytelength.js:9:23)
        at Module._compile (module.js:569:30)
        at Object.Module._extensions..js (module.js:580:10)
        at Module.load (module.js:503:32)
        at tryModuleLoad (module.js:466:12)
        at Function.Module._load (module.js:458:3)
        at Function.Module.runMain (module.js:605:10)
  ...
not ok 87 parallel/test-buffer-compare
  ---
  duration_ms: 0.409
  severity: fail
  stack: |-
    Mismatched <anonymous> function calls. Expected exactly 1, actual 2.
        at Object.exports.mustCall (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:481:10)
        at Object.expectsError (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:707:27)
        at Object.<anonymous> (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-buffer-compare.js:31:23)
        at Module._compile (module.js:569:30)
        at Object.Module._extensions..js (module.js:580:10)
        at Module.load (module.js:503:32)
        at tryModuleLoad (module.js:466:12)
        at Function.Module._load (module.js:458:3)
        at Function.Module.runMain (module.js:605:10)
  ...
not ok 97 parallel/test-buffer-includes
  ---
  duration_ms: 0.407
  severity: fail
  stack: |-
    Mismatched <anonymous> function calls. Expected exactly 1, actual 3.
        at Object.exports.mustCall (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:481:10)
        at Object.expectsError (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:707:27)
        at Object.<anonymous> (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-buffer-includes.js:274:30)
        at Module._compile (module.js:569:30)
        at Object.Module._extensions..js (module.js:580:10)
        at Module.load (module.js:503:32)
        at tryModuleLoad (module.js:466:12)
        at Function.Module._load (module.js:458:3)
        at Function.Module.runMain (module.js:605:10)
  ...
not ok 99 parallel/test-buffer-indexof
  ---
  duration_ms: 0.708
  severity: fail
  stack: |-
    Mismatched <anonymous> function calls. Expected exactly 1, actual 3.
        at Object.exports.mustCall (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:481:10)
        at Object.expectsError (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:707:27)
        at Object.<anonymous> (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-buffer-indexof.js:347:33)
        at Module._compile (module.js:569:30)
        at Object.Module._extensions..js (module.js:580:10)
        at Module.load (module.js:503:32)
        at tryModuleLoad (module.js:466:12)
        at Function.Module._load (module.js:458:3)
        at Function.Module.runMain (module.js:605:10)
  ...
not ok 103 parallel/test-buffer-negative-length
  ---
  duration_ms: 0.407
  severity: fail
  stack: |-
    Mismatched <anonymous> function calls. Expected exactly 1, actual 5.
        at Object.exports.mustCall (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:481:10)
        at Object.expectsError (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:707:27)
        at Object.<anonymous> (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-buffer-negative-length.js:7:34)
        at Module._compile (module.js:569:30)
        at Object.Module._extensions..js (module.js:580:10)
        at Module.load (module.js:503:32)
        at tryModuleLoad (module.js:466:12)
        at Function.Module._load (module.js:458:3)
        at Function.Module.runMain (module.js:605:10)
  ...
not ok 105 parallel/test-buffer-no-negative-allocation
  ---
  duration_ms: 0.407
  severity: fail
  stack: |-
    Mismatched <anonymous> function calls. Expected exactly 1, actual 12.
        at Object.exports.mustCall (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:481:10)
        at Object.expectsError (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:707:27)
        at Object.<anonymous> (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-buffer-no-negative-allocation.js:6:20)
        at Module._compile (module.js:569:30)
        at Object.Module._extensions..js (module.js:580:10)
        at Module.load (module.js:503:32)
        at tryModuleLoad (module.js:466:12)
        at Function.Module._load (module.js:458:3)
        at Function.Module.runMain (module.js:605:10)
  ...
not ok 107 parallel/test-buffer-over-max-length
  ---
  duration_ms: 0.409
  severity: fail
  stack: |-
    Mismatched <anonymous> function calls. Expected exactly 1, actual 12.
        at Object.exports.mustCall (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:481:10)
        at Object.expectsError (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:707:27)
        at Object.<anonymous> (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-buffer-over-max-length.js:10:33)
        at Module._compile (module.js:569:30)
        at Object.Module._extensions..js (module.js:580:10)
        at Module.load (module.js:503:32)
        at tryModuleLoad (module.js:466:12)
        at Function.Module._load (module.js:458:3)
        at Function.Module.runMain (module.js:605:10)
  ...
not ok 112 parallel/test-buffer-regression-649
  ---
  duration_ms: 0.408
  severity: fail
  stack: |-
    Mismatched <anonymous> function calls. Expected exactly 1, actual 5.
        at Object.exports.mustCall (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:481:10)
        at Object.expectsError (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:707:27)
        at Object.<anonymous> (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-buffer-regression-649.js:9:24)
        at Module._compile (module.js:569:30)
        at Object.Module._extensions..js (module.js:580:10)
        at Module.load (module.js:503:32)
        at tryModuleLoad (module.js:466:12)
        at Function.Module._load (module.js:458:3)
        at Function.Module.runMain (module.js:605:10)
  ...
not ok 117 parallel/test-buffer-slow
  ---
  duration_ms: 0.416
  severity: fail
  stack: |-
    Mismatched <anonymous> function calls. Expected exactly 1, actual 2.
        at Object.exports.mustCall (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:481:10)
        at Object.expectsError (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:707:27)
        at Object.<anonymous> (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-buffer-slow.js:51:33)
        at Module._compile (module.js:569:30)
        at Object.Module._extensions..js (module.js:580:10)
        at Module.load (module.js:503:32)
        at tryModuleLoad (module.js:466:12)
        at Function.Module._load (module.js:458:3)
        at Function.Module.runMain (module.js:605:10)
  ...
not ok 123 parallel/test-buffer-write
  ---
  duration_ms: 0.410
  severity: fail
  stack: |-
    Mismatched <anonymous> function calls. Expected exactly 1, actual 2.
        at Object.exports.mustCall (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:481:10)
        at Object.expectsError (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:707:27)
        at Object.<anonymous> (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-buffer-write.js:6:30)
        at Module._compile (module.js:569:30)
        at Object.Module._extensions..js (module.js:580:10)
        at Module.load (module.js:503:32)
        at tryModuleLoad (module.js:466:12)
        at Function.Module._load (module.js:458:3)
        at Function.Module.runMain (module.js:605:10)
  ...
not ok 1417 parallel/test-writeint
  ---
  duration_ms: 0.407
  severity: fail
  stack: |-
    Mismatched <anonymous> function calls. Expected exactly 1, actual 12.
        at Object.exports.mustCall (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:481:10)
        at Object.expectsError (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:707:27)
        at Object.<anonymous> (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-writeint.js:28:33)
        at Module._compile (module.js:569:30)
        at Object.Module._extensions..js (module.js:580:10)
        at Module.load (module.js:503:32)
        at tryModuleLoad (module.js:466:12)
        at Function.Module._load (module.js:458:3)
        at Function.Module.runMain (module.js:605:10)
  ...
not ok 1418 parallel/test-writeuint
  ---
  duration_ms: 0.415
  severity: fail
  stack: |-
    Mismatched <anonymous> function calls. Expected exactly 1, actual 2.
        at Object.exports.mustCall (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:481:10)
        at Object.expectsError (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:707:27)
        at testUint (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-writeuint.js:152:27)
        at Object.<anonymous> (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-writeuint.js:171:1)
        at Module._compile (module.js:569:30)
        at Object.Module._extensions..js (module.js:580:10)
        at Module.load (module.js:503:32)
        at tryModuleLoad (module.js:466:12)
        at Function.Module._load (module.js:458:3)
    Mismatched <anonymous> function calls. Expected exactly 1, actual 2.
        at Object.exports.mustCall (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:481:10)
        at Object.expectsError (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:707:27)
        at testUint (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-writeuint.js:152:27)
        at Object.<anonymous> (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-writeuint.js:171:1)
        at Module._compile (module.js:569:30)
        at Object.Module._extensions..js (module.js:580:10)
        at Module.load (module.js:503:32)
        at tryModuleLoad (module.js:466:12)
        at Function.Module._load (module.js:458:3)
    Mismatched <anonymous> function calls. Expected exactly 1, actual 2.
        at Object.exports.mustCall (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:481:10)
        at Object.expectsError (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:707:27)
        at testUint (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-writeuint.js:152:27)
        at Object.<anonymous> (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-writeuint.js:171:1)
        at Module._compile (module.js:569:30)
        at Object.Module._extensions..js (module.js:580:10)
        at Module.load (module.js:503:32)
        at tryModuleLoad (module.js:466:12)
        at Function.Module._load (module.js:458:3)
    Mismatched <anonymous> function calls. Expected exactly 1, actual 2.
        at Object.exports.mustCall (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:481:10)
        at Object.expectsError (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:707:27)
        at testUint (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-writeuint.js:152:27)
        at Object.<anonymous> (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-writeuint.js:171:1)
        at Module._compile (module.js:569:30)
        at Object.Module._extensions..js (module.js:580:10)
        at Module.load (module.js:503:32)
        at tryModuleLoad (module.js:466:12)
        at Function.Module._load (module.js:458:3)
    Mismatched <anonymous> function calls. Expected exactly 1, actual 2.
        at Object.exports.mustCall (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:481:10)
        at Object.expectsError (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:707:27)
        at testUint (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-writeuint.js:152:27)
        at Object.<anonymous> (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-writeuint.js:171:1)
        at Module._compile (module.js:569:30)
        at Object.Module._extensions..js (module.js:580:10)
        at Module.load (module.js:503:32)
        at tryModuleLoad (module.js:466:12)
        at Function.Module._load (module.js:458:3)
    Mismatched <anonymous> function calls. Expected exactly 1, actual 2.
        at Object.exports.mustCall (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:481:10)
        at Object.expectsError (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:707:27)
        at testUint (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-writeuint.js:152:27)
        at Object.<anonymous> (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-writeuint.js:171:1)
        at Module._compile (module.js:569:30)
        at Object.Module._extensions..js (module.js:580:10)
        at Module.load (module.js:503:32)
        at tryModuleLoad (module.js:466:12)
        at Function.Module._load (module.js:458:3)
  ... 

@refack
Copy link
Contributor

refack commented Jul 12, 2017

Just help thing along I rebased and added the missing call count in the tests
Quick test: https://ci.nodejs.org/job/node-test-commit-linuxone/7240/

@refack
Copy link
Contributor

refack commented Jul 12, 2017

Full CI: https://ci.nodejs.org/job/node-test-pull-request/9095/ CI is ✔️ (except for known unrelated issues)

PR-URL: nodejs#13976
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@refack refack merged commit dbfe8c4 into nodejs:master Jul 12, 2017
@starkwang
Copy link
Contributor Author

@refack Thank you for your continued support! :D

@refack
Copy link
Contributor

refack commented Jul 13, 2017

@refack Thank you for your continued support! :D

This project progresses by the efforts of contributors such as yourself (Collaborators are here mostly to support that)!
Keep up the great work 🥇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations. errors Issues and PRs related to JavaScript errors originated in Node.js core. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants