test: migrate message util tests from Python to JS #49721 #50333
Conversation
nodejs-github-bot
added
needs-ci
| [90m at Module._compile (node:internal*modules*cjs*loader**)[39m | ||
| [90m at Module._extensions..js (node:internal*modules*cjs*loader**)[39m | ||
| [90m at Module.load (node:internal*modules*cjs*loader**)[39m | ||
| [90m at Module._load (node:internal*modules*cjs*loader**)[39m | ||
| [90m at Function.executeUserEntryPoint [as runMain] (node:internal*modules*run_main**)[39m | ||
| [90m at node:internal*main*run_main_module**[39m |
There was a problem hiding this comment.
We don’t want these internals in the snapshots. Renaming a method within the module loader shouldn’t break this unrelated test.
There was a problem hiding this comment.
Hey @GeoffreyBooth or @MoLow , I have a question. I'm making my edits in this PR after realizing I was going about it the wrong way. I'm having a hard time knowing when it's okay to edit the snapshot in order to get the tests to pass. I'm using past commits as a reference. But my question is, for the following, can I edit the snapshot to take " * " or should I edit my JS file to replace " * " with "90m" and "39m"?
+ actual - expected
'RangeError: New Stack Frames\n' +
' at *\n' +
+ '*[*m at *[*m {\n' +
- '*[90m at *[39m {\n' +
There was a problem hiding this comment.
You should not edit the snapshots manually, use NODE_REGENERATE_SNAPSHOTS=1 for regenerating snapshots.
you can probably remove colors from the snapshot
str = str.replace(/[^\x00-\x7F]/g, '').replace(/\u001b\[\d+m/g, '')
There was a problem hiding this comment.
@MoLow The linter don't like the control characters.
22:12 error Unexpected control character(s) in regular expression: \x00 no-control-regex
23:12 error Unexpected control character(s) in regular expression: \x1b no-control-regex
✖ 2 problems (2 errors, 0 warnings)
| Error: Number error cause | ||
| at * | ||
| at * | ||
| at * | ||
| at * | ||
| at * | ||
| at * | ||
| at * { | ||
| [cause]: 42 |
There was a problem hiding this comment.
Add Error.stackTraceLimit = 0 (or 1) before the test is run to remove these call stack lines that we don’t care about. Search for Error.stackTraceLimit in other tests to see examples.
There was a problem hiding this comment.
Added the following in the .js file locally
Error.stackTraceLimit = 7;
and it worked for me
There was a problem hiding this comment.
Using 0 instead of 7 should help suppress the stack trace lines that show the module internals (my other note).
There was a problem hiding this comment.
@GeoffreyBooth I'm assuming I should be adding the line in my test-node-output-util.js file and not the files under fixtures/util? I ran into issues doing that, but not when adding the line to my file under parallel.
There was a problem hiding this comment.
I don’t remember. If you search for Error.stackTraceLimit = 0 in the other test files you can copy the pattern they use.
Co-authored-by: Moshe Atlow <[email protected]>
| at Object.<anonymous> *[*m(**[*mtest*fixtures*util*util-inspect-error-cause.js:*:**[*m)*[*m | ||
| *[*m at Module._compile (node:internal*modules*cjs*loader:*:*)*[*m | ||
| *[*m at Module._extensions..js (node:internal*modules*cjs*loader:*:*)*[*m | ||
| *[*m at Module.load (node:internal*modules*cjs*loader:*:*)*[*m | ||
| *[*m at Module._load (node:internal*modules*cjs*loader:*:*)*[*m | ||
| *[*m at Function.executeUserEntryPoint [as runMain] (node:internal*modules*run_main:*:*)*[*m | ||
| *[*m at *[*m |
There was a problem hiding this comment.
None of these functions should be in the stack trace. I should be able to refactor the internals of the CommonJS loader without breaking unrelated tests.
Migrate the remaining util tests in the test/message folder from Python to JS.
Fixes: #47707
Migrated tests: