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

Jump to bottom

test: fix common.mustNotCall error message #42917

Merged
merged 1 commit into from
Jun 12, 2022
Merged

test: fix common.mustNotCall error message #42917

merged 1 commit into from
Jun 12, 2022

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Apr 29, 2022

When using util.inspect as a callback, the array index (second
argument) is picked up as the showHidden param, and the argument
array (third argument) as the depth param. This fails with a seemingly
unrelated error message when the argument array contains a
Symbol-related property.

Before this change, the following error was output:

node:internal/util/inspect:1014
  if (recurseTimes > ctx.depth && ctx.depth !== null) {
                   ^

TypeError: Cannot convert a Symbol value to a string
    at Array.join (<anonymous>)
    at Array.toString (<anonymous>)
    at formatRaw (node:internal/util/inspect:1014:20)
    at formatValue (node:internal/util/inspect:817:10)
    at inspect (node:internal/util/inspect:347:10)
    at Array.map (<anonymous>)
    at Object.mustNotCall [as get] (…)
    at Array.join (<anonymous>)
    at Array.toString (<anonymous>)
    at formatRaw (node:internal/util/inspect:1014:20)

Node.js v19.0.0-pre

With this change:

node:assert:170
  throw err;
  ^

AssertionError [ERR_ASSERTION]: function should not have been called at …
called with arguments: [Function: func] { customProperty: Symbol() }, 'customProperty', [Function: func] { customProperty: Symbol() }
    at Object.mustNotCall [as get] (…/test/common/index.js:516:12)
    at Object.<anonymous> (…)
    at Module._compile (node:internal/modules/cjs/loader:1105:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Module._load (node:internal/modules/cjs/loader:827:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:77:12)
    at node:internal/main/run_main_module:17:47 {
  generatedMessage: false,
  code: 'ERR_ASSERTION',
  actual: [Getter],
  expected: [Getter],
  operator: 'fail'
}

Node.js v19.0.0-pre

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Apr 29, 2022
@aduh95
Copy link
Contributor Author

aduh95 commented May 10, 2022

/cc @nodejs/testing

ljharb
ljharb reviewed May 10, 2022
View reviewed changes
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

LGTM regardless of comment

test/common/index.js Outdated Show resolved Hide resolved
@aduh95 aduh95 added the review wanted PRs that need reviews. label Jun 2, 2022
@aduh95
Copy link
Contributor Author

aduh95 commented Jun 2, 2022

Ping @nodejs/testing for reviews.

@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jun 2, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 2, 2022
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/44315/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/44332/

@aduh95
test: fix common.mustNotCall error message
5114c46 
When using `util.inspect` as a callback, the array index (second
argument) is picked up as the `showHidden` param, and the argument
array (third argument) as the `depth` param. This fails with a seemingly
unrelated error message when the argument array contains a
`Symbol`-related property.

PR-URL: nodejs#42917
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@aduh95 aduh95 force-pushed the fix-mustNotCall-error branch from ae2516c to 5114c46 Compare June 12, 2022 09:40
@aduh95 aduh95 merged commit 5114c46 into nodejs:master Jun 12, 2022
@aduh95
Copy link
Contributor Author

aduh95 commented Jun 12, 2022

Landed in 5114c46

@aduh95 aduh95 deleted the fix-mustNotCall-error branch June 12, 2022 09:41
italojs pushed a commit to italojs/node that referenced this pull request Jun 12, 2022
@aduh95 @italojs
test: fix common.mustNotCall error message
98e4d7f 
When using `util.inspect` as a callback, the array index (second
argument) is picked up as the `showHidden` param, and the argument
array (third argument) as the `depth` param. This fails with a seemingly
unrelated error message when the argument array contains a
`Symbol`-related property.

PR-URL: nodejs#42917
Reviewed-By: Michaël Zasso <targos@protonmail.com>
danielleadams pushed a commit that referenced this pull request Jun 13, 2022
@aduh95 @danielleadams
test: fix common.mustNotCall error message
cadb481 
When using `util.inspect` as a callback, the array index (second
argument) is picked up as the `showHidden` param, and the argument
array (third argument) as the `depth` param. This fails with a seemingly
unrelated error message when the argument array contains a
`Symbol`-related property.

PR-URL: #42917
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@danielleadams danielleadams mentioned this pull request Jun 13, 2022
Merged
danielleadams pushed a commit that referenced this pull request Jun 13, 2022
@aduh95 @danielleadams
test: fix common.mustNotCall error message
f8d26c6 
When using `util.inspect` as a callback, the array index (second
argument) is picked up as the `showHidden` param, and the argument
array (third argument) as the `depth` param. This fails with a seemingly
unrelated error message when the argument array contains a
`Symbol`-related property.

PR-URL: #42917
Reviewed-By: Michaël Zasso <targos@protonmail.com>
targos pushed a commit that referenced this pull request Jul 12, 2022
@aduh95 @targos
test: fix common.mustNotCall error message
48c9f20 
When using `util.inspect` as a callback, the array index (second
argument) is picked up as the `showHidden` param, and the argument
array (third argument) as the `depth` param. This fails with a seemingly
unrelated error message when the argument array contains a
`Symbol`-related property.

PR-URL: #42917
Reviewed-By: Michaël Zasso <targos@protonmail.com>
targos pushed a commit that referenced this pull request Jul 31, 2022
@aduh95 @targos
test: fix common.mustNotCall error message
cd1c1d5 
When using `util.inspect` as a callback, the array index (second
argument) is picked up as the `showHidden` param, and the argument
array (third argument) as the `depth` param. This fails with a seemingly
unrelated error message when the argument array contains a
`Symbol`-related property.

PR-URL: #42917
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@targos targos mentioned this pull request Aug 3, 2022
Merged
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
@aduh95 @guangwong
test: fix common.mustNotCall error message
fca668f 
When using `util.inspect` as a callback, the array index (second
argument) is picked up as the `showHidden` param, and the argument
array (third argument) as the `depth` param. This fails with a seemingly
unrelated error message when the argument array contains a
`Symbol`-related property.

PR-URL: nodejs/node#42917
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ljharb ljharb ljharb left review comments

@targos targos targos approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run. review wanted PRs that need reviews. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@aduh95 @nodejs-github-bot @ljharb @targos