Skip to content

Commit ed130fc

Browse files
panvapanva
committed
test: deflake util.throttle tests
Short throttle windows can expire between synchronous calls when the process is descheduled. Delayed timer dispatch can also release two queued calls together, invalidating the strict test's spacing assertion. Move timing checks to a separate test with mocked timers and a matching libuv clock. Cover both punctual and delayed dispatch, and keep real async_hooks timer-allocation checks with windows canceled before expiry. Signed-off-by: Filip Skokan <panva.ip@gmail.com> Assisted-by: Codex
1 parent 26dbe11 commit ed130fc

2 files changed

Lines changed: 181 additions & 113 deletions

File tree

‎test/parallel/test-util-throttle-timing.js‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
// Flags: --expose-internals
2+
'use strict';
3+
4+
const common = require('../common');
5+
const assert = require('node:assert');
6+
const { mock } = require('node:test');
7+
const { setImmediate } = require('node:timers/promises');
8+
const { internalBinding } = require('internal/test/binding');
9+
10+
// Throttle uses the libuv clock as well as timers. Control both before loading
11+
// the implementation, which captures setTimeout and clearTimeout.
12+
mock.timers.enable({ apis: ['Date', 'setTimeout'] });
13+
mock.method(internalBinding('timers'), 'getLibuvNow', () => Date.now());
14+
const { throttle } = require('node:util');
15+
16+
process.on('unhandledRejection', common.mustNotCall());
17+
18+
(async () => {
19+
{
20+
const values = [];
21+
const times = [];
22+
const start = Date.now();
23+
const throttled = throttle(common.mustCall(function(value) {
24+
assert.strictEqual(this, throttled);
25+
values.push(value);
26+
times.push(Date.now() - start);
27+
return value;
28+
}, 5), 2, 40);
29+
30+
assert.strictEqual(typeof throttled.cancel, 'function');
31+
assert.strictEqual(typeof throttled.hasImmediateCapacity, 'function');
32+
assert.strictEqual(typeof throttled.ref, 'function');
33+
assert.strictEqual(typeof throttled.unref, 'function');
34+
assert.strictEqual(throttled.pending, null);
35+
assert.strictEqual(throttled.pendingCount, 0);
36+
assert.strictEqual(throttled.activeCount, 0);
37+
assert.strictEqual(throttled.hasImmediateCapacity(), true);
38+
39+
const first = throttled(1);
40+
const second = throttled(2);
41+
const third = throttled(3);
42+
const fourth = throttled(4);
43+
const fifth = throttled(5);
44+
45+
assert(first instanceof Promise);
46+
assert(second instanceof Promise);
47+
assert.notStrictEqual(first, second);
48+
assert.deepStrictEqual(values, [1, 2]);
49+
assert.strictEqual(throttled.hasImmediateCapacity(), false);
50+
assert.strictEqual(throttled.pending, fifth);
51+
assert.strictEqual(throttled.pendingCount, 3);
52+
assert.strictEqual(throttled.unref(), throttled);
53+
assert.strictEqual(throttled.ref(), throttled);
54+
55+
mock.timers.tick(39);
56+
assert.deepStrictEqual(values, [1, 2]);
57+
mock.timers.tick(1);
58+
assert.deepStrictEqual(values, [1, 2, 3, 4]);
59+
assert.strictEqual(throttled.pendingCount, 1);
60+
mock.timers.tick(39);
61+
assert.deepStrictEqual(values, [1, 2, 3, 4]);
62+
mock.timers.tick(1);
63+
64+
assert.deepStrictEqual(
65+
await Promise.all([first, second, third, fourth, fifth]),
66+
[1, 2, 3, 4, 5],
67+
);
68+
assert.deepStrictEqual(values, [1, 2, 3, 4, 5]);
69+
assert.strictEqual(throttled.pending, null);
70+
assert.strictEqual(throttled.pendingCount, 0);
71+
assert.strictEqual(throttled.activeCount, 0);
72+
assert.deepStrictEqual(times, [0, 0, 40, 40, 80]);
73+
}
74+
75+
{
76+
const values = [];
77+
const throttled = throttle(common.mustCall((value) => {
78+
values.push(value);
79+
return value;
80+
}, 2), 1, 20, { maxPending: 1 });
81+
const first = throttled(1);
82+
const second = throttled(2);
83+
const dropped = throttled(3);
84+
85+
assert.deepStrictEqual(values, [1]);
86+
assert.strictEqual(throttled.pendingCount, 1);
87+
await setImmediate();
88+
await assert.rejects(dropped, { code: 'ERR_THROTTLED' });
89+
mock.timers.tick(19);
90+
assert.deepStrictEqual(values, [1]);
91+
mock.timers.tick(1);
92+
assert.deepStrictEqual(await Promise.all([first, second]), [1, 2]);
93+
assert.deepStrictEqual(values, [1, 2]);
94+
}
95+
96+
{
97+
const values = [];
98+
const throttled = throttle(common.mustCall((value) => {
99+
values.push(value);
100+
return value;
101+
}, 3), 2, 30, { overflow: 'drop' });
102+
const first = throttled(1);
103+
const second = throttled(2);
104+
await assert.rejects(throttled(3), { code: 'ERR_THROTTLED' });
105+
assert.deepStrictEqual(await Promise.all([first, second]), [1, 2]);
106+
107+
mock.timers.tick(29);
108+
assert.strictEqual(throttled.hasImmediateCapacity(), false);
109+
await assert.rejects(throttled(4), { code: 'ERR_THROTTLED' });
110+
mock.timers.tick(1);
111+
assert.strictEqual(throttled.hasImmediateCapacity(), true);
112+
assert.strictEqual(await throttled(5), 5);
113+
assert.deepStrictEqual(values, [1, 2, 5]);
114+
}
115+
116+
// A rolling window releases one slot at a time when timers run on schedule.
117+
// If dispatch is delayed until both slots expire, both calls may run together.
118+
for (const delayed of [false, true]) {
119+
const times = [];
120+
const start = Date.now();
121+
const throttled = throttle(common.mustCall((value) => {
122+
times.push(Date.now() - start);
123+
return value;
124+
}, 4), 2, 80, { strict: true });
125+
126+
const first = throttled(1);
127+
mock.timers.tick(40);
128+
const second = throttled(2);
129+
const third = throttled(3);
130+
const fourth = throttled(4);
131+
132+
mock.timers.tick(39);
133+
assert.deepStrictEqual(times, [0, 40]);
134+
if (delayed) {
135+
mock.timers.tick(41);
136+
} else {
137+
mock.timers.tick(1);
138+
assert.deepStrictEqual(times, [0, 40, 80]);
139+
mock.timers.tick(39);
140+
assert.deepStrictEqual(times, [0, 40, 80]);
141+
mock.timers.tick(1);
142+
}
143+
144+
assert.deepStrictEqual(
145+
await Promise.all([first, second, third, fourth]),
146+
[1, 2, 3, 4],
147+
);
148+
assert.deepStrictEqual(times, delayed ? [0, 40, 120, 120] : [0, 40, 80, 120]);
149+
}
150+
151+
{
152+
let recursive;
153+
const values = [];
154+
const throttled = throttle(common.mustCall((value) => {
155+
values.push(value);
156+
if (value === 1) recursive = throttled(2);
157+
return value;
158+
}, 2), 1, 20);
159+
160+
const first = throttled(1);
161+
assert.deepStrictEqual(values, [1]);
162+
assert.strictEqual(throttled.pending, recursive);
163+
assert.strictEqual(throttled.pendingCount, 1);
164+
mock.timers.tick(19);
165+
assert.deepStrictEqual(values, [1]);
166+
mock.timers.tick(1);
167+
assert.deepStrictEqual(await Promise.all([first, recursive]), [1, 2]);
168+
assert.deepStrictEqual(values, [1, 2]);
169+
}
170+
})().then(common.mustCall()).finally(() => mock.reset());

‎test/parallel/test-util-throttle.js‎

Lines changed: 11 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
const common = require('../common');
55
const assert = require('node:assert');
66
const { createHook } = require('node:async_hooks');
7-
const { setImmediate, setTimeout } = require('node:timers/promises');
7+
const { setImmediate } = require('node:timers/promises');
88
const { throttle } = require('node:util');
99
const { TIMEOUT_MAX } = require('internal/timers');
1010

@@ -95,55 +95,9 @@ throttle(() => {}, 1, 1, {
9595
);
9696
}
9797

98+
// Keep windows open across synchronous assertions, even if the process is
99+
// descheduled. Window expiration is covered in test-util-throttle-timing.js.
98100
(async () => {
99-
{
100-
const values = [];
101-
const times = [];
102-
const start = Date.now();
103-
const throttled = throttle(common.mustCall(function(value) {
104-
assert.strictEqual(this, throttled);
105-
values.push(value);
106-
times.push(Date.now() - start);
107-
return value;
108-
}, 5), 2, 40);
109-
110-
assert.strictEqual(typeof throttled.cancel, 'function');
111-
assert.strictEqual(typeof throttled.hasImmediateCapacity, 'function');
112-
assert.strictEqual(typeof throttled.ref, 'function');
113-
assert.strictEqual(typeof throttled.unref, 'function');
114-
assert.strictEqual(throttled.pending, null);
115-
assert.strictEqual(throttled.pendingCount, 0);
116-
assert.strictEqual(throttled.activeCount, 0);
117-
assert.strictEqual(throttled.hasImmediateCapacity(), true);
118-
119-
const first = throttled(1);
120-
const second = throttled(2);
121-
const third = throttled(3);
122-
const fourth = throttled(4);
123-
const fifth = throttled(5);
124-
125-
assert(first instanceof Promise);
126-
assert(second instanceof Promise);
127-
assert.notStrictEqual(first, second);
128-
assert.deepStrictEqual(values, [1, 2]);
129-
assert.strictEqual(throttled.hasImmediateCapacity(), false);
130-
assert.strictEqual(throttled.pending, fifth);
131-
assert.strictEqual(throttled.pendingCount, 3);
132-
assert.strictEqual(throttled.unref(), throttled);
133-
assert.strictEqual(throttled.ref(), throttled);
134-
135-
assert.deepStrictEqual(
136-
await Promise.all([first, second, third, fourth, fifth]),
137-
[1, 2, 3, 4, 5],
138-
);
139-
assert.deepStrictEqual(values, [1, 2, 3, 4, 5]);
140-
assert.strictEqual(throttled.pending, null);
141-
assert.strictEqual(throttled.pendingCount, 0);
142-
assert.strictEqual(throttled.activeCount, 0);
143-
assert(times[2] - times[0] >= 30);
144-
assert(times[4] - times[2] >= 30);
145-
}
146-
147101
{
148102
let running = 0;
149103
let maxRunning = 0;
@@ -220,24 +174,6 @@ throttle(() => {}, 1, 1, {
220174
assert.strictEqual(throttled.activeCount, 0);
221175
}
222176

223-
{
224-
const values = [];
225-
const throttled = throttle(common.mustCall((value) => {
226-
values.push(value);
227-
return value;
228-
}, 2), 1, 20, { maxPending: 1 });
229-
const first = throttled(1);
230-
const second = throttled(2);
231-
const dropped = throttled(3);
232-
233-
assert.deepStrictEqual(values, [1]);
234-
assert.strictEqual(throttled.pendingCount, 1);
235-
await setImmediate();
236-
await assert.rejects(dropped, { code: 'ERR_THROTTLED' });
237-
assert.deepStrictEqual(await Promise.all([first, second]), [1, 2]);
238-
assert.deepStrictEqual(values, [1, 2]);
239-
}
240-
241177
{
242178
let timeoutCount = 0;
243179
const hook = createHook({
@@ -249,7 +185,7 @@ throttle(() => {}, 1, 1, {
249185
const throttled = throttle(common.mustCall((value) => {
250186
values.push(value);
251187
return value;
252-
}, 3), 2, 30, { overflow: 'drop' });
188+
}, 3), 2, TIMEOUT_MAX, { overflow: 'drop' });
253189

254190
hook.enable();
255191
const first = throttled(1);
@@ -265,37 +201,14 @@ throttle(() => {}, 1, 1, {
265201
await assert.rejects(dropped, { code: 'ERR_THROTTLED' });
266202
assert.deepStrictEqual(await Promise.all([first, second]), [1, 2]);
267203

268-
await setTimeout(30);
204+
throttled.cancel();
269205
assert.strictEqual(await throttled(4), 4);
270206
assert.deepStrictEqual(values, [1, 2, 4]);
271207
}
272208

273-
{
274-
const times = [];
275-
const start = Date.now();
276-
const throttled = throttle(common.mustCall((value) => {
277-
times.push(Date.now() - start);
278-
return value;
279-
}, 4), 2, 80, { strict: true });
280-
281-
const first = throttled(1);
282-
await setTimeout(40);
283-
const second = throttled(2);
284-
const third = throttled(3);
285-
const fourth = throttled(4);
286-
287-
assert.deepStrictEqual(
288-
await Promise.all([first, second, third, fourth]),
289-
[1, 2, 3, 4],
290-
);
291-
assert(times[2] - times[0] >= 65);
292-
assert(times[3] - times[1] >= 65);
293-
assert(times[3] - times[2] >= 25);
294-
}
295-
296209
{
297210
const reason = new Error('cancelled');
298-
const throttled = throttle(common.mustCall((value) => value, 2), 1, 100);
211+
const throttled = throttle(common.mustCall((value) => value, 2), 1, TIMEOUT_MAX);
299212
const first = throttled(1);
300213
const second = throttled(2);
301214
const third = throttled(3);
@@ -322,7 +235,7 @@ throttle(() => {}, 1, 1, {
322235
{
323236
const reason = new Error('stop');
324237
const controller = new AbortController();
325-
const throttled = throttle(common.mustCall((value) => value), 1, 100, {
238+
const throttled = throttle(common.mustCall((value) => value), 1, TIMEOUT_MAX, {
326239
signal: controller.signal,
327240
});
328241
const first = throttled(1);
@@ -363,23 +276,6 @@ throttle(() => {}, 1, 1, {
363276
await assert.rejects(throttled(), (error) => error === expected);
364277
}
365278

366-
{
367-
let recursive;
368-
const values = [];
369-
const throttled = throttle(common.mustCall((value) => {
370-
values.push(value);
371-
if (value === 1) recursive = throttled(2);
372-
return value;
373-
}, 2), 1, 20);
374-
375-
const first = throttled(1);
376-
assert.deepStrictEqual(values, [1]);
377-
assert.strictEqual(throttled.pending, recursive);
378-
assert.strictEqual(throttled.pendingCount, 1);
379-
assert.deepStrictEqual(await Promise.all([first, recursive]), [1, 2]);
380-
assert.deepStrictEqual(values, [1, 2]);
381-
}
382-
383279
{
384280
function original(first, second) {
385281
return first + second;
@@ -409,7 +305,7 @@ throttle(() => {}, 1, 1, {
409305
if (type === 'Timeout') timeoutCount++;
410306
},
411307
});
412-
const throttled = throttle(common.mustCall((value) => value), 1, 100);
308+
const throttled = throttle(common.mustCall((value) => value), 1, TIMEOUT_MAX);
413309

414310
hook.enable();
415311
assert.strictEqual(throttled.hasImmediateCapacity(), true);
@@ -430,7 +326,7 @@ throttle(() => {}, 1, 1, {
430326
if (type === 'Timeout') timeoutCount++;
431327
},
432328
});
433-
const throttled = throttle(common.mustCall((value) => value), 1, 100);
329+
const throttled = throttle(common.mustCall((value) => value), 1, TIMEOUT_MAX);
434330
hook.enable();
435331
const first = throttled(1);
436332
const second = throttled(2);
@@ -440,6 +336,8 @@ throttle(() => {}, 1, 1, {
440336
const thirdRejection = assert.rejects(third, { code: 'ABORT_ERR' });
441337

442338
assert.strictEqual(timeoutCount, 1);
339+
assert.strictEqual(throttled.unref(), throttled);
340+
assert.strictEqual(throttled.ref(), throttled);
443341
throttled.cancel();
444342
assert.strictEqual(await first, 1);
445343
await Promise.all([secondRejection, thirdRejection]);

0 commit comments

Comments
 (0)