Skip to content

assert: make assert.fail doc compliant - #13862

Merged
refack merged 2 commits into
nodejs:masterfrom
BridgeAR:improve-assert
Jul 3, 2017
Merged

assert: make assert.fail doc compliant#13862
refack merged 2 commits into
nodejs:masterfrom
BridgeAR:improve-assert

Conversation

@BridgeAR

@BridgeARBridgeAR commented Jun 22, 2017

Copy link
Copy Markdown
Member

I refactored the assert module a tiny bit. Now assert.fail is actually doc compliant and has no fifth argument anymore.
While doing that I was also able to remove the lazy assert call in the AssertionError constructor.
I also removed the lazy loading of the errors in assert. As the errors lazy load assert it is fine to load the errors in assert upfront.

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)

assert

@nodejs-github-botnodejs-github-bot added assert Issues and PRs related to the assert subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core. labels Jun 22, 2017
@BridgeARBridgeAR changed the title assert: make doc compliantassert: make assert.fail doc compliantJun 22, 2017
Comment threadlib/assert.js Outdated

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.

Why not just fail? (I'm looking at this on GitHub so I don't see the whole scope)

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

See the response above

Comment threadlib/assert.js Outdated

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.

I've been looking around /lib/ there is no standard, but IMHO _name functions are usualy exposed-but-considered-private. You can give this one a more explicit name like, failImpl or failInner

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I'm not a huge fan of renaming all of these functions but I don't have strong feelings about it, so now it's inner*.

Comment threadlib/assert.js Outdated

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.

I see no harm in keep this modules style and doing:

functionfail(){
...
}assert.fail=fail;

Or am I wrong and there's a name clash?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

It was not about the style here but about the line length. The only other function that was special was ok as that was accessed directly before declaration. This change is actually also the reason why I wrote assert.fail and not only fail as the function was not visible otherwise. I changed changed it in a way that seemed most convenient to me.

Comment threadlib/assert.js Outdated

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.

Wait, what? 🤦‍♂️

Comment threadlib/assert.js Outdated

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.

Also could you remove all the /*optional*/ is driving my IDE (WebStorm) crazy.

Comment threadlib/assert.js Outdated

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.

I know it was a mess before, but could you clean this up a bit:

constexpectedName=(expected&&expected.name ? ` (${expected.name})` : '');constformatted=(message ? `: ${message}` : '.');constfinalMessage=expectedName+formatted;

@refack

Copy link
Copy Markdown
Contributor

Good change (commented on some nits, mosley style so optional, and non-blocking), great that you broke the dependency cycle!
I think if you give it another pass you can clean it up some more, especially _throws (ohh now I get why you chose _fail, IMHO change them both).

@refack

Copy link
Copy Markdown
Contributor

@jasnelljasnell added the semver-major PRs that contain breaking changes and should be released in the next major version. label Jun 22, 2017
@jasnell

Copy link
Copy Markdown
Member

@nodejs/ctc ... PTAL

@jasnell

Copy link
Copy Markdown
Member

This is necessarily semver-major, even if the prior signature was not documented.

@BridgeAR

Copy link
Copy Markdown
MemberAuthor

Comments addressed. I was actually able to simplify a couple more things in the innerThrows function.

@BridgeAR

Copy link
Copy Markdown
MemberAuthor

@jasnell we could actually go the other way around and simply document the fifth argument instead of removing it. In that case it would be semver-minor instead of semver-major.

@BridgeAR

Copy link
Copy Markdown
MemberAuthor

Missed the changed line length. I just fixed that

@refack

Copy link
Copy Markdown
Contributor

joyeecheung
joyeecheung previously approved these changes Jun 24, 2017
@BridgeAR

Copy link
Copy Markdown
MemberAuthor

I have two more commits that use this currently as a base and they lead to some nice benchmark improvements:

Benchmarks
 improvement confidence p.value
assert/deepequal-buffer.js method="deepEqual" len=100 n=500000 2.33 % 7.316323e-01
assert/deepequal-buffer.js method="deepStrictEqual" len=100 n=500000 0.04 % 9.930054e-01
assert/deepequal-buffer.js method="notDeepEqual" len=100 n=500000 -5.85 % 2.784226e-01
assert/deepequal-buffer.js method="notDeepStrictEqual" len=100 n=500000 -5.59 % 3.001234e-01
assert/deepequal-object.js method="deepEqual" size=100 n=10000 50.46 % *** 2.288171e-18
assert/deepequal-object.js method="deepEqual" size=1000 n=1000 67.95 % *** 3.059217e-30
assert/deepequal-object.js method="deepEqual" size=10000 n=100 79.66 % *** 9.207480e-33
assert/deepequal-object.js method="deepStrictEqual" size=100 n=10000 65.35 % *** 3.240965e-23
assert/deepequal-object.js method="deepStrictEqual" size=1000 n=1000 81.71 % *** 1.205872e-28
assert/deepequal-object.js method="deepStrictEqual" size=10000 n=100 94.75 % *** 5.241022e-29
assert/deepequal-object.js method="notDeepEqual" size=100 n=10000 229.85 % *** 1.889593e-23
assert/deepequal-object.js method="notDeepEqual" size=1000 n=1000 608.17 % *** 4.146461e-26
assert/deepequal-object.js method="notDeepEqual" size=10000 n=100 1062.95 % *** 2.408876e-33
assert/deepequal-object.js method="notDeepStrictEqual" size=100 n=10000 242.26 % *** 1.586300e-22
assert/deepequal-object.js method="notDeepStrictEqual" size=1000 n=1000 622.27 % *** 2.609994e-26
assert/deepequal-object.js method="notDeepStrictEqual" size=10000 n=100 1064.28 % *** 5.299886e-27
assert/deepequal-prims-and-objs-big-array-set.js method="deepEqual Array" len=100000 n=25 prim="array" 510.02 % *** 2.595144e-27
assert/deepequal-prims-and-objs-big-array-set.js method="deepEqual Array" len=100000 n=25 prim="boolean" 515.89 % *** 3.743109e-28
assert/deepequal-prims-and-objs-big-array-set.js method="deepEqual Array" len=100000 n=25 prim="new-array" 513.81 % *** 3.622029e-33
assert/deepequal-prims-and-objs-big-array-set.js method="deepEqual Array" len=100000 n=25 prim="null" 519.45 % *** 5.168069e-32
assert/deepequal-prims-and-objs-big-array-set.js method="deepEqual Array" len=100000 n=25 prim="number" 506.67 % *** 1.843979e-28
assert/deepequal-prims-and-objs-big-array-set.js method="deepEqual Array" len=100000 n=25 prim="object" 516.84 % *** 1.017218e-31
assert/deepequal-prims-and-objs-big-array-set.js method="deepEqual Array" len=100000 n=25 prim="string" 511.24 % *** 2.280310e-30
assert/deepequal-prims-and-objs-big-array-set.js method="deepEqual Array" len=100000 n=25 prim="undefined" 515.58 % *** 4.776631e-30
assert/deepequal-prims-and-objs-big-array-set.js method="deepEqual Set" len=100000 n=25 prim="array" 31.11 % *** 9.964406e-23
assert/deepequal-prims-and-objs-big-array-set.js method="deepEqual Set" len=100000 n=25 prim="boolean" 31.64 % *** 1.781798e-19
assert/deepequal-prims-and-objs-big-array-set.js method="deepEqual Set" len=100000 n=25 prim="new-array" 30.52 % *** 4.417665e-20
assert/deepequal-prims-and-objs-big-array-set.js method="deepEqual Set" len=100000 n=25 prim="null" 26.33 % *** 2.951717e-06
assert/deepequal-prims-and-objs-big-array-set.js method="deepEqual Set" len=100000 n=25 prim="number" 19.30 % *** 3.005158e-07
assert/deepequal-prims-and-objs-big-array-set.js method="deepEqual Set" len=100000 n=25 prim="object" 29.05 % *** 1.158809e-17
assert/deepequal-prims-and-objs-big-array-set.js method="deepEqual Set" len=100000 n=25 prim="string" 31.94 % *** 2.672107e-16
assert/deepequal-prims-and-objs-big-array-set.js method="deepEqual Set" len=100000 n=25 prim="undefined" 25.14 % *** 1.475360e-04
assert/deepequal-prims-and-objs-big-array-set.js method="deepStrictEqual Array" len=100000 n=25 prim="array" 512.38 % *** 9.062277e-30
assert/deepequal-prims-and-objs-big-array-set.js method="deepStrictEqual Array" len=100000 n=25 prim="boolean" 520.17 % *** 1.775437e-28
assert/deepequal-prims-and-objs-big-array-set.js method="deepStrictEqual Array" len=100000 n=25 prim="new-array" 510.74 % *** 1.518219e-28
assert/deepequal-prims-and-objs-big-array-set.js method="deepStrictEqual Array" len=100000 n=25 prim="null" 519.06 % *** 2.101566e-28
assert/deepequal-prims-and-objs-big-array-set.js method="deepStrictEqual Array" len=100000 n=25 prim="number" 504.35 % *** 1.429286e-25
assert/deepequal-prims-and-objs-big-array-set.js method="deepStrictEqual Array" len=100000 n=25 prim="object" 512.10 % *** 1.181912e-28
assert/deepequal-prims-and-objs-big-array-set.js method="deepStrictEqual Array" len=100000 n=25 prim="string" 515.83 % *** 4.265263e-32
assert/deepequal-prims-and-objs-big-array-set.js method="deepStrictEqual Array" len=100000 n=25 prim="undefined" 514.06 % *** 2.112530e-28
assert/deepequal-prims-and-objs-big-array-set.js method="deepStrictEqual Set" len=100000 n=25 prim="array" 29.32 % *** 3.259328e-19
assert/deepequal-prims-and-objs-big-array-set.js method="deepStrictEqual Set" len=100000 n=25 prim="boolean" 24.08 % *** 3.541972e-05
assert/deepequal-prims-and-objs-big-array-set.js method="deepStrictEqual Set" len=100000 n=25 prim="new-array" 31.80 % *** 7.752780e-26
assert/deepequal-prims-and-objs-big-array-set.js method="deepStrictEqual Set" len=100000 n=25 prim="null" 31.01 % *** 2.799161e-26
assert/deepequal-prims-and-objs-big-array-set.js method="deepStrictEqual Set" len=100000 n=25 prim="number" 28.69 % *** 9.321106e-15
assert/deepequal-prims-and-objs-big-array-set.js method="deepStrictEqual Set" len=100000 n=25 prim="object" 29.79 % *** 3.149769e-22
assert/deepequal-prims-and-objs-big-array-set.js method="deepStrictEqual Set" len=100000 n=25 prim="string" 30.36 % *** 5.547048e-24
assert/deepequal-prims-and-objs-big-array-set.js method="deepStrictEqual Set" len=100000 n=25 prim="undefined" 33.25 % *** 9.415303e-15
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepEqual Array" len=100000 n=25 prim="array" 498.52 % *** 4.150282e-28
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepEqual Array" len=100000 n=25 prim="boolean" 503.56 % *** 4.086493e-27
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepEqual Array" len=100000 n=25 prim="new-array" 499.51 % *** 8.200783e-30
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepEqual Array" len=100000 n=25 prim="null" 502.45 % *** 7.676734e-30
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepEqual Array" len=100000 n=25 prim="number" 497.00 % *** 5.051971e-29
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepEqual Array" len=100000 n=25 prim="object" 504.16 % *** 9.443596e-27
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepEqual Array" len=100000 n=25 prim="string" 500.16 % *** 6.245976e-29
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepEqual Array" len=100000 n=25 prim="undefined" 506.09 % *** 5.548733e-30
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepEqual Set" len=100000 n=25 prim="array" 38.99 % *** 2.117152e-21
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepEqual Set" len=100000 n=25 prim="boolean" 41.34 % *** 8.039099e-13
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepEqual Set" len=100000 n=25 prim="new-array" 34.59 % *** 4.047460e-08
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepEqual Set" len=100000 n=25 prim="null" 40.82 % *** 1.008765e-11
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepEqual Set" len=100000 n=25 prim="number" 41.32 % *** 3.700953e-18
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepEqual Set" len=100000 n=25 prim="object" 43.00 % *** 3.259377e-15
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepEqual Set" len=100000 n=25 prim="string" 41.71 % *** 2.297337e-16
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepEqual Set" len=100000 n=25 prim="undefined" 42.77 % *** 1.315782e-25
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepStrictEqual Array" len=100000 n=25 prim="array" 500.61 % *** 2.938624e-25
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepStrictEqual Array" len=100000 n=25 prim="boolean" 509.20 % *** 3.660616e-29
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepStrictEqual Array" len=100000 n=25 prim="new-array" 502.57 % *** 2.517452e-29
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepStrictEqual Array" len=100000 n=25 prim="null" 507.65 % *** 1.022300e-31
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepStrictEqual Array" len=100000 n=25 prim="number" 495.83 % *** 2.382429e-28
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepStrictEqual Array" len=100000 n=25 prim="object" 506.00 % *** 1.020662e-29
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepStrictEqual Array" len=100000 n=25 prim="string" 504.18 % *** 8.501536e-32
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepStrictEqual Array" len=100000 n=25 prim="undefined" 508.25 % *** 8.319102e-37
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepStrictEqual Set" len=100000 n=25 prim="array" 39.18 % *** 1.174312e-23
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepStrictEqual Set" len=100000 n=25 prim="boolean" 41.92 % *** 6.687850e-17
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepStrictEqual Set" len=100000 n=25 prim="new-array" 38.17 % *** 7.104991e-24
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepStrictEqual Set" len=100000 n=25 prim="null" 38.26 % *** 1.134893e-17
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepStrictEqual Set" len=100000 n=25 prim="number" 39.22 % *** 9.407632e-21
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepStrictEqual Set" len=100000 n=25 prim="object" 40.20 % *** 5.403801e-14
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepStrictEqual Set" len=100000 n=25 prim="string" 39.22 % *** 6.780387e-24
assert/deepequal-prims-and-objs-big-array-set.js method="notDeepStrictEqual Set" len=100000 n=25 prim="undefined" 36.45 % *** 2.965572e-10
assert/deepequal-prims-and-objs-big-loop.js method="deepEqual" n=1000000 prim="array" -3.34 % *** 9.582538e-09
assert/deepequal-prims-and-objs-big-loop.js method="deepEqual" n=1000000 prim="boolean" -3.16 % *** 1.354914e-05
assert/deepequal-prims-and-objs-big-loop.js method="deepEqual" n=1000000 prim="new-array" -5.34 % *** 2.144227e-04
assert/deepequal-prims-and-objs-big-loop.js method="deepEqual" n=1000000 prim="null" -3.63 % *** 6.492770e-07
assert/deepequal-prims-and-objs-big-loop.js method="deepEqual" n=1000000 prim="number" -3.15 % ** 6.193770e-03
assert/deepequal-prims-and-objs-big-loop.js method="deepEqual" n=1000000 prim="object" -2.63 % ** 1.635324e-03
assert/deepequal-prims-and-objs-big-loop.js method="deepEqual" n=1000000 prim="string" -3.61 % *** 5.859919e-07
assert/deepequal-prims-and-objs-big-loop.js method="deepEqual" n=1000000 prim="undefined" -3.80 % *** 8.281596e-05
assert/deepequal-prims-and-objs-big-loop.js method="deepStrictEqual" n=1000000 prim="array" 6.93 % *** 3.282310e-10
assert/deepequal-prims-and-objs-big-loop.js method="deepStrictEqual" n=1000000 prim="boolean" 6.47 % *** 2.540042e-11
assert/deepequal-prims-and-objs-big-loop.js method="deepStrictEqual" n=1000000 prim="new-array" 5.41 % *** 1.114413e-08
assert/deepequal-prims-and-objs-big-loop.js method="deepStrictEqual" n=1000000 prim="null" 7.37 % *** 8.377163e-11
assert/deepequal-prims-and-objs-big-loop.js method="deepStrictEqual" n=1000000 prim="number" 5.75 % *** 2.572949e-11
assert/deepequal-prims-and-objs-big-loop.js method="deepStrictEqual" n=1000000 prim="object" 6.25 % *** 2.319331e-07
assert/deepequal-prims-and-objs-big-loop.js method="deepStrictEqual" n=1000000 prim="string" 5.74 % *** 8.195711e-14
assert/deepequal-prims-and-objs-big-loop.js method="deepStrictEqual" n=1000000 prim="undefined" 4.92 % *** 2.495528e-04
assert/deepequal-prims-and-objs-big-loop.js method="notDeepEqual" n=1000000 prim="array" 27.98 % *** 3.872747e-20
assert/deepequal-prims-and-objs-big-loop.js method="notDeepEqual" n=1000000 prim="boolean" -2.18 % ** 3.618656e-03
assert/deepequal-prims-and-objs-big-loop.js method="notDeepEqual" n=1000000 prim="new-array" 28.03 % *** 3.314333e-28
assert/deepequal-prims-and-objs-big-loop.js method="notDeepEqual" n=1000000 prim="null" -1.86 % * 2.765366e-02
assert/deepequal-prims-and-objs-big-loop.js method="notDeepEqual" n=1000000 prim="number" -2.71 % ** 3.011431e-03
assert/deepequal-prims-and-objs-big-loop.js method="notDeepEqual" n=1000000 prim="object" 25.20 % *** 4.464850e-16
assert/deepequal-prims-and-objs-big-loop.js method="notDeepEqual" n=1000000 prim="string" -2.22 % * 1.423177e-02
assert/deepequal-prims-and-objs-big-loop.js method="notDeepEqual" n=1000000 prim="undefined" -1.91 % * 2.223972e-02
assert/deepequal-prims-and-objs-big-loop.js method="notDeepStrictEqual" n=1000000 prim="array" 20.30 % *** 6.285555e-22
assert/deepequal-prims-and-objs-big-loop.js method="notDeepStrictEqual" n=1000000 prim="boolean" 7.98 % *** 2.964258e-14
assert/deepequal-prims-and-objs-big-loop.js method="notDeepStrictEqual" n=1000000 prim="new-array" 20.61 % *** 1.250668e-21
assert/deepequal-prims-and-objs-big-loop.js method="notDeepStrictEqual" n=1000000 prim="null" 7.87 % *** 1.945697e-09
assert/deepequal-prims-and-objs-big-loop.js method="notDeepStrictEqual" n=1000000 prim="number" 7.78 % *** 3.784016e-13
assert/deepequal-prims-and-objs-big-loop.js method="notDeepStrictEqual" n=1000000 prim="object" 18.37 % *** 2.987944e-18
assert/deepequal-prims-and-objs-big-loop.js method="notDeepStrictEqual" n=1000000 prim="string" 8.33 % *** 7.416061e-16
assert/deepequal-prims-and-objs-big-loop.js method="notDeepStrictEqual" n=1000000 prim="undefined" 7.66 % *** 5.268512e-14
assert/deepequal-typedarrays.js len=1000000 method="deepEqual" n=1 type="Float32Array" 825.51 % *** 1.769654e-26
assert/deepequal-typedarrays.js len=1000000 method="deepEqual" n=1 type="Float64Array" 830.17 % *** 7.208875e-30
assert/deepequal-typedarrays.js len=1000000 method="deepEqual" n=1 type="Int16Array" 0.74 % 3.572383e-01
assert/deepequal-typedarrays.js len=1000000 method="deepEqual" n=1 type="Int32Array" 0.33 % 8.281712e-01
assert/deepequal-typedarrays.js len=1000000 method="deepEqual" n=1 type="Int8Array" 2.12 % 1.004341e-01
assert/deepequal-typedarrays.js len=1000000 method="deepEqual" n=1 type="Uint16Array" 0.97 % 3.406747e-01
assert/deepequal-typedarrays.js len=1000000 method="deepEqual" n=1 type="Uint32Array" -0.53 % 7.429723e-01
assert/deepequal-typedarrays.js len=1000000 method="deepEqual" n=1 type="Uint8Array" 1.22 % 2.354753e-01
assert/deepequal-typedarrays.js len=1000000 method="deepEqual" n=1 type="Uint8ClampedArray" 1.94 % 1.373714e-01
assert/deepequal-typedarrays.js len=1000000 method="deepStrictEqual" n=1 type="Float32Array" 828.24 % *** 1.710967e-27
assert/deepequal-typedarrays.js len=1000000 method="deepStrictEqual" n=1 type="Float64Array" 829.86 % *** 6.907388e-29
assert/deepequal-typedarrays.js len=1000000 method="deepStrictEqual" n=1 type="Int16Array" 0.47 % 4.531061e-01
assert/deepequal-typedarrays.js len=1000000 method="deepStrictEqual" n=1 type="Int32Array" 1.08 % 1.685682e-01
assert/deepequal-typedarrays.js len=1000000 method="deepStrictEqual" n=1 type="Int8Array" 0.48 % 4.986249e-01
assert/deepequal-typedarrays.js len=1000000 method="deepStrictEqual" n=1 type="Uint16Array" 0.26 % 7.337130e-01
assert/deepequal-typedarrays.js len=1000000 method="deepStrictEqual" n=1 type="Uint32Array" -0.07 % 8.645550e-01
assert/deepequal-typedarrays.js len=1000000 method="deepStrictEqual" n=1 type="Uint8Array" -0.35 % 5.936551e-01
assert/deepequal-typedarrays.js len=1000000 method="deepStrictEqual" n=1 type="Uint8ClampedArray" -0.05 % 8.619136e-01
assert/deepequal-typedarrays.js len=1000000 method="notDeepEqual" n=1 type="Float32Array" 986.37 % *** 5.592733e-27
assert/deepequal-typedarrays.js len=1000000 method="notDeepEqual" n=1 type="Float64Array" 987.71 % *** 2.506846e-26
assert/deepequal-typedarrays.js len=1000000 method="notDeepEqual" n=1 type="Int16Array" 993.93 % *** 2.601085e-38
assert/deepequal-typedarrays.js len=1000000 method="notDeepEqual" n=1 type="Int32Array" 984.40 % *** 3.427367e-29
assert/deepequal-typedarrays.js len=1000000 method="notDeepEqual" n=1 type="Int8Array" 988.12 % *** 8.782491e-30
assert/deepequal-typedarrays.js len=1000000 method="notDeepEqual" n=1 type="Uint16Array" 987.60 % *** 8.585867e-27
assert/deepequal-typedarrays.js len=1000000 method="notDeepEqual" n=1 type="Uint32Array" 981.23 % *** 2.542324e-29
assert/deepequal-typedarrays.js len=1000000 method="notDeepEqual" n=1 type="Uint8Array" 2.99 % 2.962923e-01
assert/deepequal-typedarrays.js len=1000000 method="notDeepEqual" n=1 type="Uint8ClampedArray" 982.95 % *** 2.418260e-27
assert/deepequal-typedarrays.js len=1000000 method="notDeepStrictEqual" n=1 type="Float32Array" 4.20 % 6.680757e-02
assert/deepequal-typedarrays.js len=1000000 method="notDeepStrictEqual" n=1 type="Float64Array" 1.02 % 7.778071e-01
assert/deepequal-typedarrays.js len=1000000 method="notDeepStrictEqual" n=1 type="Int16Array" 5.87 % ** 7.262476e-03
assert/deepequal-typedarrays.js len=1000000 method="notDeepStrictEqual" n=1 type="Int32Array" 6.40 % 9.935904e-02
assert/deepequal-typedarrays.js len=1000000 method="notDeepStrictEqual" n=1 type="Int8Array" 5.51 % * 1.328911e-02
assert/deepequal-typedarrays.js len=1000000 method="notDeepStrictEqual" n=1 type="Uint16Array" 5.81 % ** 8.281852e-03
assert/deepequal-typedarrays.js len=1000000 method="notDeepStrictEqual" n=1 type="Uint32Array" 7.68 % ** 1.617419e-03
assert/deepequal-typedarrays.js len=1000000 method="notDeepStrictEqual" n=1 type="Uint8Array" -1.32 % 4.273639e-01
assert/deepequal-typedarrays.js len=1000000 method="notDeepStrictEqual" n=1 type="Uint8ClampedArray" 4.02 % * 4.113977e-02
assert/throws.js method="doesNotThrow RegExp" n=1000000 261.80 % *** 6.572196e-24
assert/throws.js method="doesNotThrow TypeError" n=1000000 571.93 % *** 8.693018e-27
assert/throws.js method="doesNotThrow" n=1000000 127.76 % *** 4.291609e-22
assert/throws.js method="throws RegExp" n=1000000 1.50 % ** 3.911706e-03
assert/throws.js method="throws TypeError" n=1000000 8.39 % *** 5.176500e-09
assert/throws.js method="throws" n=1000000 2.41 % *** 2.203446e-08

Should I push them here or open a separate PR for them and rebase accordingly?

@BridgeAR

Copy link
Copy Markdown
MemberAuthor

@refack@joyeecheung would you prefer to remove the fifth argument or would you rather document it to prevent this from being semver major?

@joyeecheung

joyeecheung commented Jun 26, 2017

Copy link
Copy Markdown
Member

@BridgeAR I don't have a strong opinion, although if we keep this as semver-major then we might as well get the optimization in this PR

@BridgeAR

Copy link
Copy Markdown
MemberAuthor

I just pushed to two commits so you can have a look at it.

@joyeecheung The performance improvements should not be semver-major as no external logic is changed.

@BridgeAR

Copy link
Copy Markdown
MemberAuthor

@mscdex would you mind to also take a look?

@refack

Copy link
Copy Markdown
Contributor

@refack

Copy link
Copy Markdown
Contributor

Should I push them here or open a separate PR for them and rebase accordingly?

IMHO yes since since they change https://benchmarking.nodejs.org/

@refack

refack commented Jun 26, 2017

Copy link
Copy Markdown
Contributor

@refack@joyeecheung would you prefer to remove the fifth argument or would you rather document it to prevent this from being semver major?

Originaly I was pro removal, now I think stackStartFunction should stay and be documented. This is a good PR that should be backported to v8.x and v6.x.

@refackrefack left a comment

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.

Bring back stackStartFunction and document, so this will be semver-patch

Comment threadlib/internal/errors.js Outdated

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.

IMHO in the signature replace constructor(options) with constructor({message, actual, operator, expected, stackStartFunction})

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I would like to change that as well but this would be a semver major as the error would change again... Therefore: maybe at a later point.

@refack

Copy link
Copy Markdown
Contributor

Doc suggestion:

## assert.fail(actual, expected, message, operator, constructorOpt)<!-- YAMLadded: v0.1.21changes: - version: v5.5.0 description: add `constructorOpt` parameter-->*`actual` {any}
*`expected` {any}
*`message` {any}
*`operator` {string} (default: '!=')
*`constructorOpt` {function} first stack frame to exclude. See [`Error.captureStackTrace`]
...
[`Error.captureStackTrace`]: errors.html#errors_error_capturestacktrace_targetobject_constructoropt

@BridgeARBridgeAR mentioned this pull request Jun 28, 2017
4 tasks
@BridgeAR

Copy link
Copy Markdown
MemberAuthor

I rebased this to only contain changes for the docs and stylistic changes. Everything else is in #13973

Instead of removing support for the fifth argument it is now documented.

refack pushed a commit to refack/node that referenced this pull request Jul 24, 2017
PR-URL: nodejs#13862
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
refack pushed a commit to refack/node that referenced this pull request Jul 24, 2017
1. Rename private functions
2. Use destructuring
3. Remove obsolete comments
PR-URL: nodejs#13862
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
addaleax pushed a commit that referenced this pull request Jul 24, 2017
PR-URL: #13862
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Backport-PR-URL: #14428
Backport-Reviewed-By: Anna Henningsen <anna@addaleax.net>
addaleax pushed a commit that referenced this pull request Jul 24, 2017
1. Rename private functions
2. Use destructuring
3. Remove obsolete comments
PR-URL: #13862
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Backport-PR-URL: #14428
Backport-Reviewed-By: Anna Henningsen <anna@addaleax.net>
@addaleaxaddaleax mentioned this pull request Aug 2, 2017
@BridgeARBridgeAR mentioned this pull request Sep 21, 2017
4 tasks
BridgeAR added a commit to BridgeAR/node that referenced this pull request Sep 21, 2017
1. Rename private functions
2. Use destructuring
3. Remove obsolete comments
PR-URL: nodejs#13862
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Sep 21, 2017
PR-URL: nodejs#13862
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
MylesBorins pushed a commit that referenced this pull request Sep 22, 2017
1. Rename private functions
2. Use destructuring
3. Remove obsolete comments
Backport-PR-URL: #15516
PR-URL: #13862
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
MylesBorins pushed a commit that referenced this pull request Sep 22, 2017
Backport-PR-URL: #15516
PR-URL: #13862
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
@MylesBorinsMylesBorins mentioned this pull request Sep 22, 2017
MylesBorins pushed a commit that referenced this pull request Sep 26, 2017
1. Rename private functions
2. Use destructuring
3. Remove obsolete comments
Backport-PR-URL: #15516
PR-URL: #13862
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
MylesBorins pushed a commit that referenced this pull request Sep 26, 2017
Backport-PR-URL: #15516
PR-URL: #13862
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
refack pushed a commit to refack/node that referenced this pull request Oct 10, 2017
* refactor the code
1. Rename private functions
2. Use destructuring
3. Remove obsolete comments
* remove eslint rule
PR-URL: nodejs#13862
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
@refackrefack removed their assignment Oct 20, 2018
@BridgeAR
BridgeAR deleted the improve-assert branch April 1, 2019 23:36
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

assertIssues and PRs related to the assert subsystem.errorsIssues and PRs related to JavaScript errors originated in Node.js core.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@BridgeAR@refack@jasnell@joyeecheung@addaleax@MylesBorins@nodejs-github-bot