v4.x-staging to v4.x - PR for CI Testing (do not land) #3566
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When v8 implemented proper one-byte string support Node's internal "binary" encoding implementation was removed in favor of it. The result was that "binary" encoding effectively became "latin-1" encoding. Because of this and because one-byte strings are natively supported by v8 the buffer encoding is not deprecated and will not be removed. Ref: 83261e7 "deps: update v8 to 3.17.13" PR-URL: #3441 Reviewed-By: Ben Noordhuis <ben@strongloop.com>
Upon creating a TLSSocket object, set the default isServer option to false Updated tls docs and added test-tls-socket-default-options PR-URL: #2614 Reviewed-By: Fedor Indutny <fedor@indutny.com>
This comment was a bit misleading, since it was missing the `encoding` argument. PR-URL: #3248 Reviewed-By: Brendan Ashworth <brendan.ashworth@me.com>
This paragraph conveys that detached child processes do not stay running in the background in general. Instead clarify that this refers to the parent process exiting before the detached child process is complete. Reviewed-By: Trevor Norris <trev.norris@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> PR-URL: #3250
Avoids doing a buffer.concat on the internal buffer when that array has only a single thing in it. Reviewed-By: Chris Dickinson <chris@neversaw.us> Reviewed-By: James M Snell <jasnell@gmail.com> PR-URL: #3300
This is a two-part fix: - Fix pending data notification in `OutgoingMessage` to notify server about flushed data too - Fix pause/resume behavior for the consumed socket. `resume` event is emitted on a next tick, and `socket._paused` can already be `true` at this time. Pause the socket again to avoid PAUSED error on parser. Fix: #3332 PR-URL: #3342 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Assert is now locked. Userland alternatives should be used. Assert is for testing Node.js itself. Document potentially surprising use of enumerable properties only in deep equality assertions. Ref: #3124 Ref: #3122 PR-URL: #3330 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Fedor Indutny <fedor@indutny.com> Reviewed-By: Rod Vagg <rod@vagg.org>
Ensure that keylen for pbkdf2 is documented as a length of bytes and not bits. PR-URL: #3334 Reviewed-By: Fedor Indutny <fedor@indutny.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
When the test fails (as it does frequently on FreeBSD unfortunately) provide a non-cryptic error message that also provides a line number. PR-URL: #3335 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: #3353 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Rod Vagg <rod@vagg.org>
Now that we have backticks we no longer need to use util.format to template strings! This commit was inspired by #3324, and it replaces instances of util.format with backtick strings in a number of tests Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: James M Snell <jasnell@gmail.com> PR-URL: #3359
Markdown requires 4-space indent to correctly format code blocks. This fixes the example so it's correctly presented as code. PR-URL: #3372 Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
The assert.fail function signature has the message as the third argument
but, understandably, it is often assumed that it is the first argument
(or at least the first argument if no other arguments are passed).
This corrects the assert.fail() invocations in the Node.js tests.
Before:
assert.fail('message');
// result: AssertionError: 'message' undefined undefined
After:
assert.fail(null, null, 'message');
// result: AssertionError: message
PR-URL: #3378
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
If the symlink portion of the test was being skipped due to a combination of OS support and user privileges, then an assertion would always fail. This fixes that problem, improves assertion error reporting and splits the test to make it clear that it is a test for links and symlinks. Fixes: #3311 PR-URL: #3418 Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Require the test setup to obtain an EMFILE error and not ENFILE as ENFILE means there is a race condition with other processes that may close files before `spawn()` is called by the test. Fixes: #2666 PR-URL: #3430 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
Test should be skipped if ipv6 is unavailable on the running system. Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: James M Snell <jasnell@gmail.com> PR-URL: #3444
Currently there are many instances where assert.fail is directly passed to a callback for error handling. Unfortunately this will swallow the error as it is the third argument of assert.fail that sets the message not the first. This commit adds a new function to test/common.js that simply wraps assert.fail and calls it with the provided message. Tip of the hat to @Trott for pointing me in the direction of this. PR-URL: #3453 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
Fixed an intermittent issue on AIX where the 100ms timeout was reached before the 'connection' event was fired. This resulted in a failure as serverConnection would be undefined and the assert.equal would throw an error. Changed the flow of the test so that the timeout is only set after a connection has been made. Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> PR-URL: #3458
test-child-process-fork-regr-gh-2847 could fail depending on timing and how messages were packed into tcp packets. If all of the requests fit into one packet then the test worked otherwise, otherwise errors could occur. This PR modifies the test to be tolerant while still validating that some of the connection can be made succesfully Reviewed-By: James M Snell <jasnell@gmail.com> PR-URL: #3459
This test is already partially disabled for several platforms with the comment that the required info is not provided at the C++ level. I'm adding AIX as and PPC BE linux as they currently fall into the same category. We are working to see if we can change that in v8 but it will be non-trivial if is possible at all so I don't want to leave the CI with failing tests until that point. PR-URL: #3491 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
malloc(0) and realloc(ptr, 0) have implementation-defined behavior in that the standard allows them to either return a unique pointer or a nullptr for zero-sized allocation requests. Normalize by always using a nullptr. Fixes: #3496 PR-URL: #3499 Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Trevor Norris <trev.norris@gmail.com>
The existing test never ran because typeof Symbol === 'function' and not 'symbol'. We have Symbols now so remove the check and just run the test. PR-URL: #3327 Reviewed-By: James M Snell <jasnell@gmail.com>
Previously the wrong end of the history was limited on load. PR-URL: #2356 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed By: Evan Lucas <evanlucas@me.com>
PR-URL: #2947 Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Add SIGTRAP and the corresponding exit code to the list of signals/exit codes that are expected when running tests that throw an uncaught error and have --abort-on-uncaught-exception enabled. Also refactor a bit related comments so that they better reflect what's actually happening. Fixes #3239. PR: #3354 PR-URL: #3354 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
The test sometimes fail on an assertion but no useful error message was generated for debugging. Modify the test to generate useful debugging message. PR-URL: #3501 Reviewed-By: Trevor Norris <trev.norris@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
inherits is used in lib and tests but its functionality itself is not tested yet. PR-URL: #3507 Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Original commit message:
[heap] fix crash during the scavenge of ArrayBuffer
Scavenger should not attempt to visit ArrayBuffer's storage, it is a
user-supplied pointer that may have any alignment. Visiting it, may
result in a crash.
BUG=
R=jochen
Review URL: https://codereview.chromium.org/1406133003
Cr-Commit-Position: refs/heads/master@{#31611}
PR-URL: #3549
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
The debug process running "node debug a.js" will be stuck when the script ends. This is because the debug handler has been unrefed. We shouldn't unref the debug handler to avoid this problem. PR-URL: #2778 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
common.Error() is just util.isError() which is deprecated. Use assert.throws() instead. PR-URL: #3084 Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Fixes a persistently troublesome failing test by splitting it out into multiple parallel tests. Reviewed By: Evan Lucas <evanlucas@me.com> Reviewed By: Trevor Norris <trev.norris@gmail.com> Reviewed By: James M Snell <jasnell@gmail.com> PR-URL: #3287
The Pi 1's in CI don't always fail on the buffer.toString() tests. But they time out sometimes, so let's split the tests up so they don't. PR-URL: #3323 Reviewed By: Evan Lucas <evanlucas@me.com> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed By: Trevor Norris <trev.norris@gmail.com>
common does not need util properties anymore. Remove them. PR-URL: #3304 Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Brendan Ashworth <brendan.ashworth@me.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
util is loaded just for one use of util.format(). Replace it with a string template. While we're at it, delete nearby lengthy comment justifying use of fs.readFileSync(). PR-URL: #3324 Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed By: Evan Lucas <evanlucas@me.com>
f2a45ca contained a test for a regression that had been introduced by the original change that 77a10ed ported. While 77a10ed did not contain that regression, the test that f2a45ca contained should still be in the code base to prevent any regression from happening in the future. Original message for the commit that contained the test: domains: fix stack clearing after error handled caeb677 introduced a regression where the domains stack would not be cleared after an error had been handled by the top-level domain. This change clears the domains stack regardless of the position of the active domain in the stack. PR: #9364 PR-URL: nodejs/node-v0.x-archive#9364 Reviewed-By: Trevor Norris <trev.norris@gmail.com> Reviewed-By: Julien Gilli <julien.gilli@joyent.com> PR: #3356 PR-URL: #3356 Reviewed-By: Ben Noordhuis <ben@strongloop.com>
Instead of creating new timer - reuse the timer from the freelist. This won't make the freelist timer active for the duration of `uv_close()`, and will let the event-loop exit properly. Fix: #1264 PR-URL: #3407 Reviewed-By: Trevor Norris <trev.norris@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
As it is, the comments are not handled properly in REPL. So, if the comments have `'` or `"`, then they are treated as incomplete string literals and the error is thrown in REPL. This patch refactors the existing logic and groups everything in a class. Fixes: #3421 PR-URL: #3515 Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Before this commit, sending a SIGUSR1 at program exit could trigger a hard to reproduce race condition where `v8::Debug::DebugBreak(isolate)` got called when the isolate was in the process of being torn down. A similar race condition is in theory possible when sending signals to two threads simultaneously but I haven't been able to reproduce that myself (and I tried, oh how I tried.) This commit fixes the race condition by turning `node_isolate` into a `std::atomic` and using it as an ad hoc synchronization primitive in places where that is necessary. A bare minimum std::atomic polyfill is added for OS X because Apple wouldn't be Apple if things just worked out of the box. PR-URL: #3528 Reviewed-By: Fedor Indutny <fedor@indutny.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This is a reland of #3165. The patch abates the truncation of script filenames in the perf-event output produced by V8. V8 commits: Original: v8/v8@03ef3cd Reland: v8/v8@010897c Original commit message: improve perf_basic_prof filename reporting The buffer used for appending filenames to the string printed to the perf_basic_prof log was unnecessarily too small. Bump it up to be at least kUtf8BufferSize. Truncation of filenames makes it really hard to work with profiles gathered on Node.js. Because of the way Node.js works, you can have node module dependencies in deeply nested directories. The last thing you want when investigating a performance problem is to have script names be truncated. This patch is a stop-gap. Ideally, I want no truncation of the filename at all and use a dynamically growing buffer. That would be a larger change, and I wanted to have a quick fix available that can be back-ported to Node.js LTS release. R=yangguo@chromium.org,yurys@chromium.org BUG= Review URL: https://codereview.chromium.org/1388543002 PR-URL: #3520 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
jasnell
added
the
meta
If the URL passed to `http.request` is not properly parsable by `url.parse`, we fall back to use `localhost` and port 80. This creates confusing error messages like in this question http://stackoverflow.com/q/32675907/1903116. PR-URL: #2966 Reviewed-By: James M Snell <jasnell@gmail.com>
Check that `node --debug-brk -e 0` immediately quits. PR-URL: #3585 Reviewed-By: Evan Lucas <evanlucas@me.com> 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Preparing for the v4.2.2. Opening this PR to kick off a full CI test run against the currently staged commits. Please do not land this PR.