This repository has been archived by the owner on Oct 15, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 339
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
PR-URL: nodejs/node#11488 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Julien Gilli <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Original commit message: [api] Fix DescriptorInterceptor with access check. The DescriptorInterceptor should intercept all Object.getOwnPropertyDescriptor calls. This CL fixes the interceptor's behavior if the iterator state is ACCESS_CHECK. BUG= Review-Url: https://codereview.chromium.org/2707263002 Cr-Commit-Position: refs/heads/master@{#43417} PR-URL: nodejs/node#11712 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: James M Snell <[email protected]>
Fixes: nodejs/node#11717 PR-URL: nodejs/node#11718 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Brian White <[email protected]>
I identified a number of files where it said "the the" in the comments of the source code and in general documentation texts. I replaced these occurences with a single instance of "the". PR-URL: nodejs/node#11711 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
When ArrayBufferAllocator has an undefined zeroFill property, Buffer.allocUnsafe() should zero fill. Refs: nodejs/node@27e84ddd4e1#commitcomment-19182129 PR-URL: nodejs/node#11706 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
Refactor dgram module to use the more efficient module.exports = {} pattern. PR-URL: nodejs/node#11696 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Ron Korving <[email protected]>
Previously the error messages are mostly `[object Object]` after the options get normalized. Use util.inspect to make it more useful. Refactor the listen option test, add precise error message validation and a few more test cases. PR-URL: nodejs/node#11693 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
The font size for tablet sized screens (or half a laptop screen) is overly large, reduce it to a more readable size. Fixes: nodejs/nodejs.org#919 PR-URL: nodejs/node#11695 Fixes: https://github.com/nodejs/nodejs.org#919 Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
The `NODE_ISOLATE_SLOT` macro has been unused since c3cd453. PR-URL: nodejs/node#11692 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
This modifies the script that generates the docs to create a static link from each Stability Index callout bar to the Stability Index explanations in documentation.html. PR-URL: nodejs/node#11664 Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
PR-URL: nodejs/node#11653 Reviewed-By: Claudio Rodriguez <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
PR-URL: nodejs/node#10941 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs/node#10941 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs/node#10941 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs/node#10941 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
Previously, separate incoming Cookie headers would be concatenated with a comma, which can cause confusion in userland code when it comes to parsing the final Cookie header value. This commit concatenates using a semicolon instead. Fixes: nodejs/node#11256 PR-URL: nodejs/node#11259 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
This commit enables automatic concatenation of multiple Cookie header values with a semicolon, except when 2D header arrays are used. Fixes: nodejs/node#11256 PR-URL: nodejs/node#11259 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
'file' should have been 'file:' but since the WHATWG URL spec suggests using an opaque origin (which is what was already being used for file URLs), we'll just keep using that, making this merely a cleanup. PR-URL: nodejs/node#11691 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node#11691 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
* We use these functions that are declared in <unicode/ustring.h> u_strFromUTF8() u_strToUTF8() * At present, <unicode/ustring.h> is indirectly included, but this will likely change in future ICUs. Adding this header has been the right thing to do for many years, so it is backwards compatible. Fixes: nodejs/node#11753 PR-URL: nodejs/node#11754 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Timothy Gu <[email protected]>
Add missing 'i=0' from for-loops from punycode and get-ciphers benchmarks. PR-URL: nodejs/node#11720 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Missing elements in sparse arrays used to be serialized to empty placeholders delimited with commas by util.inspect() and in some cases the result was a syntactically correct representation of a JavaScript array with shorter length than the original one. This commit implements @TimothyGu's suggestion to change the way util.inspect() formats sparse arrays to something similar to how Firefox shows them. Fixes: nodejs/node#11570 PR-URL: nodejs/node#11576 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: nodejs/node#11622 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]>
The getter for process.env already allows symbols to be used, and `in` operator as a read-only operator can do the same. `delete a[b]` operator in ES always returns `true` without doing anything when `b in a === false`. Allow symbols in the deleter accordingly. PR-URL: nodejs/node#11709 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Currently test-tls-legacy-deprecated will fail if node was built using --without-ssl. This commit adds a check to verify is crypto support exists and if not skip this test. PR-URL: nodejs/node#11747 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
0-dns-cert.pem and 0-dns-key.pem were stored in `test/fixtures/key` directory, but the cert file cannot be created with the openssl command via Makefile. Added a script to create it with using `asn1.js` and `asn1.js-rfc5280` and moved them out of key directory and put into `test/fixtures/0-dns`. The domains listed in the cert were also changed into example.com and example.org to show the use for only testing. Fixes: nodejs/node#10228 PR-URL: nodejs/node#11579 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sam Roberts <[email protected]>
PR-URL: nodejs/node#11730 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Is a semver-breaking change Refs: nodejs/node#11189 (comment) PR-URL: nodejs/node#11723 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
A prior io.js era commit inappropriately removed the original copyright statements from the source. This restores those in any files still remaining from that edit. Ref: nodejs/TSC#174 Ref: nodejs/node#10599 PR-URL: nodejs/node#10155 Note: This PR was required, reviewed-by and approved by the Node.js Foundation Legal Committee and the TSC. There is no `Approved-By:` meta data.
Previously all errors resulting from JSON.stringify were treated as a proof for circularity of the object structure. That is not the case if the `toJSON` method of the object throws an error. Explicitly check for the exact error message when determining the object structure's cyclicity. PR-URL: nodejs/node#11708 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
It can happen that the connection and server is closed before the second reponse has been processed by server. In this case, the `res.setTimeout()` callback will never be called causing the test to fail. Fix this by only closing the connection and server when the 2nd has been received. PR-URL: nodejs/node#11790 Fixes: nodejs/node#11768 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
When the release was cut, the changelog heading in CHANGELOG_V7 was accidentally omitted. PR-URL: nodejs/node#11823 Fixes: nodejs/node#11822 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
* Change var to const in an example of server creation. * Add missing semicolons. * Use `console` syntax highlighting in `telnet` and `nc` invocation examples and add shell prompt symbols to be consistent with the rest of the documentation. PR-URL: nodejs/node#11786 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
A module `assertPort` in `lib/internal/net.js` is not used anymore. Refs: nodejs/node#11667 PR-URL: nodejs/node#11812 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
Add a case to test an URL object that has no path at all for `url.resolveObject`. PR-URL: nodejs/node#11811 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
PR-URL: nodejs/node#11626 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]>
Move the core logic from `LineParser` should fail handling into the recoverable error check for the REPL default eval. PR-URL: nodejs/node#6171 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
Previously when the child dies with errors in this test, the parent will just hang and timeout, the errors in the child would be swallowed. This makes it fail so at least there is more information about why this test fails. Also removes the unnecessary child.kill() call. PR-URL: nodejs/node#11684 Ref: nodejs/node#11667 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Notable changes: * module: The [module loading global fallback] (https://nodejs.org/dist/latest-v6.x/docs/api/modules.html#modules_loading_from_the_global_folders) to the Node executable's directory now works correctly on Windows. (Richard Lau) [#9283](nodejs/node#9283) * net: `Socket.prototype.connect` now once again functions without a callback. (Juwan Yoo) [#11762](nodejs/node#11762) * url: `URL.prototype.origin` now properly specified an opaque return of `'null'` for `file://` URLs. (Brian White) [#11691](nodejs/node#11691) PR-URL: nodejs/node#11831
Make sure test matches this specific error message. PR-URL: nodejs/node#11815 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node#11810 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node#11810 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]>
Fix comment about remove listener (not setting) PR-URL: nodejs/node#11804 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Brian White <[email protected]>
Refs: nodejs/node#9399 PR-URL: nodejs/node#11802 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Brian White <[email protected]>
Refs: nodejs/node#9399 PR-URL: nodejs/node#11799 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
Fix a mistyped module name in example REPL sessions found in the description of the 'warning' event: it should be `events` instead of `event`. PR-URL: nodejs/node#11780 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
`test/known_issues/test-repl-function-redefinition-edge-case.js` had been introduced as a part of nodejs/node#7624 but the meat of the test became fixed in 007386e. Despite that, the test continued to fail since it was broken itself: there was a missing colon in the expected output. This commit adds the missing colon and moves the test from `test/known_issues` to `test/parallel`. PR-URL: nodejs/node#11772 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Refs: nodejs/node#9399 PR-URL: nodejs/node#11764 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Even though arrow functions and ES5 anonymous functions are technically the same for util.js, it won't hurt to test both. The same goes for async functions. PR-URL: nodejs/node#11781 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Timothy Gu <[email protected]>
CI - https://ci.nodejs.org/job/chakracore-test-windows/40/ @digitalinfinity - Please review. |
LGTM - I reviewed just the most recent commits and they look fine. Do we have an issue to audit the unit tests that node-chakracore skips? |
Not really because it was implicit before we merge . 😄 But officially, i have opened #188 |
PR-URL: nodejs#186 Reviewed-By: Hitesh Kanwathirtha <[email protected]>
* test-assert-checktag : This is because of our implementation of `global` with proxy. `Object.getPrototypeOf(global)` returns `global` object and this fails setting the value. I was not sure reasoning behind it. For now just skip the test. * test-process-env-symbols : This is chakrashim bug because of which other test cases are failing. I tried fixing it but there were other failures because of that. Basically `HasProperty` of `EnvGetter` returns `undefined` if it is symbol. But in chakrashim, we think that it is a valid value and return true. * test-util-format : This should be removed, once my PR merges in nodejs/node PR-URL: nodejs#186 Reviewed-By: Hitesh Kanwathirtha <[email protected]>
PR-URL: nodejs#186 Reviewed-By: Hitesh Kanwathirtha <[email protected]>
test: more chakrashim test fixes PR-URL: nodejs#186 Reviewed-By: Hitesh Kanwathirtha <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Merge nodejs/node@7acc482 as of 2017-03-14
Interesting commits to review
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
core