util: improve inspects RegExp support #25192
Conversation
nodejs-github-bot
added
the
util
This adds support for inspect to distinguish regular expression subclasses and ones with null prototype from "normal" regular expressions.
So far we do not test all data types for subclasses and this extends the existing tests for WeakSet, WeakMap and BigInt64Array.
BridgeAR
force-pushed
the
improve-reg-exp-support
branch
from
9fd659e
to
eabee50
Compare
|
AIX failure is a known issue (#24305). I'll open a PR to mark it as flaky. Pi issue is a new one. It's a crash rather than a simpler test failure and it could be something unrelated to the test itself. Will open an issue for it to track if it recurs. Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/19769/ ✔️ |
| const expectedWithoutProto = `[${base.name}: null prototype] ${rawExpected}`; | ||
| assert.strictEqual(util.inspect(value), expected); | ||
| value.foo = 'bar'; | ||
| assert.notStrictEqual(util.inspect(value), expected); |
There was a problem hiding this comment.
One small suggestion: may be we want to pull out the test cases for properties in a separate block. Would be happy to see strictEqual rather than notStrictEqual(as it can silently fail on wrong op's).
There was a problem hiding this comment.
We have lots of tests which verify that the properties are actually displayed correct and they always hit the same code path. Therefore verifying that it's a different result should be sufficient.
Below there's also another test to verify that at least the property is indeed displayed as expected.
The reason for not using the strict comparison here is that each data type has different ways of displaying some parts and it's difficult to generalize that.
There was a problem hiding this comment.
Sure, it is sufficient as you said above but thought it could be better using strictEqual. Anyways this is not a blocker to land.
antsmartian
added
the
author ready
|
@nodejs/util PTAL. This needs another review. |
This adds support for inspect to distinguish regular expression subclasses and ones with null prototype from "normal" regular expressions. PR-URL: nodejs#25192 Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
So far we do not test all data types for subclasses and this extends the existing tests for WeakSet, WeakMap and BigInt64Array. PR-URL: nodejs#25192 Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This adds support for inspect to distinguish regular expression subclasses and ones with null prototype from "normal" regular expressions. PR-URL: #25192 Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
So far we do not test all data types for subclasses and this extends the existing tests for WeakSet, WeakMap and BigInt64Array. PR-URL: #25192 Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This adds support for inspect to distinguish regular expression subclasses and ones with null prototype from "normal" regular expressions. PR-URL: nodejs#25192 Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
So far we do not test all data types for subclasses and this extends the existing tests for WeakSet, WeakMap and BigInt64Array. PR-URL: nodejs#25192 Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Please check the commit messages as description.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes