Skip to content

Commit

Permalink
test: deflake test-perf-hooks.js
Browse files Browse the repository at this point in the history
Previously when checking the initial timing we did a lot of checks
after accessing and copying timing.duration and before we check
that timing.duration is roughly the same as performance.now(),
which can lead to flakes if the overhead of the checking is
big enough. Update the test to check timing.duration against
performance.now() as soon as possible when it's copied instead
of computed.
:#

PR-URL: #49892
Refs: nodejs/reliability#676
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
  • Loading branch information
joyeecheung authored and targos committed Nov 11, 2023
1 parent f564ed4 commit 126407d
Showing 1 changed file with 32 additions and 30 deletions.
62 changes: 32 additions & 30 deletions test/sequential/test-perf-hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,40 @@ const epsilon = 50;
assert.strictEqual(performance.nodeTiming.name, 'node');
assert.strictEqual(performance.nodeTiming.entryType, 'node');

// If timing.duration gets copied into the argument instead of being computed
// via the getter, this should be called right after timing is created.
function checkNodeTiming(timing) {
// Calculate the difference between now() and duration as soon as possible.
const now = performance.now();
const delta = Math.abs(now - timing.duration);

log(JSON.stringify(timing, null, 2));
// Check that the properties are still reasonable.
assert.strictEqual(timing.name, 'node');
assert.strictEqual(timing.entryType, 'node');

// Check that duration is positive and practically the same as
// performance.now() i.e. measures Node.js instance up time.
assert.strictEqual(typeof timing.duration, 'number');
assert(timing.duration > 0, `timing.duration ${timing.duration} <= 0`);
assert(delta < 10,
`now (${now}) - timing.duration (${timing.duration}) = ${delta} >= ${10}`);

// Check that the following fields do not change.
assert.strictEqual(timing.startTime, initialTiming.startTime);
assert.strictEqual(timing.nodeStart, initialTiming.nodeStart);
assert.strictEqual(timing.v8Start, initialTiming.v8Start);
assert.strictEqual(timing.environment, initialTiming.environment);
assert.strictEqual(timing.bootstrapComplete, initialTiming.bootstrapComplete);

assert.strictEqual(typeof timing.loopStart, 'number');
assert.strictEqual(typeof timing.loopExit, 'number');
}

log('check initial nodeTiming');
// Copy all the values from the getters.
const initialTiming = { ...performance.nodeTiming };
checkNodeTiming(initialTiming);

{
const {
Expand Down Expand Up @@ -87,36 +119,6 @@ const initialTiming = { ...performance.nodeTiming };
`bootstrapComplete ${bootstrapComplete} >= ${testStartTime}`);
}

function checkNodeTiming(timing) {
// Calculate the difference between now() and duration as soon as possible.
const now = performance.now();
const delta = Math.abs(now - timing.duration);

log(JSON.stringify(timing, null, 2));
// Check that the properties are still reasonable.
assert.strictEqual(timing.name, 'node');
assert.strictEqual(timing.entryType, 'node');

// Check that duration is positive and practically the same as
// performance.now() i.e. measures Node.js instance up time.
assert.strictEqual(typeof timing.duration, 'number');
assert(timing.duration > 0, `timing.duration ${timing.duration} <= 0`);
assert(delta < 10,
`now (${now}) - timing.duration (${timing.duration}) = ${delta} >= 10`);

// Check that the following fields do not change.
assert.strictEqual(timing.startTime, initialTiming.startTime);
assert.strictEqual(timing.nodeStart, initialTiming.nodeStart);
assert.strictEqual(timing.v8Start, initialTiming.v8Start);
assert.strictEqual(timing.environment, initialTiming.environment);
assert.strictEqual(timing.bootstrapComplete, initialTiming.bootstrapComplete);

assert.strictEqual(typeof timing.loopStart, 'number');
assert.strictEqual(typeof timing.loopExit, 'number');
}

log('check initial nodeTiming');
checkNodeTiming(initialTiming);
assert.strictEqual(initialTiming.loopExit, -1);

function checkValue(timing, name, min, max) {
Expand Down

0 comments on commit 126407d

Please sign in to comment.