Use non-capturing grouping or remove capturing completely when:
* capturing is useless per se, e.g. in test() check;
* captured groups are not used afterwards at all;
* some of the later captured groups are not used afterwards.
PR-URL: https://github.com/nodejs/node/pull/13718
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Use non-capturing grouping or remove capturing completely when:
* capturing is useless per se, e.g. in test() check;
* captured groups are not used afterwards at all;
* some of the later captured groups are not used afterwards.
PR-URL: https://github.com/nodejs/node/pull/13718
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Make test-http(s)-set-timeout-server tests more similar and resolve the
following issues:
* `test-http-set-timeout-server.js` had a `secReceived` check in
`serverResponseTimeoutWithPipeline()` that was added to prevent
flakiness, but this did not exist in the https counterpart.
* `test-https-set-timeout-server.js` utilized `common.mustCall()`,
`common.mustNotCall()`, etc., while the http counterpart utilized the
old method of checking flags on exit of the process.
Refs: https://github.com/nodejs/node/issues/13588
PR-URL: https://github.com/nodejs/node/pull/13625
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/13714
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* Do not repeat RegExp creation in cycle.
* Use sufficient string instead of RegExp in split().
PR-URL: https://github.com/nodejs/node/pull/13709
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* format test per project guide
* use listener that emits clear message
* use common.mustNotCall() to confirm different listener is not invoked
* add comment describing the test
PR-URL: https://github.com/nodejs/node/pull/13689
Reviewed-By: David Cai <davidcai1993@yahoo.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
We are currenly builing Node with --shared-zlib which happens to be
version 1.2.8. The test for zlib.createDeflateRaw is expected to fail
but does not when using version 1.2.8.
As far as I can tell the fix referred to in the comments was
introduced in version 1.2.9:
- Reject a window size of 256 bytes if not using the zlib wrapper
This commit suggests adding a check for the version and skipping this
assert if the version is less than 1.2.9.
Refs: http://zlib.net/ChangeLog.txt
PR-URL: https://github.com/nodejs/node/pull/13697
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Currently when node is configured --without-intl the tests in this
commit fail. This commit adds checks for internationalization for these
tests
PR-URL: https://github.com/nodejs/node/pull/13699
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Commit d7e4ae1eb0
("test: add common.hasIntl") added common.hasIntl but I was not able to
find it in the README.md so this commit adds it.
PR-URL: https://github.com/nodejs/node/pull/13699
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* /m is needless if ^ and $ are not used
* /g is needless in split()
* /g is needless in test() with one-time RegExp/String
PR-URL: https://github.com/nodejs/node/pull/13690
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Update ESLint and configuration to version 4.0.0.
PR-URL: https://github.com/nodejs/node/pull/13645
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Make the "can we lint?" check in Makefile and vcbuild.bat depend on
bin/eslint.js rather than lib/eslint.js. In ESLint 4.0.0, lib/eslint.js
is not present. The lint rules call bin/eslint.js so check for that
instead.
PR-URL: https://github.com/nodejs/node/pull/13645
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Currently, there are a few test-tls-wrap-econnreset test that fail when
Node is configured --without-ssl:
Error: Node.js is not compiled with openssl crypto support
This commit adds crypto checks and skips these tests if no crypto
support unavailable.
PR-URL: https://github.com/nodejs/node/pull/13691
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fake the socket handle to ensure an immediate error is returned
uniformly across all platforms.
PR-URL: https://github.com/nodejs/node/pull/13693
Reviewed-By: James M Snell <jasnell@gmail.com>
Add `buffer.constants`, containing length limits for `Buffer` and
`string` instances.
This could be useful for programmers to tell whether a value can
be turned into a string or not.
Ref: https://github.com/nodejs/node/issues/13465
PR-URL: https://github.com/nodejs/node/pull/13467
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Until now, the async_hooks PromiseHook did not register the Promise’s
async id and trigger id on the id stack, so inside the `.then()` handler
those ids would be invalid.
To fix this, add push and pop calls to its `before` and `after` parts,
respectively. Some care needs to be taken for the cases that the
Promise hook is being disabled or enabled during the execution
of a Promise handler; in the former case, actually removing the hook
is delayed by adding another task to the microtask queue, in the latter
case popping the id off the async id stack is skipped if the ids don’t
match.
Fixes: https://github.com/nodejs/node/issues/13583
PR-URL: https://github.com/nodejs/node/pull/13585
Reviewed-By: Trevor Norris <trevnorris@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/13673
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* alphabetize modules
* replace callback invocation counts with common.mustCall()
* add block-scoping
* remove commented-out code that uses an identifier not in the test
* move exit handlers to relevant blocks to keep tests cohesive
* common.noop -> common.mustNotCall()
* check results from `data` event
PR-URL: https://github.com/nodejs/node/pull/13643
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
In util.format, if a percent sign without a known type is encountered,
just print it instead of silently ignoring it and the next character.
PR-URL: https://github.com/nodejs/node/pull/13674
Fixes: https://github.com/nodejs/node/issues/13665
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/13685
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* Add common.mustCall().
* Add check for error existence, not only for error details.
* Use test(), not match() in a boolean context.
* Properly reverse meanings of assert messages.
PR-URL: https://github.com/nodejs/node/pull/13680
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
There are a number of void casts of clear_error_on_return which is a
usage of the RAII idiom. The ClearErrorOnReturn struct only has a
destructor and no constructor which I believe was an issue in GCC
prior to version 4.8.0, which lead to a unused variable warning.
I'm wondering if these cast could be removed since GCC 4.8.5 or newer
is required now. An alternative solution would be to add an empty
constructor which should work allowing the compiler to detect that a
variable is used only for its side-effects.
Not sure if this was the sole reason for having these casts but wanted
to bring it up just in case.
Refs: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=10416
PR-URL: https://github.com/nodejs/node/pull/13669
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* Adding --inspect-port with debug port, instead of parsing `execArgv`
* Export CLI debug options to `process.binding('config').debugOptions`
(currently used only in tests)
PR-URL: https://github.com/nodejs/node/pull/13619
Refs: https://github.com/nodejs/node/pull/9659
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
* use common.mustNotCall() to confirm callback is not invoked
* blank line after common module per test writing guide
PR-URL: https://github.com/nodejs/node/pull/13661
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
In non-buffer tests, change usage of the Buffer constructor to one of
the recommended alternatives.
PR-URL: https://github.com/nodejs/node/pull/13649
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Brian White <mscdex@mscdex.net>
> Generally it will correspond the name of the resource's constructor.
should read "Generally, it will correspond to the name..."
PR-URL: https://github.com/nodejs/node/pull/13666
Fixes: https://github.com/nodejs/node/issues/13663
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
The 4.8.x releases don't fully support C++11. Update the prerequisites
for node.js 8 so that we won't have to work around compiler bugs in the
future.
To be decided if we should also update the minimum clang version.
I believe clang 3.4.2 properly supports C++11 but am not 100% sure.
PR-URL: https://github.com/nodejs/node/pull/13466
Ref: https://github.com/nodejs/node/pull/11840
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com>
* block scope `paused`
* change name of block-scoped `file3` etc. to `file`
* alphabetize modules
* confirm contents provided in `data` callback
* confirm `data` callbacks will not fire on tests for errors
PR-URL: https://github.com/nodejs/node/pull/13618
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* Use existing errors where suitable
* Assign code to a REPL specific error
* Include documentation for the new error code
PR-URL: https://github.com/nodejs/node/pull/11347
Ref: https://github.com/nodejs/node/issues/11273
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
PR-URL: https://github.com/nodejs/node/pull/13553
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Also, add tests to ensure they will always return this, and to confirm
they return this when these doc changes are back-ported to earlier
release lines.
PR-URL: https://github.com/nodejs/node/pull/13531
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Add more information to the "ECONNRESET" errors generated when the
socket hang ups before establishing the secure connection.
These kind of errors are really hard to troubleshoot without this info.
PR-URL: https://github.com/nodejs/node/pull/7476
Reviewed-By: Trevor Norris <trevnorris@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Yazhong Liu <yorkiefixer@gmail.com>
`opts` in `createServer` will be immutable that won't change origional
opts value. What's more, it's optional which can make `requestListener`
be the first argument.
PR-URL: https://github.com/nodejs/node/pull/13599
Fixes: https://github.com/nodejs/node/issues/13584
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Vse Mozhet Byt
jklepatch
Daijiro Wachi
DuanPengfei
Rich Trott
Daniel Bevenius
Roman Reiss
Brian White
Anna Henningsen
Eugene Ostroukhov
Gibson Fahnestock
Tamás Hódi
Nikolai Vavilov
Michaël Zasso
cjihrig
Refael Ackermann
Teddy Katz
Michael Dawson
cornholio
Chris Young
Ben Noordhuis
Sebastian Van Sande
Rod Vagg
sreepurnajasti
Dan Homola
Sam Roberts
José F. Romaniello
XadillaX