Skip to content

Conversation

@BridgeAR
Copy link
Member

@BridgeARBridgeAR commented Jun 22, 2017

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
lib/assert.js Outdated
Copy link
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
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

lib/assert.js Outdated
Copy link
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
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*.

lib/assert.js Outdated
Copy link
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
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.

lib/assert.js Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Wait, what? 🤦‍♂️

lib/assert.js Outdated
Copy link
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.

lib/assert.js Outdated
Copy link
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
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
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
Member

@nodejs/ctc ... PTAL

@jasnell
Copy link
Member

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

@BridgeAR
Copy link
MemberAuthor

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

@BridgeAR
Copy link
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
MemberAuthor

Missed the changed line length. I just fixed that

@refack
Copy link
Contributor

joyeecheung
joyeecheung previously approved these changes Jun 24, 2017
@BridgeAR
Copy link
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
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
Copy link
Member

joyeecheung commented Jun 26, 2017

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

@mscdex would you mind to also take a look?

@refack
Copy link
Contributor

@refack
Copy link
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
Copy link
Contributor

refack commented Jun 26, 2017

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

Copy link
Contributor

@refackrefack left a comment

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

Copy link
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
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
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
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 <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
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 <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
addaleax pushed a commit that referenced this pull request Jul 24, 2017
PR-URL: #13862 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Backport-PR-URL: #14428 Backport-Reviewed-By: Anna Henningsen <[email protected]>
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 <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Backport-PR-URL: #14428 Backport-Reviewed-By: Anna Henningsen <[email protected]>
@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 <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Sep 21, 2017
PR-URL: nodejs#13862 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
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 <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 22, 2017
Backport-PR-URL: #15516 PR-URL: #13862 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
@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 <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 26, 2017
Backport-PR-URL: #15516 PR-URL: #13862 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
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 <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
@refackrefack removed their assignment Oct 20, 2018
@BridgeARBridgeAR 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