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

Make tests compatible with Node.js 12 and prerelease versions #197

Merged
merged 3 commits into from Dec 19, 2018

Conversation

silverwind
Copy link
Contributor

@silverwind silverwind commented Dec 16, 2018

Two commits here to make the tests compatible with Node.js 12 as well as prerelease versions of Node.js.

First, the output semantics of console.log for cases of non-string first argument will change which breaks a few output expectations in the tests. I made the tests compatible with any Node.js version by ensuring that the first argument is a string, in which case the output will be the same in all versions.

Additionally, the first commit makes the version check of tools/colorless-console.js compatible with prerelease versions of Node.js.

Related:

@silverwind silverwind changed the title Node 12 Make tests compatible with Node.js 12 and prerelase versions Dec 16, 2018
semver.satisfies does not match a prerelease version against a
non-prerelease one, e.g. 12.0.0-pre does not match >= 10. Use a regex
match to workaround this fact.
In Node.js 12, the formatting of console arguments will change slightly.
Previously, a string other than the first argument was formatted using
single quotes if the first argument was non-string. Now, quotes are
never added regardless of position of a string argument.

To make test compatible in all Node.js versions, I work around by
ensuring the first argument to console.log is a string which prevents
the quotes from being added on older versions of Node.js.

Ref: nodejs/node#23162
@silverwind silverwind changed the title Make tests compatible with Node.js 12 and prerelase versions Make tests compatible with Node.js 12 and prerelease versions Dec 19, 2018
@fabiosantoscode
Copy link
Collaborator

Gee, that's great! I'll release it as soon as possible!

@fabiosantoscode
Copy link
Collaborator

What does console.log output look like then?

@fabiosantoscode fabiosantoscode merged commit 6602668 into terser:master Dec 19, 2018
@silverwind
Copy link
Contributor Author

silverwind commented Dec 19, 2018

What does console.log output look like then?

Before 12:

console.log(1, 'string')
// => 1 'string'
console.log('string', 'string')
// => string string 

After 12:

console.log(1, 'string')
// => 1 string
console.log('string', 'string')
// => string string 

The type of the first argument no longer has significance on the quoting of further string arguments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants