Update the Writing Tests guide to specify that tests should include a
brief description of what they are designed to test.
PR-URL: https://github.com/nodejs/node/pull/9415
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>oc
If there is a link in the headings, when the ToC is generated, that is
not properly linked and the square brackets are left as they are.
Even if we fix this, different parts of the heading will link to
different sections or even different pages. For example,
### What makes [`Buffer.allocUnsafe()`] and
[`Buffer.allocUnsafeSlow()`] "unsafe"?
will point to three different sections. `allocUnsafe` and
`allocUnsafeSlow` will link to their corresponding sections and all
other words actually link to the heading in the document. This could be
visually confusing.
PR-URL: https://github.com/nodejs/node/pull/9416
Fixes: https://github.com/nodejs/node/issues/9331
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Brian White <mscdex@mscdex.net>
As the comments are indented in Makefile, they are actually echoed
on the screen. This patch makes sure that the comments actually start
at the beginning of the line, and so not echoed and ignored.
PR-URL: https://github.com/nodejs/node/pull/9375
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The governance documentation refers to a CTC meeting "moderator" but
that is a confusing term. "Meeting chair" seems more correctly
descriptive. Change instances of "moderator" to "meeting chair".
Refs: https://github.com/nodejs/CTC/issues/23
PR-URL: https://github.com/nodejs/node/pull/9432
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Josh Gavant <josh.gavant@outlook.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Timer-dependent tests fail frequently on certain platforms in CI when
run in parallel with other tests, likely due to competition for
resources. Move test-repl-timeout-throw to sequential to avoid this
problem. Also did some minor refactoring (var->const and more use of
assert.strictEqual of looser assertions).
PR-URL: https://github.com/nodejs/node/pull/9431
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergstrom <bugs@bergstroem.nu>
This is a partial revert of
14d1a8a631, which coerced the offset
of Buffer#slice() using the | operator. This causes some edge
cases to be handled incorrectly. This commit restores the old
behavior, but converts offsets to integers using Math.trunc().
This commit does not revert any tests, and adds an additional
regression test.
Refs: https://github.com/nodejs/node/issues/9096
Refs: https://github.com/nodejs/node/pull/9101
PR-URL: https://github.com/nodejs/node/pull/9341
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
* toLocaleUpperCase() and toLocaleLowerCase() do not function properly
without this flag.
* basic test case. The test case would fail if `--no_icu_case_mapping`
was set.
Fixes: https://github.com/nodejs/node/issues/9445
PR-URL: https://github.com/nodejs/node/pull/9454
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
ESLint has a `func-name-matching` rule that requires that function names
match the variable or property to which they are being assigned.
The code base currently has 100% compliance with this rule.
Enable the rule to keep it that way.
PR-URL: https://github.com/nodejs/node/pull/9408
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
* remove unnecessary backslash (`\`) escaping in regular expressions
* favor `===` over `==`
* multiline arrays indentation consistent with other indentation
PR-URL: https://github.com/nodejs/node/pull/9374
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
checkExecSyncError() creates error objects for execSync() and
execFileSync(). If the child process created stderr output, then
it is attached to the end of the error message. However, stderr
can be an empty Buffer object, which always passes the truthy
check, leading to an extra newline in the error message. This
commit adds a length check, which will work with both strings and
Buffers.
PR-URL: https://github.com/nodejs/node/pull/9343
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This inadvertently changed to `[object Object]` with the V8 upgrade
in 8a24728...96933df. Use `Symbol.toStringTag` to undo this
particular change.
Fixes: https://github.com/nodejs/node/issues/9274
PR-URL: https://github.com/nodejs/node/pull/9279
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
test-stream2-readable-empty-buffer-no-eof fails on resource-constrained
machines due to its use of timers. Removing timers makes it more
reliable and doesn’t affect the validity of the test, as it only uses
relative timing relations.
Failures were noticed on freebsd10-64 in CI. I am able to replicate the
failure with `tools/test.py --repeat=100 -j 100`. When run alone, it
passes reliably.
Refs: https://github.com/nodejs/node/pull/9359
PR-URL: hkttps://github.com/nodejs/node/pull/9360
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Add tests for constructor behavior and parameter validation. Remove
condition check that cannot be triggered (nothing is greater than
`Infinity`).
PR-URL: https://github.com/nodejs/node/pull/9366
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
As it is, node-gyp produces a lot of build related verbose messages.
Latest node-gyp upgrade allows us to specify --silent flag to suppress
those messages. Except for CI, addons build will run silently.
PR-URL: https://github.com/nodejs/node/pull/8990
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
PR-URL: https://github.com/nodejs/node/pull/7376
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Move mislocated sentences to correct location.
PR-URL: https://github.com/nodejs/node/pull/9365
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
The timer is not necessary. The test will timeout via the test harness
if the test fails. This should resolve spurious CI failures.
PR-URL: https://github.com/nodejs/node/pull/9361
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This commit adds a public channel property to ChildProcess. The
existing _channel property is aliased to the new property, with
the intention of deprecating and removing it in the future.
Fixes: https://github.com/nodejs/node/issues/9313
PR-URL: https://github.com/nodejs/node/pull/9322
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/9345
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
PR-URL: https://github.com/nodejs/node/pull/9345
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Also removed a TODO comment that is no longer viable and left a note
about the potentially confusing property naming convention for future
readers.
PR-URL: https://github.com/nodejs/node/pull/9362
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Bryan English <bryan@bryanenglish.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
the changes are related to https://github.com/nodejs/node/issues/8913
regarding the naming of just the inline anonymous
functions that are not assigned to a variable
PR-URL: https://github.com/nodejs/node/pull/9356
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
the changes are related https://github.com/nodejs/node/issues/8913
regarding the naming of just the inline anonymous
functions that are not assigned to a variable
PR-URL: https://github.com/nodejs/node/pull/9357
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
* var -> const
* remove function names where V8 inference is as good or better
* add function names where there is no V8 inference
* assert.equal -> strictEqual
* move assertion from exit handler to response end handler
PR-URL: https://github.com/nodejs/node/pull/9344
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
The changes introdcued here replace the deprecated
v8 method SetNamedPropertyHandler() to SetHandler()
in node.cc.
Prior to refactoring, the method defined callbacks
when accessing object properties defined by Strings
and not Symbols.
test/parallel/test-v8-interceptStrings-not-Symbols.js
demonstrates that this behaviour remained unchanged
after refactoring.
PR-URL: https://github.com/nodejs/node/pull/9062
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/9294
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
PR-URL: https://github.com/nodejs/node/pull/9379
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
PR-URL: https://github.com/nodejs/node/pull/9338
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
This commit removes two truthy checks for object properties that
are immediately followed by a strict equality check. The other
item in the comparison is guaranteed to be a function by this
point in the code, so the truthy check is redundant.
PR-URL: https://github.com/nodejs/node/pull/9330
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Use assert.strictEqual() instead of assert.equal(),
=== instead of ==, and const instead of var.
PR-URL: https://github.com/nodejs/node/pull/9308
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Bryan English <bryan@bryanenglish.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Remove unneeded timers from some tests and move others from parallel
testing to sequential testing.
This is to resolve test failures on freebsd10-64 on CI. The failures
are all due to timers firing later than expected. Timers firing later
than they are set for can happen on resource-constrained hosts and is
not a bug.
In general, it may be wise to put tests that depend on timing into
sequential testing rather than parallel testing, as the timing can
be affected by other simultaneously-running test processes.
Fixes: https://github.com/nodejs/node/issues/8041
Fixes: https://github.com/nodejs/node/issues/9227
PR-URL: https://github.com/nodejs/node/pull/9317
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: Johan Bergstrom <bugs@bergstroem.nu>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
_convertCustomFds() is only called from normalizeSpawnArguments(),
which always provides an options object. Therefore, there is no
need to check for options in _convertCustomFds().
PR-URL: https://github.com/nodejs/node/pull/9307
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit adds a test for spawn()'s deprecated customFds option.
PR-URL: https://github.com/nodejs/node/pull/9307
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The test has two test cases, but only the first was being run due to a
small bug. This change fixes the bug.
PR-URL: https://github.com/nodejs/node/pull/9305
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
The test was sometimes timing out due to a race condition. In OS X,
events for `fs.watch()` might only start showing up after a delay. This
is a limitation of the operating system. To work around that, there was
a timer in the test that delayed the writing of the file by 100ms.
However, sometimes that was not enough, and so the event never fired,
and the test timed out.
Change the timer to an interval so that it fires repeatedly until it is
picked up. This change only affects OS X.
Fixes: https://github.com/nodejs/node/issues/8511
PR-URL: https://github.com/nodejs/node/pull/9303
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Gibson Fahnestock
Sakthipriyan Vairamani (thefourtheye)
Rich Trott
cjihrig
Kat Marchán
Steven R. Loomis
Roman Reiss
Rod Vagg
Josh Gavant
Anna Henningsen
Kyle E. Mitchell
Zeke Sikelianos
Yangyang Liu
jedireza
Pedro Victor
AnnaMag
Jesse McCarthy
coderaiser
kobelb
Isobel Redelmeier
James M Snell