assert: Fix deepEqual/deepStrictEqual on equivalent typed arrays

[feross]

Checklist

• make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
• tests are included
• commit message follows commit guidelines

Affected core subsystem(s)

assert

Description of change

Fix #8001.

The typed array's underlying ArrayBuffer is used in Buffer.from.
Let's respect it's .byteOffset or .byteLength (i.e. position within the
parent ArrayBuffer).

[feross]

For some reason, I'm getting the following test error after this change:

../test/cctest/test_inspector_socket.cc:360: Failure
Value of: err
  Actual: -16
Expected: 0
[  FAILED  ] InspectorSocketTest.WriteBeforeHandshake (1 ms)

Any ideas?

[addaleax]

That test failure looks unrelated and it doesn’t happen for me, does it persist or was it just a one-off? (/cc @ofrobots @pavelfeldman @eugeneo? Maybe you would want to create a @nodejs/v8-inspector team or something for easier pinging?)

The code change itself looks good to me (thanks for noticing this!). You might want to use something like a trailing Fixes: https://github.com/nodejs/node/issues/8001 line in the commit message instead of Fix #8001 (full URLs are generally preferred for accessibility and distinction from the v0.x repo), and if you have the time, a regression test would be awesome.

CI run: https://ci.nodejs.org/job/node-test-commit/4438/

[ofrobots]

I can reproduce the failure on master on a mac. Seems like a regression that the CI is missing? @addaleax what platform did you try on? BTW, I have also created @nodejs/v8-inspector. Thanks for the idea.

[addaleax]

@ofrobots Linux 4.4.0-31-generic #50-Ubuntu SMP Wed Jul 13 00:07:12 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux, Ubuntu 16.04. So yeah, seems totally possible that this is platform-specific.

[ofrobots]

Bisect points to 0190db4. I have opened a separate issue for this: #8006.
EDIT: grammar.

[princejwesley]

I can reproduce the failure on master branch/mac platform. I tried running make cctest multiple times. Sometimes, InspectorSocketTest is running without failure.

[feross]

I fixed up the commit message and added a regression test. Thanks, @addaleax!

Since the test failures look unrelated, is this ready to merge?

[addaleax]

@feross I obviously can’t speak for everyone here, but this LGTM! The commits will be squashed and the subject line shortened to something at most 50 characters in length; that can be done by you or when landing the PR, though.

[princejwesley]

LGTM

[jasnell]

This is a great catch @feross ... LGTM

[cjihrig]

LGTM

[jasnell]

Another CI run since there was a (seemingly unrelated) failure in the last one, just to be safe: https://ci.nodejs.org/job/node-test-pull-request/3569/

[jasnell]

Landed in 387ab62 and a8438a0

[MylesBorins]

v4.x appears to have the same problem with the example provided in #8001. Buffer.from does not exist in v4.x, but we can replicate this fix with the original buffer interface. Unfortunately there are still failures when the patch is applied

Uint8Array { '0': 2, '1': 3, '2': 4 }
Uint8Array { '0': 2, '1': 3, '2': 4 }

assert.js:89
  throw new assert.AssertionError({
  ^
AssertionError: Uint8Array { '0': 2, '1': 3, '2': 4 } deepEqual Uint8Array { '0': 2, '1': 3, '2': 4 }
    at Object.<anonymous> (/private/var/folders/ty/q7q6b07j5r3c7nvnpzm6hkq40000gn/T/test/example.js:9:8)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Function.Module.runMain (module.js:441:10)
    at startup (node.js:139:18)
    at node.js:990:3

@feross or @jasnell would you be willing to look into backporting?

[feross]

@thealphanerd I'd love to, but I don't have the time at the moment. If this doesn't get addressed in a few weeks, feel free to ping me again and I'll do my best :)

[MylesBorins]

@feross any more time lately?

[feross]

@thealphanerd Unfortunately, no. Hopefully someone else can pick this up.