(v6.x backport) src: remove GTEST_DONT_DEFINE_ASSERT_EQ in util.h#14836
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This file contains hidden or 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
Export a new common.noop no-operation function for general use. Allow using common.mustCall() without a fn argument to simplify test cases. Replace various non-op functions throughout tests with common.noop PR-URL: nodejs#12027 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Teddy Katz <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
Prefer using `common.mustNotCall()` over `common.mustCall(fn, 0)` PR-URL: nodejs#12027 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Teddy Katz <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
PR-URL: nodejs#12935 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
* add `mustCall` and `mustNotCall` to all callbacks * added `known_issue` for port binding Backport-PR-URL: nodejs#14327 PR-URL: nodejs#13100 Refs: nodejs#13055 Refs: nodejs#12999 Refs: nodejs#13526 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Fix the bug that you start process without network at first, but it connected lately, `dns.resolve` will stay failed with ECONNREFUSED because c-ares servers fallback to 127.0.0.1 at the very beginning. If c-ares servers "127.0.0.1" is detected and its not set by user self, and last query is not OK, recreating `ares_channel` operation will be triggered to reload servers. Fixes: nodejs#1644 Backport-PR-URL: nodejs#14434 PR-URL: nodejs#13076 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
A few of the CLI option values exposed as properties on the process object were missing a comment, fix this. Backport-PR-URL: nodejs#14483 PR-URL: nodejs#12245 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
bootstrap_node.js was directly parsing process.execArgv to see if internals should be exposed, even though the argv was already parsed by node. This is unusual and unnecessary, change it to set the option value from the parser onto the config binding. Backport-PR-URL: nodejs#14483 PR-URL: nodejs#12245 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: nodejs#14495Fixes: nodejs#14430 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Backport-PR-URL: nodejs#14787 PR-URL: nodejs#14438 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Khaidi Chu <[email protected]>
`path.resolve()` and `path.join()` are left alone in this commit due to the lack of clear semantics. Backport-PR-URL: nodejs#14787 PR-URL: nodejs#14440Fixes: nodejs#14405 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
As the length of `path` is known at this point, there is no point in making an exact copy using `slice`. Backport-PR-URL: nodejs#14787 PR-URL: nodejs#14438 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Khaidi Chu <[email protected]>
* we use u_setDataDirectory() in "unicode/putil.h" * at present, this header is indirectly included, but this will change in ICU 59 * no impact on past ICUs. * this is an exact analog to nodejs#11753 PR-URL: nodejs#12078 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]>
Timers should work even if the user has monkey-patched `.call()` and `.apply()` to undesirable values. PR-URL: nodejs#12960 Ref: nodejs#12956 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Original commit message: This commit adds a getter for the private is_verbose_ member. The use case for this comes from Node.js where the ability to avoid calling FatalException if the TryCatch is verbose would be nice to have. BUG= Review-Url: https://codereview.chromium.org/2840803002 Cr-Commit-Position: refs/heads/master@{nodejs#45018} PR-URL: nodejs#12826 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
This commit attempts to address the TODO regarding not calling FatalException if the try_catch is verbose. PR-URL: nodejs#12826 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
`configure` will now call `node_gyp` as a module instead of forking makes it easier to debug PR-URL: nodejs#12653 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
When a no-op message event handler is used in a test, make it clear what is expected by using `common.mustCall()` and `common.mustNotCall()`. PR-URL: nodejs#13125 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Use common.mustCall() in test-fs-makeStatsCallback to confirm that the callback is invoked. PR-URL: nodejs#13132 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Alexey Orlenko <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
In test/pummel/test-stream2-basic.js, check that noop functions are called the expected number of times. PR-URL: nodejs#13146 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: nodejs#13146 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
test-stream2-basic runs in a few seconds. It can be moved to parallel. PR-URL: nodejs#13146 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Use the same dtrace command on ARM as on i386. Patch obtained from FreeBSD PR 218081 [1]. 1. https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=218081 PR-URL: nodejs#12193 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#13215 Refs: nodejs#12586 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
Use `common.mustNotCall()` in test-stream2-objects.js to confirm that noop function is never invoked. PR-URL: nodejs#13249 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Use `common.mustCall()` to make sure noop function is called as expected. PR-URL: nodejs#13259 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
* Use common.mustNotCall() and common.mustCall() as appropriate * Use block scoping * Move assertions out of `exit` handler and into callbacks * Order assert.strictEqual() args per docs * Remove console.log() calls * Move test from `parallel` to `sequential` so `common.PORT` can be used without conflicting with OS-provided ports in other `parallel` tests PR-URL: nodejs#13273 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#13359 Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: David Cai <[email protected]>
* Change common.noop to common.mustNotCall() to verify callback is not invoked. * Add destructuring assignment for clarity. Yeah, clarity. That's why. PR-URL: nodejs#13443 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Use `common.mustNotCall()` to confirm that callback is not invoked when `dns.lookup()` throws. PR-URL: nodejs#13456 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
* Check that noop callback is or isn't invoked as appropriate using common.mustCall() and common.mustNotCall() * Fix typo in array literal PR-URL: nodejs#13480 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
* Add exit listener to child process to check return code. Previously, child process faiilure would not cause the test to fail. * Use common.mustNotCall() to guarantee callback is not invoked. * Insert blank line per test writing guide. PR-URL: nodejs#13904 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
Currently, node_crypto_bio and node_crypto_clienthello are not in the crypto namespace but simply in the node namespace. Not sure if this was intentional or not, but I think it would make sense to move them to be consistent. PR-URL: nodejs#13957 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original-PR-URL: nodejs#13741 Original-Reviewed-By: Colin Ihrig <[email protected]> Original-Reviewed-By: Roman Reiss <[email protected]> Original-Reviewed-By: James M Snell <[email protected]> Original-Reviewed-By: Luigi Pinca <[email protected]> Original-Reviewed-By: Gibson Fahnestock <[email protected]> PR-URL: nodejs#14044 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
* use `common.mustNotCall()` to confirm callback is not called * reorder modules to conform with test-writing guide * match full error message in `assert.throws()` PR-URL: nodejs#13721 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]>
It is possible that `recvmsg()` may return an error on ancillary data reception when receiving a `NODE_HANDLE` message (for example `MSG_CTRUNC`). This would end up, if the handle type was `net.Socket`, on a `message` event with a non null but invalid `sendHandle`. To improve the situation, send a `NODE_HANDLE_NACK` that'll cause the sending process to retransmit the message again. In case the same message is retransmitted 3 times without success, close the handle and print a warning. Fixes: nodejs#9706 PR-URL: nodejs#13235 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
The at-exit addons test uses asserts like the other addons tests, but at-exit is the only one that undefines NDEBUG to make sure that asserts are enabled. This commit removes the undef for consistency. PR-URL: nodejs#13998 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#13852 Refs: nodejs#12586 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]>
PR-URL: nodejs#13716 Reviewed-By: Refael Ackermann <[email protected]> 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]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]>
* use common.mustNotCall() to confirm callback is not invoked * whitespace change per test-writing guide PR-URL: nodejs#13996 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
For simplicity and clarity (if not brevity), add CTC and CTC Emeriti to Collaborators list in README. This will avoid confusion about who is and isn't a Collaborator. PR-URL: nodejs#13284 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
* Make changes to `test-https-set-timeout-server` to resolve inconsistencies with its http counterpart: - Apply the changes analogous to those in nodejsGH-13802 to the https test. - Add a missing `common.mustCall()` wrapper. - Make small stylistic changes (e.g., remove unnecessary line breaks in comments) to make it visually consistent with the http test. * Use arrow functions. PR-URL: nodejs#13935Fixes: nodejs#13588 Refs: nodejs#13802 Refs: nodejs#13625 Refs: nodejs#13822 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: James M Snell <[email protected]>
The sequential/test-regress-GH-4027 test is flaky with an increased system load, failing when the watched file is unlinked before the first state of the watched file is retrieved. After increasing the delay before unlinking and calling setTimeout after watchFile, the flakiness stopped reproducing. PR-URL: nodejs#14010Fixes: nodejs#13800 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
* Do not require if test is skipped. * Do not re-require without need. * Sort requiring by module names. PR-URL: nodejs#14008 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Add clarification to the documentation on util.format() and console.log() regarding how excessive arguments are treated when the first argument is a non-format string compared to when it is not a string at all. PR-URL: nodejs#14027Fixes: nodejs#13908 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Also fix repl and url libs for the rule. PR-URL: nodejs#14032 Refs: http://eslint.org/docs/rules/no-use-before-define Reviewed-By: Daijiro Wachi <[email protected]>
As indicated by the FIXME comment, this macro guard is no longer needed. PR-URL: nodejs#12638 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
nodejs-github-bot added c++ 
bnoordhuis approved these changes Aug 15, 2017
MylesBorinsforce-pushed the v6.x-staging branch 2 times, most recently from cebce8b to 5a93c16Compare
Contributor
MylesBorins commented Aug 16, 2017
ContributorAuthor
danbev commented Aug 16, 2017
MylesBorinsforce-pushed the v6.x-staging branch 2 times, most recently from f9419c2 to 403c465CompareMylesBorinsforce-pushed the v6.x-staging branch 2 times, most recently from aaf4e13 to 31f572cCompareContributor
MylesBorins commented Sep 19, 2017
This already landed 🎉 |
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
20 participants
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.
As indicated by the FIXME comment, this macro guard is no longer needed.
PR-URL: #12638
Reviewed-By: Anna Henningsen [email protected]
Reviewed-By: Colin Ihrig [email protected]
Reviewed-By: James M Snell [email protected]
Reviewed-By: Ben Noordhuis [email protected]
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
src