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.

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

util.inspect generates different output in Node minor update #8138

Closed
maxrimue opened this issue Aug 17, 2016 · 7 comments
Closed

util.inspect generates different output in Node minor update #8138

maxrimue opened this issue Aug 17, 2016 · 7 comments
Labels
util Issues and PRs related to the built-in util module.

Comments

@maxrimue
Copy link

  • Version: v6.4.0
  • Platform: Darwin 15.6.0 Darwin Kernel Version 15.6.0: Thu Jun 23 18:25:34 PDT 2016; root:xnu-3248.60.10~1/RELEASE_X86_64 x86_64
  • Subsystem: util/utilities

Since the update from Node.js v6.3 to Node.js v6.4, util.inspect generates a different output for the same object. This broke our tests and I'm not 100% sure if this is intended to happen.

Take this object:

{ desc: 'A command with no name',
  builder: [Function],
  handler: [Function] }

Then, running util.inspect on it:
Node v6.3:

'{ desc: \'A command with no name\',\n  builder: [ [Function: Function] ],\n  handler: [ [Function: Function] ] }'

Node v6.4:

'{ desc: \'A command with no name\',\r\n  builder: [ [Function: Function] ],\r\n  handler: [ [Function: Function] ] }'

Thanks

@yorkie
Copy link
Contributor

yorkie commented Aug 17, 2016

See:

This is the correct behavior :-)

@maxrimue
Copy link
Author

@yorkie Oh I see, thanks!

@addaleax
Copy link
Member

The tests for yargs@4.8.0 through yargs@5.0.0 are failing with Node v6.4.0, and I think the citgm runs should have caught that… @thealphanerd any idea?

@MylesBorins
Copy link
Contributor

@addaleax
Copy link
Member

I’ll try to take a look at that

@bengl
Copy link
Member

bengl commented Aug 17, 2016

Looks like yargs' tests fail on 6.4.0 when process.stdout.isTTY is true.

e.g.

$ npm t
// fail
$ npm t > result.out
$ cat result.out
// success

It makes sense, then, that this wasn't caught by citgm.

@addaleax
Copy link
Member

@bengl Yup, I can confirm that… see yargs/yargs#597 for an attempt to fix this on yargs’ side

addaleax added a commit to addaleax/yargs that referenced this issue Aug 19, 2016
evanlucas added a commit to evanlucas/node that referenced this issue Aug 19, 2016
This reverts commit fce4b98.

This was a breaking change and should have been marked semver-major.
The change that was made altered the output of util.format() and
util.inspect(). With how much those are used in the wild, this type of
change deserves more justification.

Fixes: nodejs#8138
PR-URL: nodejs#8143
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
nexdrew pushed a commit to yargs/yargs that referenced this issue Aug 19, 2016
* test: fix process mocking in case of exception

Fix how the process/io mocking in `test/helpers/utils.js` handles
synchronous uncaught exceptions by adding a `finally` handler
resetting the mocked state.

* test: skip tests which require the window size when run without TTY

Skip two tests that would throw an exception when the test output
is redirected to a non-TTY (e.g. `npm test | cat`).

* test: show more debugging information when integration tests fail

* test: maybe fix tests on AppVeyor machines

* ci: temporarily set Node v6 version to v6.3.1

See nodejs/node#8138.
evanlucas added a commit that referenced this issue Aug 20, 2016
This reverts commit fce4b98.

This was a breaking change and should have been marked semver-major.
The change that was made altered the output of util.format() and
util.inspect(). With how much those are used in the wild, this type of
change deserves more justification.

Fixes: #8138
PR-URL: #8143
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
util Issues and PRs related to the built-in util module.
Projects
None yet
Development

No branches or pull requests

5 participants