FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

errors, buffer: Migrate buffer errors to use internal/errors by starkwang · Pull Request #13976 · nodejs/node · GitHub

/ node Public

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

Merged
refack merged 1 commit into
nodejs:masterfrom
starkwang:buffer-internal-errors
Jul 12, 2017
ConversationCommits (1)ChecksFiles changed
Merged

errors, buffer: Migrate buffer errors to use internal/errors#13976
refack merged 1 commit into
nodejs:masterfrom
starkwang:buffer-internal-errors

Conversation

starkwang commented Jun 29, 2017

Copy link
Copy Markdown
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 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
starkwang mentioned this pull request Jun 29, 2017
Closed
80 tasks
mhdawson reviewed Jun 29, 2017
View reviewed changes
Comment thread lib/buffer.js Outdated

mhdawson Jun 29, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

starkwang Jun 30, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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?

Comment thread lib/buffer.js Outdated

mhdawson Jun 29, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

same comment as above

Comment thread lib/buffer.js Outdated

mhdawson Jun 29, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

same comment as above

Comment thread lib/buffer.js Outdated

mhdawson Jun 29, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

same comment as above

Comment thread lib/buffer.js Outdated

mhdawson Jun 29, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

starkwang Jul 1, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

fixed.

mhdawson reviewed Jun 29, 2017
View reviewed changes
Comment thread lib/buffer.js Outdated

mhdawson Jun 29, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

starkwang Jun 30, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

fixed.

starkwang force-pushed the buffer-internal-errors branch from cd92a68 to 735590b Compare June 30, 2017 16:30
jasnell reviewed Jun 30, 2017
View reviewed changes
Comment thread doc/api/errors.md Outdated

jasnell Jun 30, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

starkwang Jul 1, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

fixed.

starkwang force-pushed the buffer-internal-errors branch 5 times, most recently from 07afe26 to 1778514 Compare July 1, 2017 15:58
refack previously requested changes Jul 5, 2017
View reviewed changes
Comment thread test/parallel/test-buffer-from.js Outdated

refack Jul 5, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

starkwang Jul 5, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

fixed

Comment thread test/common/index.js Outdated

refack Jul 5, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Would this work is these were just strings?

starkwang Jul 5, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

starkwang Jul 5, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

fixed

Comment thread lib/internal/errors.js Outdated

refack Jul 5, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

starkwang Jul 5, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

fixed

Comment thread doc/api/errors.md Outdated

refack Jul 5, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality 
Used when a Node.js API in called in an unsupported manner.

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

starkwang Jul 5, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

fixed

starkwang force-pushed the buffer-internal-errors branch 3 times, most recently from ed482e1 to 512cb57 Compare July 5, 2017 08:14
starkwang commented Jul 5, 2017
View reviewed changes
Comment thread lib/buffer.js Outdated

starkwang Jul 5, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

mhdawson Jul 6, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It should probably be a RangeError

starkwang commented Jul 5, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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

mhdawson reviewed Jul 6, 2017
View reviewed changes
Comment thread lib/buffer.js Outdated

mhdawson Jul 6, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

mhdawson Jul 6, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Sames goes for other similar instances.

refack Jul 6, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

mhdawson reviewed Jul 6, 2017
View reviewed changes
Comment thread lib/buffer.js Outdated

mhdawson Jul 6, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

starkwang Jul 7, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

mhdawson reviewed Jul 6, 2017
View reviewed changes
Comment thread lib/buffer.js Outdated

mhdawson Jul 6, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think it should likely be 'string encoding'

mhdawson Jul 6, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

refack Jul 6, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

starkwang Jul 7, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think UNKNOWN_ENCODING is more suitable for this.

mhdawson reviewed Jul 6, 2017
View reviewed changes
Comment thread lib/buffer.js Outdated

mhdawson Jul 6, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

refack Jul 6, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

mhdawson reviewed Jul 6, 2017
View reviewed changes
Comment thread lib/buffer.js Outdated

mhdawson Jul 6, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

refack Jul 6, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

https://nodejs.org/api/buffer.html#buffer_buf_equals_otherbuffer - otherBuffer

mhdawson commented Jul 6, 2017

Copy link
Copy Markdown
Member

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

refack reviewed Jul 6, 2017
View reviewed changes
Comment thread doc/api/errors.md Outdated

refack Jul 6, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

sorry, in -> is

Comment thread lib/buffer.js Outdated

refack Jul 6, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Comment thread lib/buffer.js Outdated

refack Jul 6, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Comment thread test/common/index.js Outdated

refack Jul 6, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 dismissed their stale review July 6, 2017 20:28

CR addressed

refack commented Jul 6, 2017

Copy link
Copy Markdown
Contributor

@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 force-pushed the buffer-internal-errors branch 2 times, most recently from dd2bdc4 to 7c28ac2 Compare July 7, 2017 07:28

starkwang commented Jul 7, 2017

Copy link
Copy Markdown
Contributor Author

Pushed commit to address comments.

starkwang force-pushed the buffer-internal-errors branch 2 times, most recently from 4a6cd45 to ac9dc54 Compare July 7, 2017 14:03

starkwang commented Jul 7, 2017

Copy link
Copy Markdown
Contributor Author

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

refack commented Jul 7, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

refack reviewed Jul 7, 2017
View reviewed changes
Comment thread test/common/index.js Outdated

refack Jul 7, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

starkwang Jul 7, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

refack Jul 7, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

That's what the CI is for

starkwang force-pushed the buffer-internal-errors branch from ac9dc54 to 4035559 Compare July 7, 2017 22:47

refack commented Jul 7, 2017

Copy link
Copy Markdown
Contributor

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

starkwang mentioned this pull request Jul 8, 2017
Closed
2 tasks
starkwang force-pushed the buffer-internal-errors branch from 4035559 to bcde08c Compare July 9, 2017 03:51
mcollina approved these changes Jul 10, 2017
View reviewed changes

mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

jasnell commented Jul 12, 2017

Copy link
Copy Markdown
Member

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

refack commented Jul 12, 2017

Copy link
Copy Markdown
Contributor

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 force-pushed the buffer-internal-errors branch from bcde08c to 0b2a6cb Compare July 12, 2017 17:00

refack commented Jul 12, 2017

Copy link
Copy Markdown
Contributor

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 commented Jul 12, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

errors,buffer: port errors to internal/errors
dbfe8c4 
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 force-pushed the buffer-internal-errors branch from 0b2a6cb to dbfe8c4 Compare July 12, 2017 21:01
refack merged commit dbfe8c4 into nodejs:master Jul 12, 2017

starkwang commented Jul 13, 2017

Copy link
Copy Markdown
Contributor Author

@refack Thank you for your continued support! :D

refack commented Jul 13, 2017

Copy link
Copy Markdown
Contributor

@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 🥇

This was referenced Jul 15, 2017
Closed
Closed
Trott mentioned this pull request Jul 24, 2017
Closed
2 tasks
refack mentioned this pull request Jul 24, 2017
Closed
jasnell mentioned this pull request Oct 30, 2017
Merged
reviewher mentioned this pull request Nov 13, 2017
Closed
refack removed their assignment Oct 20, 2018
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

mcollina mcollina approved these changes
jasnell jasnell approved these changes
benjamingr benjamingr approved these changes
+2 more reviewers
refack refack approved these changes
mhdawson mhdawson left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL