Node: Expose the existing mechanisms behind `assert.deepEqual`

[osher]

Is your feature request related to a problem? Please describe.

I've been working on a testing tools set as part of some new proprietary ecosystem
(with the intent to open-source elements from it once they are stable enough in their form and require less maintenance, but so far it's proprietary :( ).
As part of this testing framework, I need to perform a deep comparison of objects.

As a start - I got assert.deepEqual - which is great. However - it ends with an exception.
Sometimes I would like to manage the exception myself - I just need the indication.

I decided to open this issue when I came across more use-cases:

• data healer - need to act on a decision if a model does not comply with expected values.
  For this - we need a form of containsEql.
• suite setup & teardown - need to add test-fixture data, and validate that essential objects are part of the target env (e.g. object describing super-user, or root categories, root tags, etc).

In all of the cases, it's beneficial to be able to perform both containsEql and deepEqual.

I did see util.isDeepStrictEqual - and despite the confusing name that gave me hope.

(I mean, it took me a lot of tries to understand that strict does not mean reference-comparison, but comparison without conversion of values. The first time I saw it I struggled to understand what it means - does it compare shallow copies? I had to try and see for myself. Anyway - I'm side tracking)

Describe the solution you'd like
I would propose on the same spirit of util.isDeepStrictEqual(..)
util.isDeepStrictContain(..)

I also saw the underlying mechanisms with the bStirct flag, and thought it could also be useful to let users inject it if they so choose, but I'm not sure about the names or the form it should take.

I'll be happy to start work on it if we can agree on the form it should take.

I mean, the mechanisms are there, and their functionality is all well-tested.
We just need to feature them and add only tests that show they are available, or even reuse parts of the existing tests - but expect a boolean instead of an error.

Describe alternatives you've considered
All current alternatives come from userland codes, which basically duplicate the same logic and introduce their own takes and flavors. Which is a pity...
We have that logic in node, and it's a good logic that runs deep and already serves now a role in APIs used by userland.

[BridgeAR]

The strict in the utility function is indeed unfortunate.

I am fine to expand the functionality in a similar fashion as laid out here. I would like to have options to define specific behaviour such as abstractEquality and prototypeCheck. To do so, we should add a new function without the strict in the name.

Adding the contains functionality is a tiny bit more work and we should make sure the performance won't degrade in case we combine the logic.

[benjamingr]

As a start - I got assert.deepEqual - which is great. However - it ends with an exception.
Sometimes I would like to manage the exception myself - I just need the indication.

Can you explain why (other than API ugliness) you can't accomplish this by wrapping the assert with a try/catch and seeing if an exception was thrown?

util.isDeepStrictContains(..)

That sounds reasonable to me, but I'd like to understand how that behaves - and if we add an isDeepStrict... variant we should also add an assert variant most likely.

[osher]

@benjamingr

wrapping the assert with a try/catch and seeing if an exception was thrown?

Sure. Try-Catch is a heavy mechanism. The construction of an error is heavy and throwing and catching it is heavy. Transpiled async wraps it in Promise.reject is lighter than throw - but it still goes through the constructor and the stack-trace gathering.
More of then than not I don't mind that in tests - but I do mind that on production code.
(how purist of me... but yes)

we should also add an assert variant most likely

Hmm. That makes sense to me.
My base assumption is that we're using existing logic, no new wheels. We'll have to start and see where does this road go

It's a dangerous business, Frodo, going out your door. You step onto the road, and if you don't keep your feet, there's no knowing where you might be swept off to.

:wink:

@BridgeAR
I'd love to do a performance bench, we have to get a starting point of reference to measure the change.
What I usually do is open the node shell and write snippets based on Date.now or even process.hrtime if the difference is not that evident.
see example here.
However, in many cases it's hard to get an overwhelming conclusion, and harder test it manually across OS.

How would you propose to test performance?
Is there a way such a test is found in this codebase you can point me to as a reference?

[BridgeAR]

There are enough benchmarks in ./benchmark/assert/*. It's possible to compare before and after by running something like: node benchmark/compare.js --old ./nodeOld --new ./nodeNew assert > assert.csv and afterwards cat assert.csv | Rscript benchmark/compare.R.

[osher]

@BridgeAR - that's great. I found the benches.
did not quite understand the compare command. what should the --old and --new parameters provide? path to a node compilation? saved snap of the js sources?

MM. I also understand I have to install R runetime or something?

[BridgeAR]

@osher the parameters are documented in benchmark/compare.js. The mentioned ones are binaries and it allows to compare different Node.js versions.

R is used to transform the raw data in nice to read output such as x percent increase / decrease between different benchmarks.

[lundibundi]

@osher take a look at this guide for detailed explanation https://github.com/nodejs/node/blob/master/doc/guides/writing-and-running-benchmarks.md.

[osher]

awsome. cool stuff.
just what I was looking for!