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

[8.x Backport] backport multiple commits to 8.x #22380

Closed
wants to merge 33 commits into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Aug 17, 2018

Backport multiple commits to 8.x

/cc @MylesBorins @nodejs/lts

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

BridgeAR and others added 15 commits August 17, 2018 11:36
PR-URL: nodejs#13857
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
PR-URL: nodejs#15606
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Bartosz Sosnowski <[email protected]>
`readable.push()` supports `undefined` in non-object mode, but it was
not previously documented.

PR-URL: nodejs#18283
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Fixes: nodejs#18639

PR-URL: nodejs#18716
Refs: nodejs#18639
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
PR-URL: nodejs#18928
Reviewed-By: Claudio Rodriguez <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
There are two minor issues in the AsyncHook constructor, if the object
passed in has an after and/or destroy property that are not functions
the errors thrown will still be:
TypeError [ERR_ASYNC_CALLBACK]: before must be a function

This commit updates the code and adds a unit test.

PR-URL: nodejs#19000
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
When socket is closed on a response for a request that is being piped to
a stream there is a condition where aborted event will be fired to http
client when socket is closing and the incomingMessage stream is still
set to readable.

We need a check for request being complete and to only raise the
'aborted' event on the http client if we have not yet completed reading
the response from the server.

Fixes: nodejs#18756

PR-URL: nodejs#18999
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Tests in progress to reproduce issue consistently.

Fixes: nodejs#18756

PR-URL: nodejs#18999
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#18999
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Directory symlinks in Windows require the 'dir' flag to be passed to
create the symlink correctly.

PR-URL: nodejs#19049
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Khaidi Chu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: nodejs#19052
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: nodejs#18080
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
libuv and zlib symbols are also purposefully re-exported by Node.js for
use in Addons.

Refs: nodejs#17444

PR-URL: nodejs#19013
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: nodejs#18888
Fixes: nodejs#18887
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Added a simple example showing how to rename
a file.

Refs: https://github.com/nodejs/node/issues11135

PR-URL: nodejs#18812
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@nodejs-github-bot nodejs-github-bot added async_hooks Issues and PRs related to the async hooks subsystem. http Issues or PRs related to the http subsystem. process Issues and PRs related to the process subsystem. v8.x labels Aug 17, 2018
Trott and others added 10 commits August 17, 2018 15:56
There are some uses of "in general" that are unnecessary. (We don't need
to be tentative about the fact that tests should pass, for example.)

PR-URL: nodejs#19123
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Remove request that user provide a list of subsystems in the pull
request template.

* We already ask them to put the subsystems in the first line of the
  commit message.
* The subsystem is usually easy to determine.
* We have a bot that applies subsystem labels on new PRs and it seems to
  do a rather good job. Tools over rules. Let the bot do it.
* The fewer unnecessary things we ask for in the template, the lower the
  barrier to entry.
* Anecdotal, but I have never found it useful to have the person
  submitting the PR list out subsystems in the pull request post.

PR-URL: nodejs#19125
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: Kyle Farnung <[email protected]>
Currently test-http2-client-write-empty-string.js will throw "Error
[ERR_NO_CRYPTO]: Node.js is not compiled with OpenSSL crypto support"
when configured --without-ssl.

This commit moves the require of http2 to after the crypto check to
avoid this error.

PR-URL: nodejs#19111
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
iculslocs uses stdio, but didn't include the header.

PR-URL: nodejs#19150
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Fixes: nodejs#17892
Fixes: nodejs#17893
Fixes: nodejs#18992

PR-URL: nodejs#18993
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
PR-URL: nodejs#19162
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#19124
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
On Windows with msys installation, `nm` is available but
it is not able to grab symbols from the Windows build.
Skipping the test if nm outputs anything to stderr fixes that.

PR-URL: nodejs#19107
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Add a simple example showing how to use the inspector API to access
the CPU profiler.

PR-URL: nodejs#19172
Reviewed-By: Eugene Ostroukhov <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Khaidi Chu <[email protected]>
Remove suggestion to avoid `readable` event and `readabe.read()` method.
No explanation was provided for this suggestion.

PR-URL: nodejs#19193
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Chen Gang <[email protected]>
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
Backport-PR-URL: #22380
PR-URL: #18928
Reviewed-By: Claudio Rodriguez <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
There are two minor issues in the AsyncHook constructor, if the object
passed in has an after and/or destroy property that are not functions
the errors thrown will still be:
TypeError [ERR_ASYNC_CALLBACK]: before must be a function

This commit updates the code and adds a unit test.

Backport-PR-URL: #22380
PR-URL: #19000
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
When socket is closed on a response for a request that is being piped to
a stream there is a condition where aborted event will be fired to http
client when socket is closing and the incomingMessage stream is still
set to readable.

We need a check for request being complete and to only raise the
'aborted' event on the http client if we have not yet completed reading
the response from the server.

Fixes: #18756

Backport-PR-URL: #22380
PR-URL: #18999
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
Tests in progress to reproduce issue consistently.

Fixes: #18756

Backport-PR-URL: #22380
PR-URL: #18999
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
Backport-PR-URL: #22380
PR-URL: #18999
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
Directory symlinks in Windows require the 'dir' flag to be passed to
create the symlink correctly.

Backport-PR-URL: #22380
PR-URL: #19049
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Khaidi Chu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
Backport-PR-URL: #22380
PR-URL: #19052
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
Backport-PR-URL: #22380
PR-URL: #18080
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
libuv and zlib symbols are also purposefully re-exported by Node.js for
use in Addons.

Refs: #17444

Backport-PR-URL: #22380
PR-URL: #19013
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
Backport-PR-URL: #22380
PR-URL: #18888
Fixes: #18887
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
Added a simple example showing how to rename
a file.

Refs: https://github.com/nodejs/node/issues11135

Backport-PR-URL: #22380
PR-URL: #18812
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
There are some uses of "in general" that are unnecessary. (We don't need
to be tentative about the fact that tests should pass, for example.)

Backport-PR-URL: #22380
PR-URL: #19123
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins MylesBorins closed this Sep 6, 2018
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
Remove request that user provide a list of subsystems in the pull
request template.

* We already ask them to put the subsystems in the first line of the
  commit message.
* The subsystem is usually easy to determine.
* We have a bot that applies subsystem labels on new PRs and it seems to
  do a rather good job. Tools over rules. Let the bot do it.
* The fewer unnecessary things we ask for in the template, the lower the
  barrier to entry.
* Anecdotal, but I have never found it useful to have the person
  submitting the PR list out subsystems in the pull request post.

Backport-PR-URL: #22380
PR-URL: #19125
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: Kyle Farnung <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
Currently test-http2-client-write-empty-string.js will throw "Error
[ERR_NO_CRYPTO]: Node.js is not compiled with OpenSSL crypto support"
when configured --without-ssl.

This commit moves the require of http2 to after the crypto check to
avoid this error.

Backport-PR-URL: #22380
PR-URL: #19111
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
iculslocs uses stdio, but didn't include the header.

Backport-PR-URL: #22380
PR-URL: #19150
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
Fixes: #17892
Fixes: #17893
Fixes: #18992

Backport-PR-URL: #22380
PR-URL: #18993
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
Backport-PR-URL: #22380
PR-URL: #19162
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
Backport-PR-URL: #22380
PR-URL: #19124
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
On Windows with msys installation, `nm` is available but
it is not able to grab symbols from the Windows build.
Skipping the test if nm outputs anything to stderr fixes that.

Backport-PR-URL: #22380
PR-URL: #19107
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
Add a simple example showing how to use the inspector API to access
the CPU profiler.

Backport-PR-URL: #22380
PR-URL: #19172
Reviewed-By: Eugene Ostroukhov <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Khaidi Chu <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
Remove suggestion to avoid `readable` event and `readabe.read()` method.
No explanation was provided for this suggestion.

Backport-PR-URL: #22380
PR-URL: #19193
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Chen Gang <[email protected]>
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
Backport-PR-URL: #22380
PR-URL: #19238
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
Use arrow functions.

Backport-PR-URL: #22380
PR-URL: #19130
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
test-performance can fail due to resource constraints. Move it from
parallel to sequential so it does not compete with other tests for
resources.

Fixes: #19197

Backport-PR-URL: #22380
PR-URL: #19228
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
In test-tls-wrap-event-emitter, the text of a TypeError is checked as
part of the test. However, this text is generated by the JavaScript
engine (V8) and not Node.js. Thus, we should not check the text as JS
engine error messages can change without it being considered a breaking
change for Node.js.

A side effect is that node-chakracore will no longer need to patch this
test to pass.

Backport-PR-URL: #22380
PR-URL: #19215
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
Using `assert.doesNotThrow()` has no benefit over adding a comment
next to some code that should not throw. Therefore it is from now
on discouraged to use it.

Backport-PR-URL: #22380
PR-URL: #18699
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
Below syntax errors are handled without force .break/clear
  - Unexpected Token (prefix errors)
  - missing ) after argument list

In the multiline expression, recoverable errors are truly
recoverable, otherwise syntax error will be thrown.

Backport-PR-URL: #22380
PR-URL: #18915
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
This commit improves asserion messages in parallel/test-crypto-hash.js.
Instead of just simple string literal, messages are changed to also
include values used in assertion, which should improve debugging
in case of errors.

Backport-PR-URL: #22380
PR-URL: #18984
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
When using shared lib build, the binary path in the stack frames points
to shared lib. Change the checking criteria in the test case to match
that.

Refs: #18535

Signed-off-by: Yihong Wang <[email protected]>

Backport-PR-URL: #22380
PR-URL: #19213
Refs: #18535
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
async_hooks Issues and PRs related to the async hooks subsystem. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. http Issues or PRs related to the http subsystem. process Issues and PRs related to the process subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.