Commit Graph

36 Commits

Author SHA1 Message Date
Ruben Bridgewater
02c44a4894
assert: reduce diff noise
Assertion errors that produce a diff show a diff for identical entries
in case one side of the comparison has more object properties than the
other one. Those lines are now taken into account and will not show up
as diverging lines anymore.

Refs: https://github.com/nodejs/node/issues/22763

PR-URL: https://github.com/nodejs/node/pull/23048
Refs: https://github.com/nodejs/node/issues/22763
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2018-09-27 13:36:23 +02:00
Ruben Bridgewater
b8a8eedf32
assert: switch inputs to values
The wording seems clearer when using `values` instead of `inputs`.

PR-URL: https://github.com/nodejs/node/pull/23056
Refs: https://github.com/nodejs/node/issues/22763
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
2018-09-27 13:25:18 +02:00
Ruben Bridgewater
28902f33aa
assert: improve diff output
The output is now a tiny bit improved by sorting object properties
when inspecting the values that are compared with each other. That
reduces the overall diff for identical objects with a different
property insertion order.

PR-URL: https://github.com/nodejs/node/pull/22788
Refs: https://github.com/nodejs/node/issues/22763
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
2018-09-19 13:22:32 +02:00
Ruben Bridgewater
be5e3964b3
assert: fix loose set and map comparison
The fast path did not anticipate different ways to express a loose
equal string value (e.g., 1n == '+0001'). This is now fixed with the
downside that all primitives that could theoretically have equal
entries must go through a full comparison.

Only some strings, symbols, undefined, null and NaN can be detected
in a fast path as those entries have a strictly limited set of
possible equal entries.

PR-URL: https://github.com/nodejs/node/pull/22495
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
2018-09-05 01:49:22 +02:00
Ruben Bridgewater
3479b1ca82
util,assert: improve performance
This significantly improves regular and typed array performance by
not checking the indices keys anymore. This can be done with a V8
API that allows to only retrieve the non indices property keys.

PR-URL: https://github.com/nodejs/node/pull/22197
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
2018-08-20 02:11:46 +02:00
Ruben Bridgewater
d164d9d606
util,assert: improve comparison performance
This adds a smarter logic to compare object keys (including symbols)
and it also skips the object key comparison for (typed) arrays, if
possible.

Besides that it adds a fast path for empty objects, arrays, sets and
maps and fast paths for sets and maps with an unequal size.

On top of that a few functions are now safer to call by using
uncurryThis and by caching the actual function.

Overall, this is a significant performance boost for comparisons.

PR-URL: https://github.com/nodejs/node/pull/22258
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
2018-08-15 17:43:15 +02:00
Ruben Bridgewater
c3bd65146e test: improve test coverage for comparisons
PR-URL: https://github.com/nodejs/node/pull/22212
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2018-08-12 10:40:10 -07:00
Ruben Bridgewater
c7ca199c38
util,assert: fix boxed primitives bug
Currently the comparison could throw an error in case a boxed
primitive has no valueOf function on one side of the assert call.

PR-URL: https://github.com/nodejs/node/pull/22243
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: George Adams <george.adams@uk.ibm.com>
2018-08-12 18:38:58 +01:00
Ruben Bridgewater
85bfd71312
assert: fix loose assert with map and set
There was an oversight when checking the possible loose comparisons.
This is now fixed by also accepting `''` instead of `0` or `false`.

PR-URL: https://github.com/nodejs/node/pull/22145
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
2018-08-09 14:10:01 +02:00
Ruben Bridgewater
1d859ef532
assert: improve loose assertion message
So far the error message from a loose assertion is not always very
informative and could be misleading. This is fixed by:

* showing more from the actual error message
* having a better error description
* not using custom inspection
* inspecting a higher depth
* inspecting the full array instead of up to 100 entries

PR-URL: https://github.com/nodejs/node/pull/22155
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
2018-08-09 14:04:30 +02:00
Ruben Bridgewater
0518b9edf3
assert: multiple improvements
1) Switched + / - and red / green in diffs. It seems like that style
   is more natural to most people.

2) Short primitives do not use the diff anymore. Especially short
   numbers can be read well like 1 !== 2. Cases that can not be
   displayed like that (e.g., -0 and +0) use the regular diff output.

3) Improved error descriptions. It was not always clear what the
   messages stood for. That should now be resolved.

4) Added a position indicator for single lines in case a tty is used
   and the line is shorter than the visual columns.

5) Color detection is now done by checking stderr instead of stdout.

PR-URL: https://github.com/nodejs/node/pull/21628
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
2018-08-04 10:04:32 +02:00
cjihrig
7597d45bee
test: disable colors in test-assert-deep.js
When test/parallel/test-assert-deep.js is run with a TTY as
stdout, color codes in assertion messages cause the test to fail.
This commit disables colors when stdout is a TTY.

PR-URL: https://github.com/nodejs/node/pull/20695
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2018-05-14 23:39:16 -04:00
Ruben Bridgewater
bfe54df812 assert: improve default error messages
This improves the error messages for:
- assert.notDeepStrictEqual
- assert.deepStrictEqual
- assert.notStrictEqual
- assert.strictEqual

Those will now always use the same error message as used in the
strict mode.

PR-URL: https://github.com/nodejs/node/pull/19467
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2018-04-14 10:43:43 -07:00
Ruben Bridgewater
644fdd60d4
test: minor refactoring
Add punctuation and comments about code that should not throw.
Also remove a obsolete test and refactor some tests.

PR-URL: https://github.com/nodejs/node/pull/18669
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
2018-02-16 16:54:07 +01:00
Ruben Bridgewater
caee112e52
test: remove assert.doesNotThrow()
There is actually no reason to use `assert.doesNotThrow()` in the
tests. If a test throws, just let the error bubble up right away
instead of first catching it and then rethrowing it.

PR-URL: https://github.com/nodejs/node/pull/18669
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
2018-02-16 16:53:47 +01:00
Ruben Bridgewater
9b4aa78f72
test: refactor assert test
This adds puctiations to the comments, uses a capital letters for
the first character, removes a few obsolete comments and switches
to assert.ok when suitable.

It also moves all `assert.deepEqual()` and `assert.deepStrictEqual()`
tests to the appropriate file.

PR-URL: https://github.com/nodejs/node/pull/18610
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
2018-02-10 02:39:14 +01:00
Rich Trott
203b548381
test: add block scoping to test-assert-deep
Add block scoping to test-assert-deep.js to reduce likelihood of one
test case having side effects that affect another test case.

PR-URL: https://github.com/nodejs/node/pull/16532
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
2017-10-29 20:05:12 +01:00
Ruben Bridgewater
db2e093e05
assert: handle enumerable symbol keys
PR-URL: https://github.com/nodejs/node/pull/15169
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
2017-09-19 20:13:02 -03:00
Ruben Bridgewater
b0d3bec95c
assert: use Same-value equality in deepStrictEqual
PR-URL: https://github.com/nodejs/node/pull/15398
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
2017-09-15 23:37:22 -03:00
Ruben Bridgewater
22ae8c0248
assert: fix boxed primitives in deepStrictEqual
Unbox all primitives and compare them as well instead of
only comparing boxed strings.

PR-URL: https://github.com/nodejs/node/pull/15050
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
2017-09-12 17:29:37 -03:00
Ruben Bridgewater
ea2e6363f2
assert: use SameValueZero in deepStrictEqual
Comparing NaN will not throw anymore.

PR-URL: https://github.com/nodejs/node/pull/15036
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
2017-09-05 08:15:33 -03:00
Ruben Bridgewater
204d94fc75
assert: handle errors properly with deep*Equal
PR-URL: https://github.com/nodejs/node/pull/15001
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2017-09-01 02:00:15 -03:00
Ruben Bridgewater
4381100371
assert: handle sparse arrays in deepStrictEqual
Detect sparse array ends and add a fail early path for
unequal array length.

PR-URL: https://github.com/nodejs/node/pull/15027
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
2017-08-28 23:19:03 -03:00
Ruben Bridgewater
8db39971b7 test: clean up some assert deepEqual tests
PR-URL: https://github.com/nodejs/node/pull/14491
Fixes: https://github.com/nodejs/node/issues/14441
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
2017-08-01 13:27:39 +02:00
Ruben Bridgewater
a8149c4799 assert: fix deepEqual inconsistencies
PR-URL: https://github.com/nodejs/node/pull/14491
Fixes: https://github.com/nodejs/node/issues/14441
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
2017-08-01 13:26:07 +02:00
Sebastiaan Deckers
bb29405904
lib,src: fix consistent spacing inside braces
PR-URL: https://github.com/nodejs/node/pull/14162
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2017-07-21 15:13:47 -04:00
Ruben Bridgewater
5203bb0b16
assert: improve deepEqual Set and Map worst case
This change improves the algorithm for the worst case from O(n^2)
to O(n log n) by using a lazily initiated set with object or
not strict equal primitives keys.

In addition a few comments got fixed and a statement simplified.

PR-URL: https://github.com/nodejs/node/pull/14258
Reviewed-By: Refael Ackermann <refack@gmail.com>
2017-07-19 19:25:46 -04:00
Ruben Bridgewater
d6fece1436
test: add optional throw fn to expectsError
PR-URL: https://github.com/nodejs/node/pull/14089
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
2017-07-08 16:00:57 -04:00
Vse Mozhet Byt
76340e3f10 test: fix RegExp nits
* Remove needless RegExp flag

  In fixed case, `/g` flag is needless in the boolean context.

* Remove needless RegExp capturing

  Use non-capturing grouping or remove capturing completely when:

  * capturing is useless per se, e.g. in test() check;
  * captured groups are not used afterward at all;
  * some of the later captured groups are not used afterward.

* Use test, not match/exec in boolean context

  match() and exec() return a complicated object,
  unneeded in a boolean context.

* Do not needlessly repeat RegExp creation

  This commit takes RegExp creation out of cycles and other repetitions.

  As long as the RegExp does not use /g flag and match indices,
  we are safe here.

  In tests, this fix hardly gives a significant performance gain,
  but it increases clarity and maintainability,
  reassuring some RegExps to be identical.

  RegExp in functions are not taken out of their functions:
  while these functions are called many times
  and their RegExps are recreated with each call,
  the performance gain in test cases
  does not seem to be worth decreasing function self-dependency.

PR-URL: https://github.com/nodejs/node/pull/13770
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
2017-06-21 03:40:27 +03:00
Joseph Gentle
7cddcc9715 assert: fix deepEqual similar sets and maps bug
This fixes a bug where deepEqual and deepStrictEqual would have
incorrect behaviour in sets and maps containing multiple equivalent
keys.

PR-URL: https://github.com/nodejs/node/pull/13426
Fixes: https://github.com/nodejs/node/issues/13347
Refs: https://github.com/nodejs/node/pull/12142
Reviewed-By: Refael Ackermann <refack@gmail.com>
2017-06-05 18:33:49 -04:00
rmdm
b1ed55f259 assert: fix deepEqual RangeError: Maximum call stack size exceeded
Fixes: https://github.com/nodejs/node/issues/13314
Refs: https://github.com/nodejs/node/issues/6416

This commit changes semantics of the memos cycles tracker. Before
the change it was used to track all currently wisited nodes of an
object tree, which is a bit shifted from its original intention of
tracking cycles. The change brings intended semantics, by tracking
only objects of the current branch of the object tree.

PR-URL: https://github.com/nodejs/node/pull/13318
Fixes: https://github.com/nodejs/node/issues/13314
Ref: https://github.com/nodejs/node/issues/6416
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
2017-06-01 20:48:59 -07:00
James M Snell
e48d58b8b2 assert: fix AssertionError, assign error code
Using `assert.AssertionError()` without the `new` keyword results
in a non-intuitive error:

```js
> assert.AssertionError({})
TypeError: Cannot assign to read only property 'name' of function 'function ok(value, message) {
  if (!value) fail(value, true, message, '==', assert.ok);
}'
    at Function.AssertionError (assert.js:45:13)
    at repl:1:8
    at realRunInThisContextScript (vm.js:22:35)
    at sigintHandlersWrap (vm.js:98:12)
    at ContextifyScript.Script.runInThisContext (vm.js:24:12)
    at REPLServer.defaultEval (repl.js:346:29)
    at bound (domain.js:280:14)
    at REPLServer.runBound [as eval] (domain.js:293:12)
    at REPLServer.onLine (repl.js:545:10)
    at emitOne (events.js:101:20)
>
```

The `assert.AssertionError()` can only be used correctly with `new`,
so this converts it into a proper ES6 class that will give an
appropriate error message.

This also associates the appropriate internal/errors code with all
`assert.AssertionError` instances and updates the appropriate test
cases.

PR-URL: https://github.com/nodejs/node/pull/12651
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
2017-05-04 07:18:17 -07:00
Michaël Zasso
9de2e159c4 test: add second argument to assert.throws
This adds RegExp or error constructor arguments to the remaining places
where it is missing in preparation for the commit that will enforce the
presence of at least two arguments.

PR-URL: https://github.com/nodejs/node/pull/12270
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
2017-04-13 11:31:39 +02:00
Rich Trott
971fe67dce test: complete coverage for lib/assert.js
6481c93a modified `lib/assert.js` and added some tests for new
functionality, but left a single line uncovered by tests. This adds a
test that covers the currently-uncovered line (which is the final
`return` statement in `setHasSimilarElement()`).`

PR-URL: https://github.com/nodejs/node/pull/12239
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
2017-04-07 11:12:29 -07:00
Joseph Gentle
6481c93aef
assert: add support for Map and Set in deepEqual
assert.deepEqual and assert.deepStrictEqual currently return true for
any pair of Maps and Sets regardless of content. This patch adds
support in deepEqual and deepStrictEqual to verify the contents of Maps
and Sets.

Deeo equivalence checking is currently an
O(n^2) operation, and worse, it gets slower exponentially if maps
and sets were nested.

Note that this change breaks compatibility with previous versions of
deepEqual and deepStrictEqual if consumers were depending on all maps
and sets to be seen as equivalent. The old behaviour was never
documented, but nevertheless there are certainly some tests out there
which depend on it.

Support has stalled because the assert API was frozen, but was recently
unfrozen in CTC#63.

---

Later squashed in:

This change updates the checks for deep equality checking on Map and Set
to check all set values / all map keys to see if any of them match the
expected result.

This change is much slower, but based on the conversation in the pull
request its probably the right approach.

Fixes: https://github.com/nodejs/node/issues/2309
Refs: https://github.com/substack/tape/issues/342
Refs: https://github.com/nodejs/node/pull/2315
Refs: https://github.com/nodejs/CTC/issues/63
PR-URL: https://github.com/nodejs/node/pull/12142
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
2017-04-03 10:15:53 +02:00
Joyee Cheung
562cf5a81c assert: fix premature deep strict comparison
Refactors _deepEqual and fixes a few code paths that lead to
behaviors contradicting what the doc says. Before this commit
certain types of objects (Buffers, Dates, etc.) are not checked
properly, and can get away with different prototypes AND different
enumerable owned properties because _deepEqual would jump to
premature conclusion for them.

Since we no longer follow CommonJS unit testing spec,
the checks for primitives and object prototypes are moved
forward for faster failure.

Improve regexp and float* array checks:

* Don't compare lastIndex of regexps, because they are not
  enumerable, so according to the docs they should not be compared
* Compare flags of regexps instead of separate properties
* Use built-in tags to test for float* arrays instead of using
  instanceof

Use full link to the archived GitHub repository.

Use util.objectToString for future improvements to that function
that makes sure the call won't be tampered with.

PR-URL: https://github.com/nodejs/node/pull/11128
Refs: https://github.com/nodejs/node/pull/10282#issuecomment-274267895
Refs: https://github.com/nodejs/node/issues/10258#issuecomment-266963234
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
2017-02-27 18:39:40 +08:00