Skip to content

Commit

Permalink
test: tune down parallelism for some flaky tests
Browse files Browse the repository at this point in the history
These tests seem to timeout quite often. I don't know why, but one
possible reason is that they are starting a lot of threads. It seems
that tests in `test/parallel` are assumed to only start one thread each,
so having 11 threads running at a time feels like a lot.

It also seems that these tests fail in a correlated fashion: take a look
at [this reliability report][]. The failures all occur on the same build
machines on the same PRs. This suggests to me some sort of CPU
contention.

[this reliability report]: nodejs/reliability#334

On my Linux machine decreasing the parallelism & iterations here reduce
the `user` time from ~11.5 seconds to ~2 seconds, depending on the test.
I have seen these tests take 30-60 seconds on CI (Alpine in particular).

I went back to the diffs that introduced that introduced these changes
and verified that they failed at least 90% of the time with the reduced
iteration count, which feels sufficient.

Refs: #43499
Refs: #43084
PR-URL: #44090
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
  • Loading branch information
kvakil committed Aug 5, 2022
1 parent 0e034e6 commit 9ec2787
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 6 deletions.
4 changes: 2 additions & 2 deletions test/parallel/test-worker-fshandles-error-on-termination.js
Expand Up @@ -6,8 +6,8 @@ const fs = require('fs/promises');
const { scheduler } = require('timers/promises');
const { parentPort, Worker } = require('worker_threads');

const MAX_ITERATIONS = 20;
const MAX_THREADS = 10;
const MAX_ITERATIONS = 5;
const MAX_THREADS = 6;

// Do not use isMainThread so that this test itself can be run inside a Worker.
if (!process.env.HAS_STARTED_WORKER) {
Expand Down
Expand Up @@ -6,8 +6,8 @@ const fs = require('fs/promises');
const { scheduler } = require('timers/promises');
const { parentPort, Worker } = require('worker_threads');

const MAX_ITERATIONS = 20;
const MAX_THREADS = 10;
const MAX_ITERATIONS = 5;
const MAX_THREADS = 6;

// Do not use isMainThread so that this test itself can be run inside a Worker.
if (!process.env.HAS_STARTED_WORKER) {
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-worker-http2-stream-terminate.js
Expand Up @@ -11,8 +11,8 @@ const { Worker, parentPort } = require('worker_threads');
// stream activity is ongoing, in particular the C++ function
// ReportWritesToJSStreamListener::OnStreamAfterReqFinished.

const MAX_ITERATIONS = 20;
const MAX_THREADS = 10;
const MAX_ITERATIONS = 5;
const MAX_THREADS = 6;

// Do not use isMainThread so that this test itself can be run inside a Worker.
if (!process.env.HAS_STARTED_WORKER) {
Expand Down

0 comments on commit 9ec2787

Please sign in to comment.