v7.x-staging update proposal #10886
Closed
v7.x-staging update proposal #10886
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
We were transporting the heap statistics as uint32 values to JS land but those wrap around for values > 4 GB. Use 64 bits floats instead, those should last us a while. Fixes: nodejs#10185 PR-URL: nodejs#10186 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com>
1. equal => strictEqual. 2. let => const for the variable that is not reassigned. 3. fix spaces. 4. stringify erroneous raw buffer outputs. 5. fix a typo. PR-URL: nodejs#10102 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
* use common.mustCall() where appropriate * Buffer.allocUnsafe() -> Buffer.alloc() * do crypto check before loading any additional modules * specify 1ms duration for `setTimeout()` PR-URL: nodejs#10225 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
The original test lauches 10 child processes at once and bypass `test.py`'s process regulation. This PR reduces the unmanaged parallelism and is a temporary workaround for nodejs#9979 (not a real fix). PR-URL: nodejs#10329 Reviewed-By: Anna Henningsen <anna@addaleax.net>
* used let and const instead of var * used assert.strictEqual instead assert.equal PR-URL: nodejs#10357 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: Julian Duque <julianduquej@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
The destroy_ids_idle_handle_ needs to be closed on environment destruction. Not closing the handle leaves a dangling pointer in the used uv loop. This leads to undefined behavior when the uv loop is used after the environment has been destroyed. PR-URL: nodejs#10385 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#10392 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#10419 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Refs: nodejs#9901 (comment)
On Windows, creating a symlink requires admin privileges. There were two tests which created symlinks which were failing when run as non-admin. test-fs-symlink.js already had a check for privileges on Windows but it had a couple issues: 1. It assumed that whoami was the one that came with windows. However, whoami also ships with Win32 Unix utility ports like the distribution with git, which can cause this to get check tripped up. 2. On failure, the check would just return from the callback instead of exiting 3. whoami was executed asynchronously so the test would run regardless of privilege state. test-fs-options-immutable had no check. As part of this change, I refactored the privilege checking to a function in common, and changed both above tests to use the refactored function. Also documented this function in test\README.md PR-URL: nodejs#10477 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: João Reis <reis@janeasystems.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
PR-URL: nodejs#10543 Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#10550 Reviewed-By: Rich Trott <rtrott@gmail.com>
Use common.mustCall() where appropriate, var to const/let, assert.equal() -> assert.strictEqual(), explicit time provided to setTimeout() PR-URL: nodejs#10551 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Refactor and simplify parallel/test-timer-close.js. Add comment to describe the test case. PR-URL: nodejs#10517 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
* use const and let instead of var * use assert.strictEqual instead of assert.equal * use arrow functions PR-URL: nodejs#10556 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Italo A. Casas <me@italoacasas.com> Reviewed-By: Brian White <mscdex@mscdex.net>
Change var to const/let. Simplify test-timers-uncaught-exception. PR-URL: nodejs#10524 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
PR-URL: nodejs#10510 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
See: https://url.spec.whatwg.org/#dom-url-origin Also moves the tests for origins to the parsing tests since now URL#origin matches the test cases by default. PR-URL: nodejs#10552 Reviewed-By: James M Snell <jasnell@gmail.com>
new year new alias PR-URL: nodejs#10586 Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
Remove the numbers from the comments to make it clear that assert does not follow the [CJS spec](http://wiki.commonjs.org/wiki/Unit_Testing/1.0). Additionally, clean up the existing comments for consistent formatting/language and ease of reading. PR-URL: nodejs#10579 Fixes: nodejs#9063 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
This updates util.inspect() to avoid accessing out-of-range indices of the `arguments` object, which is known to cause optimization bailout. Based on an average of 10 runs of the benchmark in `benchmark/util/inspect.js`, this change improves the performance of `util.inspect` by about 10%. Relates to nodejs#10323 PR-URL: nodejs#10569 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Jackson Tian <shyvo1987@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
punycode/ICU is not specific to any particular module, so move it to a more generic location. PR-URL: nodejs#10446 Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#10446 Reviewed-By: James M Snell <jasnell@gmail.com>
Some benchmarks' results are small values, so keeping decimals when running them manually (not comparing) can be helpful. PR-URL: nodejs#10559 Reviewed-By: James M Snell <jasnell@gmail.com>
array.shift() seems to be faster than arrayClone() when the item to remove is at the front (at least with V8 5.4). PR-URL: nodejs#10572 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
These changes result in ~50% improvement in the included benchmark. PR-URL: nodejs#10580 Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#10577 Reviewed-By: Italo A. Casas <me@italoacasas.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
* use const instead of var * use common.mustCall to control functions execution * use assert.strictEqual instead of assert.equal * use arrow functions * remove console.error PR-URL: nodejs#10521 Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
`process.title` would work properly only in FreeBSD, OSX, and Linux as per test/parallel/test-setproctitle.js. This patch makes sure that the test expects an empty string in other platforms. This patch helps fix the SmartOS failures in https://ci.nodejs.org/job/node-test-commit/6962/ for nodejs#10456 PR-URL: nodejs#10597 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com>
We have had nodejs#9728 open for a while but the frequency of the failures seems to be such that we should mark it as flaky while we continue to investigate. PR-URL: nodejs#10618 Reviewed-by: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This commit makes sure EventEmitter.emit() doesn't get deoptimized by V8. The deopt happens when accessing out of bound indexes of the `arguments` object. This issue has been raised here: nodejs#10323 and this specific case might become a more serious performance issue in upcoming V8 releases. PR-URL: nodejs#10568 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
* validate the errors for all assert.throws * use arrow functions PR-URL: nodejs#10779 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
* assert unused vars in test-cli-eval.js * assert in more idiomatic way test-cli-eval * rename status to err in test-cli-eval.js PR-URL: nodejs#10759 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
* Clarify that memory is always shared and never copied. * Fix wording that sounded like ArrayBuffer has a buffer property. PR-URL: nodejs#10778 Ref: nodejs#10770 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Confirm that `getCiphers()` contains no duplicates. PR-URL: nodejs#10784 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
* validate the errors for assert.throws * use arrow functions PR-URL: nodejs#10714 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
If you alter the array returned by `tls.getCiphers()`,
`crypto.getCiphers()`, `crypto.getHashes()`, or `crypto.getCurves()`, it
will alter subsequent return values from those functions.
```js
'use strict';
const crypto = require('crypto');
var hashes = crypto.getHashes();
hashes.splice(0, hashes.length);
hashes.push('some-arbitrary-value');
console.log(crypto.getHashes()); // "['some-arbitrary-value']"
```
This is surprising. Change functions to return copy of array instead.
PR-URL: nodejs#10795
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
* use common.mustCall to validate functions executions * use common.fail to control error * remove unnecessary variables * remove unnecessary assertions * remove console.log and console.error * use arrow functions PR-URL: nodejs#10798 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
An application using node built as a shared library may legitimately implement its own signal handling routines. Current behaviour is to squash all signal handlers on node startup. This change will stop that behaviour when node is built as a shared library. PR-URL: nodejs#10539 Fixes: nodejs#10520 Refs: nodejs#615 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
* use common.mustCall to validate functions executions * use common.fail to check test fail * improve error validations * remove unnecessary assertions * use arrow functions PR-URL: nodejs#10813 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs#10783 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
The `exports` parameter is unnecessary. PR-URL: nodejs#10834 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sam Roberts <sam@strongloop.com>
PR-URL: nodejs#10826 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#10826 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Currently, the sources list contains sources and headers which are separated by a comment. I noticed two .cc files after the headers comment and this commit moves those files the start of the list where the rest of source files are. PR-URL: nodejs#10850 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Italo A. Casas <me@italoacasas.com>
This contained a duplicate link to the PR for a notable change, presumably because that PR was composed of 2 separate commits. PR-URL: nodejs#10827 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#10827 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Add links to the engine classes for the zlib single-call convenience methods. PR-URL: nodejs#10829 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Fixes: nodejs#8684 PR-URL: nodejs#8914 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
The third parameter `err` is not used anywhere. PR-URL: nodejs#10862 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs#10844 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Italo A. Casas <me@italoacasas.com>
PR-URL: nodejs#10832 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
According to nodejs#10803 getHeader need not be called only before it is flushed implicitly. PR-URL: nodejs#10817 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Introduce benchmarks for vm.runInContext() and vm.runInThisContext(). PR-URL: nodejs#10816 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Brian White <mscdex@mscdex.net>
Optimize for common cases in vm.runInContext() and vm.runInThisContext(). PR-URL: nodejs#10816 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Brian White <mscdex@mscdex.net>
This commit clarifies variables in the Filesystem docs. Prior, the documentation for fs.write() had an ambiguous remark on the parameters of offset and length. Therefore, this commit makes explicit that the length parameter in fs.write() is used to denote the number of bytes, which is a clearer reference for its usage. PR-URL: nodejs#9792 Ref: nodejs#7868 Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
PR-URL: nodejs#10883 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Italo A. Casas <me@italoacasas.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
italoacasas
force-pushed
the
v7.x-staging1
branch
from
b5c019d
to
d6af213
Compare
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.
Proposal to update
v7.x-stagingUpdate
This branch still having errors, most of then related with the changes on
readfile tests. Working on that...List of commits that aren't landing