test-dh-regr is timing out in CI because crypto.createDiffieHellman() is
considerably slower after an OpenSSL upgrade. This PR modifies the
change from 11ad744a92 which made the test
FIPS-compatible. When not in FIPS mode, the test will use a shorter key
which will enable it to run much faster.
PR-URL: https://github.com/nodejs/node/pull/28390
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Split test-crypto-dh into two tests so that it does not time out in CI.
With a recent OpenSSL upgrade, getDiffieHellman() is much slower than
before.
PR-URL: https://github.com/nodejs/node/pull/28390
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Some parts of pummel/test-crypto-dh.js will be just fine in
parallel/test-crypto-dh.js. Move them there.
PR-URL: https://github.com/nodejs/node/pull/28390
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Lots of changes, but mostly just search/replace of
fixtures.readSync(...) to fixtures.readKey([new key]...)
Benchmarks modified to use fixtures.readKey(...):
benchmark/tls/throughput.js
benchmark/tls/tls-connect.js
benchmark/tls/secure-pair.js
Also be sure to review the change to L16 of
test/parallel/test-crypto-sign-verify.js
PR-URL: https://github.com/nodejs/node/pull/27962
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Lots of changes, but mostly just search/replace of
fixtures.readSync(...) to fixtures.readKey([new key]...)
Benchmarks modified to use fixtures.readKey(...):
benchmark/tls/throughput.js
benchmark/tls/tls-connect.js
benchmark/tls/secure-pair.js
Also be sure to review the change to L16 of
test/parallel/test-crypto-sign-verify.js
PR-URL: https://github.com/nodejs/node/pull/27962
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
The test writes out a large file via `fs.createWriteStream()` but was
not listening for the `error` event, which the `fs` docs describe as the
reliable way to detect write errors.
PR-URL: https://github.com/nodejs/node/pull/27058
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This allows us to track the essentially-global objects in
Environment in the heap snapshot. Note that this patch only
tracks the fields that can be tracked correctly. There are
still several types of fields that cannot be tracked:
- v8::Data including v8::Private, v8::ObjectTemplate etc.
- Internal types that do not implement MemoryRetainer yet
- STL containers with MemoryRetainer* inside
- STL containers with numeric types inside that should not have their
nodes elided e.g. numeric keys in maps.
The `BaseObject`s are now no longer globals. They are tracked
as arguments in CleanupHookCallbacks referenced by the Environment
node. This model is closer to how their lifetime is managed
internally.
To track the per-environment strong persistent properties, this patch
divides them into those that are also `v8::Value` and those that
are just `v8::Data`. The values can be tracked by the current
memory tracker while the data cannot.
This patch also implements the `MemoryRetainer` interface in several
internal classes so that they can be tracked in the heap snapshot.
PR-URL: https://github.com/nodejs/node/pull/27018
Refs: https://github.com/nodejs/node/issues/26776
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
The test pummel/test-dtrace-jsstack is broken and probably has been for
a very long time. Remove it.
It gets skipped on anything that is non-SunOS. In our CI, that means
skipped everywhere but SmartOS.
When run on SmartOS in our CI (which never happens because it's in
pummel, but I moved it into sequential to test it), it fails because it
needs elevated privileges.
When I log into the SmartOS machine and run the test as root, it fails
with:
AssertionError [ERR_ASSERTION]: did not find expected frame stalloogle
Since I have dtrace installed on my macOS machine, I tried running it
locally but removing the SunOS check. It failed because the test leaks a
global variable. I removed the global leak check, and the test failed
because I have System Integrity Protection enabled.
In short, the test does not work in its current form, has almost
certainly not worked in a long time, and is very likely to be brittle if
we ever do fix it. I'm inclined to remove it.
PR-URL: https://github.com/nodejs/node/pull/26869
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Move test-tls-session-timeout from pummel to sequential. It isn't very
pummel-y and this will result in it being run on our CI more than once a
day. (It broke recently and it would have been caught if it was in
sequential rather than pummel.)
It must be in sequential rather than pummel because it uses
`common.PORT` which can result in test failures if more than one test
uses it at one time in parallel.
PR-URL: https://github.com/nodejs/node/pull/26865
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
The test does not work with TLS 1.3 nor should it. Force TLS version
1.2.
While at it, some refactoring:
* refresh the tmp directory in case it doesn't exist!
* add an assert.strictEqual() check on the client return `code` value
which must be zero
* use arrow functions for callbacks
* add trailing commas for multiline arrays/objects
Fixes: https://github.com/nodejs/node/issues/26839
PR-URL: https://github.com/nodejs/node/pull/26865
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
The current mechanism of uses two async handles, one owned by the
creator of the worker thread to terminate a running worker,
and another one employed by the worker to interrupt its creator on its
natural termination. The force termination piggybacks on the message-
passing mechanism to inform the worker to quiesce.
Also there are few flags that represent the other thread's state /
request state because certain code path is shared by multiple
control flows, and there are certain code path where the async
handles may not have come to life.
Refactor into an AsyncRequest abstraction that exposes routines to
install a handle as well as to save a state.
PR-URL: https://github.com/nodejs/node/pull/26099
Refs: https://github.com/nodejs/node/pull/21283
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Using `eval()` rather than `require()`'ing a fixture and deleting it
from the cache results in a roughtly 10x improvement in running time.
Fixes: https://github.com/nodejs/node/issues/25984
Refs: https://github.com/nodejs/node/issues/26229
PR-URL: https://github.com/nodejs/node/pull/26237
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
In addition, do not make too many assumptions about the startup
time and timer latency in test-process-uptime. Instead only test
that the value is likely in the correct unit (seconds) and it should
be increasing in subsequent calls.
PR-URL: https://github.com/nodejs/node/pull/26206
Fixes: https://github.com/nodejs/node/issues/26205
Refs: https://github.com/nodejs/node/pull/26016
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
* Move client renegotiation limit tests from pummel to parallel.
* Rename tests to more accurately reflect what they do.
* Refactor to use arrow functions for anonymouse callbacks and to be
consistent about trailing commas.
PR-URL: https://github.com/nodejs/node/pull/25757
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The heapdump tests take a lot more time to run than our other tests in
parallel. They are also a bit of an internal test that perhaps does not
need to be run on every commit on every platform. This change moves them
to the pummel directory where they will be run on a single platform once
a day in CI.
This shaves more than 20 seconds off `make test` on my laptop, FWIW.
PR-URL: https://github.com/nodejs/node/pull/25181
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Replace `s_client` in test-https-ci-reneg-attack with built-in
client calling `tls.renegotiate()`. This also fixes the currently-broken
test. (It is broken due to a change in behavior in a
recently-updated-in-core version of `s_client`.)
PR-URL: https://github.com/nodejs/node/pull/25720
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Fix test/pummel/test-exec.js which broke as a result of
e47f972d68
(https://github.com/nodejs/node/pull/24951).
(Until very recently, pummel tests were not run at all in CI and
currently only run nightly on master.)
PR-URL: https://github.com/nodejs/node/pull/25677
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
In test/pummel/test-https-ci-reneg-attack.js, there is a boolean that is
causing a race condition on some operating systems. It is unnecessary.
Remove it.
PR-URL: https://github.com/nodejs/node/pull/25601
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Repeated use of common.PORT was resulting in sporadic failures on some
operating systems.
PR-URL: https://github.com/nodejs/node/pull/25599
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reduce the time it takes to run test/pummel/test-hash-seed by switching
from spawnSync() to spawn(). On my computer, this reduces the runtime
from about 80 seconds to about 40 seconds. This test is not (yet) run
regularly on CI, but when it was run recently, it timed out.
PR-URL: https://github.com/nodejs/node/pull/25522
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
test/pummel/test-keep-alive.js requires `wrk` to be installed. Check if
it is, and skip the test if it isn't.
This is yet another step in preparation for running pummel tests in CI
daily.
PR-URL: https://github.com/nodejs/node/pull/25516
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
* Use port 0 instead of `common.PORT`.
* Use `//` for comments, capitalize comments, and add punctuation.
PR-URL: https://github.com/nodejs/node/pull/25485
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
* Use port 0 instead of `common.PORT`.
* Reduce `concurrent` from 100 to 50 and `connections_per_client` from 5
to 3. This is to avoid side effects from other tests. Prior to this
change, running this along with test-keep-alive would result in
failures on my local setup, apparently due to network throttling.
* Remove unnecessary `console.log()` and improve remaining
`console.log()` to provide clearer information.
PR-URL: https://github.com/nodejs/node/pull/25485
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
* Reduce ROUNDS and ATTEMPTS_PER_ROUND by half to avoid spurious test
failures as a result of side effects from other tests. (For my local
setup, test-keep-alive seems to cause this test to fail with ETIMEDOUT
and/or EADDRNOTAVAIL. It would seem to be a result of throttling.
Reducing the pummel-iness of that test and this one seems to solve the
problem.)
* Apply capitalization and punctuation to comment.
PR-URL: https://github.com/nodejs/node/pull/25485
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
* Reduce concurrent and duration options by half so as to avoid
interference with other tests. (Excessive TCP activity in this test
resulted in throttling that caused subsequent tests to fail on my
local setup.)
* Use an OS-provided port rather than `common.PORT`. This possibly
reduces side-effects on other tests (that may also be using
`common.PORT`).
* Add punctuation in comments.
PR-URL: https://github.com/nodejs/node/pull/25485
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
test-net-connect-econnrefused was recently fixed, but only in certain
circumstances. This change allows it to succeed whether it is invoked
with `node` or `tools/test.py`. Makes sure no Socket handles are left,
which is what the test is trying to determine, rather than failing if
there are no handles of any kind left.
PR-URL: https://github.com/nodejs/node/pull/25438
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
test/pummel/test-net-connect-econnrefused.js was failing because
`console.log()` resulted in an extra handle being returned by
`process._getActiveHandles()`. Remove the unnecessary `console.log()`.
PR-URL: https://github.com/nodejs/node/pull/25389
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
test-http-client-reconnect-bug depends on `http.createClient()` which
was removed in Node.js 7.0.0. The test was added way back in commit
30b0522157 for a bug fixed in Node.js
0.1.27 in early 2010. We've apparently been fine with it failing since
at least Node.js 7.0.0 which at this time is more than 2 years ago.
Remove this test.
PR-URL: https://github.com/nodejs/node/pull/25387
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
The test was duplicating some functionality in the `tmpdir` module.
PR-URL: https://github.com/nodejs/node/pull/25386
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
test-fs-watch-non-recursive was loading the `common/tmpdir` module as if
it were a built-in and was failing because of it. Fix the path. The test
now works.
PR-URL: https://github.com/nodejs/node/pull/25386
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
test-fs-watch-file.js fails for two reasons. First, there are cases
where it is checking the error message for an error whose message has
changed since the test was written. Change these instances to check for
an error code instead.
Second, there is an instance where it tries to remove a listener but
fails because `common.mustNotCall()` returns a differnet instance of a
function on each call. Store the function in a variable name so it can
be removed as a listener on a file.
PR-URL: https://github.com/nodejs/node/pull/25384
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
test-fs-largefile.js calls `fs.close()` without a callback which results
in an error as of Node.js 10.0.0. Add a `common.mustCall()` callback so
the test passes again.
PR-URL: https://github.com/nodejs/node/pull/25372
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
test-tls-securepair-client does not seem to need to be in the pummel
directory. Move it to sequential. (It can't go into parallel because it
uses common.PORT and therefore might conflict with another test that is
using system-assigned available ports.)
PR-URL: https://github.com/nodejs/node/pull/25222
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
test-tls-securepair-client has been failing for over a year but no one
noticed because pummel tests are almost never run.
In preparation for running pummel tests once a day in CI, fix
test-tls-securepair-client.
PR-URL: https://github.com/nodejs/node/pull/25222
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Tests in pummel seem to break often and stay broken because they don't
get run in CI. In preparation for running pummel tests in CI once a day,
this fixes test-tls-session-timeout. `key` and `cert` are now the
contents of the relevant files and not the paths.
PR-URL: https://github.com/nodejs/node/pull/25188
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This adds the `capitalized-comments` eslint rule to verify that
actual sentences use capital letters as starting letters. It ignores
special words and all lines below 62 characters.
PR-URL: https://github.com/nodejs/node/pull/24808
Reviewed-By: Sam Ruby <rubys@intertwingly.net>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
