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

assert.deepEqual is broken? #5907

Closed
mohayonao opened this issue Mar 26, 2016 · 1 comment
Closed

assert.deepEqual is broken? #5907

mohayonao opened this issue Mar 26, 2016 · 1 comment
Labels
assert Issues and PRs related to the assert subsystem. confirmed-bug Issues with confirmed bugs.

Comments

@mohayonao
Copy link

  • Node: v5.9.1
  • Platform: Darwin 15.4.0 Darwin Kernel Version 15.4.0: Fri Feb 26 22:08:05 PST 2016; root:xnu-3248.40.184~3/RELEASE_X86_64 x86_64

Problem:
Does not throw an exception, when comparing two Float32Array which have the small different values.

assert.deepEqual(new Float32Array([ 0 ]), new Float32Array([ 0.1 ]));
// undefined (This does not throw an AssertionError?)

This works as expected.

assert.deepEqual(new Float32Array([ 0 ]), new Float32Array([ 1 ]));
// AssertionError: Float32Array { '0': 0 } deepEqual Float32Array { '0': 1 }
//    at repl:1:8
//    at REPLServer.defaultEval (repl.js:260:27)
//    at bound (domain.js:287:14)
//    at REPLServer.runBound [as eval] (domain.js:300:12)
//    at REPLServer.<anonymous> (repl.js:429:12)
//    at emitOne (events.js:95:20)
//    at REPLServer.emit (events.js:182:7)
//    at REPLServer.Interface._onLine (readline.js:211:10)
//    at REPLServer.Interface._line (readline.js:550:8)
//    at REPLServer.Interface._ttyWrite (readline.js:827:14)
@brendanashworth brendanashworth added the assert Issues and PRs related to the assert subsystem. label Mar 26, 2016
@addaleax
Copy link
Member

This happens due to this line, where typed arrays are converted to Buffer objects entry-for-entry, i.e. each entry in the source becomes one uint8 in the Buffer. This is not specific to floats:

assert.deepEqual(new Float32Array([ 0 ]), new Float32Array([ 255 ])) // throws
assert.deepEqual(new Float32Array([ 0 ]), new Float32Array([ 256 ])) // passes, 256 becomes 0x00
assert.deepEqual(new Float32Array([ 0 ]), new Float32Array([ 257 ])) // throws

Same with Uint16Array etc.
I’ll see whether I can get a fix together.

@benjamingr benjamingr added the confirmed-bug Issues with confirmed bugs. label Mar 31, 2016
addaleax added a commit to addaleax/node that referenced this issue Mar 31, 2016
Do not convert typed arrays to `Buffer` for deepEqual since
their values may not be accurately represented by 8-bit ints.
Instead perform binary comparison of underlying `ArrayBuffer`s,
but only when the array types match.

Never apply any kind of optimization for floating-point typed
arrays since bit pattern equality is not the right kind of check
for them.

PR-URL: nodejs#5910
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Fixes: nodejs#5907
MylesBorins pushed a commit that referenced this issue Apr 5, 2016
Do not convert typed arrays to `Buffer` for deepEqual since
their values may not be accurately represented by 8-bit ints.
Instead perform binary comparison of underlying `ArrayBuffer`s,
but only when the array types match.

Never apply any kind of optimization for floating-point typed
arrays since bit pattern equality is not the right kind of check
for them.

PR-URL: #5910
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Fixes: #5907
addaleax added a commit to addaleax/node that referenced this issue Apr 11, 2016
Do not convert typed arrays to `Buffer` for deepEqual since
their values may not be accurately represented by 8-bit ints.
Instead perform binary comparison of underlying `ArrayBuffer`s,
but only when the array types match.

Never apply any kind of optimization for floating-point typed
arrays since bit pattern equality is not the right kind of check
for them.

PR-URL: nodejs#5910
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Fixes: nodejs#5907
MylesBorins pushed a commit that referenced this issue Apr 11, 2016
Do not convert typed arrays to `Buffer` for deepEqual since
their values may not be accurately represented by 8-bit ints.
Instead perform binary comparison of underlying `ArrayBuffer`s,
but only when the array types match.

Never apply any kind of optimization for floating-point typed
arrays since bit pattern equality is not the right kind of check
for them.

PR-URL: #5910
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Fixes: #5907
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assert Issues and PRs related to the assert subsystem. confirmed-bug Issues with confirmed bugs.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants