Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: migrate message util tests from Python to JS #49721 #50333

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

test: migrate message util tests from Python to JS #49721 #50333

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d542f8e
test: migrate message util tests from Python to JS
jahjahLemonade Oct 23, 2023
068e098
moved message files to fixture folder
jahjahLemonade Oct 23, 2023
985dfc5
Run make lint
jahjahLemonade Oct 23, 2023
f5489c1
Added Error.stackTraceLimit
jahjahLemonade Oct 25, 2023
eae8b32
Rebasing Code
jahjahLemonade Nov 20, 2023
bdb6a39
Added additional regex into file
jahjahLemonade Nov 30, 2023
e399adb
Updated test-node-output-util.mjs
jahjahLemonade Dec 3, 2023
93e0aca
Update test/parallel/test-node-output-util.mjs
jahjahLemonade Dec 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion test/message/util-inspect-error-cause.js → ...fixtures/util/util-inspect-error-cause.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

require('../common');
require('../../common');

const { inspect } = require('util');

Expand Down
181 changes: 181 additions & 0 deletions test/fixtures/util/util-inspect-error-cause.snapshot
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
Error: Number error cause
at *
at *
at *
at *
at *
at *
at * {
[cause]: 42
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@jahjahLemonade jahjahLemonade Oct 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@GeoffreyBooth

Added the following in the .js file locally

Error.stackTraceLimit = 7;

and it worked for me

Copy link
Member

Choose a reason for hiding this comment

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

Using 0 instead of 7 should help suppress the stack trace lines that show the module internals (my other note).

Copy link
Contributor Author

@jahjahLemonade jahjahLemonade Oct 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

I don’t remember. If you search for Error.stackTraceLimit = 0 in the other test files you can copy the pattern they use.

}
Error: Object cause
at *
at *
at *
at *
at *
at *
at * {
[cause]: {
message: 'Unique',
name: 'Error',
stack: 'Error: Unique\n' +
' at Module._compile (node:internal*modules*cjs*loader**)'
}
}
Error: undefined cause
at *
at *
at *
at *
at *
at *
at * {
[cause]: undefined
}
Error: cause that throws
at *
at *
at *
at *
at *
at *
at * {
[cause]: [Getter]
}
RangeError: New Stack Frames
at *
 at process.processTicksAndRejections (node:internal*process*task_queues**) {
[cause]: FoobarError: Individual message
at Object.<anonymous> (*test*fixtures*util*util-inspect-error-cause.js**)
 at Module._compile (node:internal*modules*cjs*loader**)
 ... 4 lines matching cause stack trace ...
 at node:internal*main*run_main_module** {
status: 'Feeling good',
extraProperties: 'Yes!',
[cause]: TypeError: Inner error
at Object.<anonymous> (*test*fixtures*util*util-inspect-error-cause.js**)
 at Module._compile (node:internal*modules*cjs*loader**)
 at Module._extensions..js (node:internal*modules*cjs*loader**)
 at Module.load (node:internal*modules*cjs*loader**)
 at Module._load (node:internal*modules*cjs*loader**)
 at Function.executeUserEntryPoint [as runMain] (node:internal*modules*run_main**)
 at node:internal*main*run_main_module**
}
}
Error: Stack causes
at Object.<anonymous> (*test*fixtures*util*util-inspect-error-cause.js**)
 at Module._compile (node:internal*modules*cjs*loader**)
 ... 4 lines matching cause stack trace ...
 at node:internal*main*run_main_module** {
[cause]: FoobarError: Individual message
at Object.<anonymous> (*test*fixtures*util*util-inspect-error-cause.js**)
 at Module._compile (node:internal*modules*cjs*loader**)
 ... 4 lines matching cause stack trace ...
 at node:internal*main*run_main_module** {
status: 'Feeling good',
extraProperties: 'Yes!',
[cause]: TypeError: Inner error
at Object.<anonymous> (*test*fixtures*util*util-inspect-error-cause.js**)
 at Module._compile (node:internal*modules*cjs*loader**)
 at Module._extensions..js (node:internal*modules*cjs*loader**)
 at Module.load (node:internal*modules*cjs*loader**)
 at Module._load (node:internal*modules*cjs*loader**)
 at Function.executeUserEntryPoint [as runMain] (node:internal*modules*run_main**)
 at node:internal*main*run_main_module**
}
}
RangeError: New Stack Frames
at *
 at process.processTicksAndRejections (node:internal*process*task_queues**) {
[cause]: Error: Stack causes
at Object.<anonymous> (*test*fixtures*util*util-inspect-error-cause.js**)
 at Module._compile (node:internal*modules*cjs*loader**)
 ... 4 lines matching cause stack trace ...
 at node:internal*main*run_main_module** {
[cause]: FoobarError: Individual message
at Object.<anonymous> (*test*fixtures*util*util-inspect-error-cause.js**)
 at Module._compile (node:internal*modules*cjs*loader**)
 ... 4 lines matching cause stack trace ...
 at node:internal*main*run_main_module** {
status: 'Feeling good',
extraProperties: 'Yes!',
[cause]: TypeError: Inner error
at Object.<anonymous> (*test*fixtures*util*util-inspect-error-cause.js**)
 at Module._compile (node:internal*modules*cjs*loader**)
 at Module._extensions..js (node:internal*modules*cjs*loader**)
 at Module.load (node:internal*modules*cjs*loader**)
 at Module._load (node:internal*modules*cjs*loader**)
 at Function.executeUserEntryPoint [as runMain] (node:internal*modules*run_main**)
 at node:internal*main*run_main_module**
Copy link
Member

Choose a reason for hiding this comment

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

We don’t want these internals in the snapshots. Renaming a method within the module loader shouldn’t break this unrelated test.

Copy link
Contributor Author

@jahjahLemonade jahjahLemonade Oct 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

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' +

Copy link
Member

Choose a reason for hiding this comment

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

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, '')

Copy link
Contributor Author

@jahjahLemonade jahjahLemonade Nov 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

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

}
}
}
RangeError: New Stack Frames
at *
at * {
[cause]: FoobarError: Individual message
at *
at *
... 4 lines matching cause stack trace ...
at * {
status: 'Feeling good',
extraProperties: 'Yes!',
[cause]: TypeError: Inner error
at *
at *
at *
at *
at *
at *
at *
}
}
Error: Stack causes
at *
at *
... 4 lines matching cause stack trace ...
at * {
[cause]: FoobarError: Individual message
at *
at *
... 4 lines matching cause stack trace ...
at * {
status: 'Feeling good',
extraProperties: 'Yes!',
[cause]: TypeError: Inner error
at *
at *
at *
at *
at *
at *
at *
}
}
RangeError: New Stack Frames
at *
at * {
[cause]: Error: Stack causes
at *
at *
... 4 lines matching cause stack trace ...
at * {
[cause]: FoobarError: Individual message
at *
at *
... 4 lines matching cause stack trace ...
at * {
status: 'Feeling good',
extraProperties: 'Yes!',
[cause]: TypeError: Inner error
at *
at *
at *
at *
at *
at *
at *
}
}
}
2 changes: 1 addition & 1 deletion test/message/util_inspect_error.js → test/fixtures/util/util_inspect_error.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

require('../common');
require('../../common');
const util = require('util');

const err = new Error('foo\nbar');
Expand Down
10 changes: 5 additions & 5 deletions test/message/util_inspect_error.out → ...fixtures/util/util_inspect_error.snapshot
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,38 +1,38 @@
{ err:
Error: foo
bar
at *util_inspect_error*
at *
at *
at *
at *
at *
at *
at node:internal*main*run_main_module**,
nested:
{ err:
Error: foo
bar
at *util_inspect_error*
at *
at *
at *
at *
at *
at *
at node:internal*main*run_main_module** } }
{
err: Error: foo
bar
at *util_inspect_error*
at *
at *
at *
at *
at *
at *
at node:internal*main*run_main_module**,
nested: {
err: Error: foo
bar
at *util_inspect_error*
at *
at *
at *
at *
Expand All @@ -43,7 +43,7 @@
}
{ Error: foo
bar
at *util_inspect_error*
at *
at *
at *
at *
Expand Down
Loading