Skip to content

Commit 717818d

Browse files
fansenzefansenze
authored
test(eslint-plugin-runner): skip worker-pool e2e suites on windows (napi terminate aborts) (#1031)
1 parent b289093 commit 717818d

6 files changed

Lines changed: 1388 additions & 1331 deletions

‎packages/eslint-plugin-runner/tests/worker-pool-e2e-cancel.test.ts‎

Lines changed: 135 additions & 127 deletions
Original file line numberDiff line numberDiff line change
@@ -17,141 +17,149 @@ import {
1717
* parseError:shutdown).
1818
*/
1919

20-
describe('WorkerPool end-to-end with a local fixture plugin', () => {
21-
test('cancelTask inside onTaskDispatched aborts before worker runs', async () => {
22-
const pool = new WorkerPool({
23-
configs: localConfigs,
24-
workerCount: 1,
25-
taskTimeoutMs: 10_000,
26-
});
27-
await pool.init();
28-
29-
const tasks: LintTask[] = [task('a.ts', 'const x = null;')];
30-
31-
const dispatchedIds: number[] = [];
32-
const results = await pool.lintBatch(tasks, (taskId) => {
33-
dispatchedIds.push(taskId);
34-
expect(pool.cancelTask(taskId)).toBe(true);
35-
});
36-
37-
expect(dispatchedIds).toHaveLength(1);
38-
expect(results).toHaveLength(1);
39-
expect(results[0].cancelled).toBe(true);
40-
expect(results[0].diagnostics).toHaveLength(0);
41-
42-
await pool.shutdown();
43-
}, 20_000);
44-
45-
test('queue: cancelTask drops a queued (not-yet-dispatched) task without posting to worker', async () => {
46-
// Drive the queue path: with the worker forced non-idle, the
47-
// task lands in pendingQueue. cancelTask on its id must mark
48-
// it cancelled. Releasing the worker (kickQueue) then resolves
49-
// it as cancelled=true without ever posting to the worker.
50-
const pool = new WorkerPool({
51-
configs: localConfigs,
52-
workerCount: 1,
53-
});
54-
await pool.init();
55-
try {
20+
// Skipped on windows: tearing down a worker that has oxc (a napi addon)
21+
// loaded aborts below the JS layer there (nodejs/node#34567) and crashes
22+
// the rstest worker running this file. These e2e tests spawn real
23+
// workers and tear them down, so they are windows-skipped; they still
24+
// run on linux/macOS.
25+
describe.skipIf(process.platform === 'win32')(
26+
'WorkerPool end-to-end with a local fixture plugin',
27+
() => {
28+
test('cancelTask inside onTaskDispatched aborts before worker runs', async () => {
29+
const pool = new WorkerPool({
30+
configs: localConfigs,
31+
workerCount: 1,
32+
taskTimeoutMs: 10_000,
33+
});
34+
await pool.init();
35+
36+
const tasks: LintTask[] = [task('a.ts', 'const x = null;')];
37+
38+
const dispatchedIds: number[] = [];
39+
const results = await pool.lintBatch(tasks, (taskId) => {
40+
dispatchedIds.push(taskId);
41+
expect(pool.cancelTask(taskId)).toBe(true);
42+
});
43+
44+
expect(dispatchedIds).toHaveLength(1);
45+
expect(results).toHaveLength(1);
46+
expect(results[0].cancelled).toBe(true);
47+
expect(results[0].diagnostics).toHaveLength(0);
48+
49+
await pool.shutdown();
50+
}, 20_000);
51+
52+
test('queue: cancelTask drops a queued (not-yet-dispatched) task without posting to worker', async () => {
53+
// Drive the queue path: with the worker forced non-idle, the
54+
// task lands in pendingQueue. cancelTask on its id must mark
55+
// it cancelled. Releasing the worker (kickQueue) then resolves
56+
// it as cancelled=true without ever posting to the worker.
57+
const pool = new WorkerPool({
58+
configs: localConfigs,
59+
workerCount: 1,
60+
});
61+
await pool.init();
62+
try {
63+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
64+
const ws = (pool as any).workers as Array<{ ready: boolean }>;
65+
ws[0].ready = false;
66+
67+
let capturedId = -1;
68+
const batchP = pool.lintBatch(
69+
[
70+
{
71+
filePath: 'q.ts',
72+
text: 'const x = null;\n',
73+
rules: { 'local/no-null': { options: [] } },
74+
collectFixes: false,
75+
suggestionsMode: 'off',
76+
configKey: LOCAL_CONFIG_DIR,
77+
},
78+
],
79+
(taskId) => {
80+
capturedId = taskId;
81+
},
82+
);
83+
expect(capturedId).toBeGreaterThan(0);
84+
const cancelled = pool.cancelTask(capturedId);
85+
expect(cancelled).toBe(true);
86+
87+
// Release the worker — kickQueue should see the cancelled
88+
// marker and resolve without postMessage.
89+
ws[0].ready = true;
90+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
91+
(pool as any).kickQueue();
92+
93+
const results = await batchP;
94+
expect(results).toHaveLength(1);
95+
expect(results[0].cancelled).toBe(true);
96+
expect(results[0].diagnostics).toEqual([]);
97+
// Task was never dispatched, so no parseError sentinel either.
98+
expect(results[0].parseError).toBeUndefined();
99+
} finally {
100+
await pool.shutdown();
101+
}
102+
}, 15_000);
103+
104+
test('shutdown drain honors q.cancelled — cancelled-then-shutdown reports cancelled:true, not parseError:shutdown', async () => {
105+
// Regression for review P2 #15. Pre-fix, the drain loop in
106+
// `shutdown` blindly stamped every queued task as
107+
// `cancelled: false, parseError: 'shutdown'` regardless of whether
108+
// the user had already called `cancelTask(taskId)` on it. The
109+
// `q.cancelled` flag (set by cancelTask for queued entries) was
110+
// ignored, so a user-initiated cancel that hadn't been picked up
111+
// by `kickQueue` yet would be mis-labelled as a shutdown failure
112+
// — a meaningful distinction for LSP result categorisation and
113+
// strict-runner error counters.
114+
const pool = new WorkerPool({
115+
configs: localConfigs,
116+
workerCount: 1,
117+
});
118+
await pool.init();
119+
// Same wedge as R1: hold the worker non-ready so kickQueue can't
120+
// dispatch, leaving every task in pendingQueue.
56121
// eslint-disable-next-line @typescript-eslint/no-explicit-any
57122
const ws = (pool as any).workers as Array<{ ready: boolean }>;
58123
ws[0].ready = false;
59124

60-
let capturedId = -1;
125+
// Capture taskIds as they get assigned so we can cancel a
126+
// specific queued one.
127+
const taskIds: number[] = [];
61128
const batchP = pool.lintBatch(
62-
[
63-
{
64-
filePath: 'q.ts',
65-
text: 'const x = null;\n',
66-
rules: { 'local/no-null': { options: [] } },
67-
collectFixes: false,
68-
suggestionsMode: 'off',
69-
configKey: LOCAL_CONFIG_DIR,
70-
},
71-
],
129+
[1, 2, 3].map((i) => ({
130+
filePath: `q${i}.ts`,
131+
text: 'const x = null;\n',
132+
rules: { 'local/no-null': { options: [] } },
133+
collectFixes: false,
134+
suggestionsMode: 'off',
135+
configKey: LOCAL_CONFIG_DIR,
136+
})),
72137
(taskId) => {
73-
capturedId = taskId;
138+
taskIds.push(taskId);
74139
},
75140
);
76-
expect(capturedId).toBeGreaterThan(0);
77-
const cancelled = pool.cancelTask(capturedId);
78-
expect(cancelled).toBe(true);
79141

80-
// Release the worker — kickQueue should see the cancelled
81-
// marker and resolve without postMessage.
82-
ws[0].ready = true;
83-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
84-
(pool as any).kickQueue();
142+
// Let enqueue finish so onTaskDispatched fires for all 3.
143+
await new Promise((r) => setTimeout(r, 30));
144+
expect(taskIds).toHaveLength(3);
145+
146+
// Cancel the MIDDLE queued task — it must surface as
147+
// `cancelled: true` from the shutdown drain, while the
148+
// first/third stay as `parseError: 'shutdown'`.
149+
pool.cancelTask(taskIds[1]);
85150

86-
const results = await batchP;
87-
expect(results).toHaveLength(1);
88-
expect(results[0].cancelled).toBe(true);
89-
expect(results[0].diagnostics).toEqual([]);
90-
// Task was never dispatched, so no parseError sentinel either.
91-
expect(results[0].parseError).toBeUndefined();
92-
} finally {
93151
await pool.shutdown();
94-
}
95-
}, 15_000);
96-
97-
test('shutdown drain honors q.cancelled — cancelled-then-shutdown reports cancelled:true, not parseError:shutdown', async () => {
98-
// Regression for review P2 #15. Pre-fix, the drain loop in
99-
// `shutdown` blindly stamped every queued task as
100-
// `cancelled: false, parseError: 'shutdown'` regardless of whether
101-
// the user had already called `cancelTask(taskId)` on it. The
102-
// `q.cancelled` flag (set by cancelTask for queued entries) was
103-
// ignored, so a user-initiated cancel that hadn't been picked up
104-
// by `kickQueue` yet would be mis-labelled as a shutdown failure
105-
// — a meaningful distinction for LSP result categorisation and
106-
// strict-runner error counters.
107-
const pool = new WorkerPool({
108-
configs: localConfigs,
109-
workerCount: 1,
110-
});
111-
await pool.init();
112-
// Same wedge as R1: hold the worker non-ready so kickQueue can't
113-
// dispatch, leaving every task in pendingQueue.
114-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
115-
const ws = (pool as any).workers as Array<{ ready: boolean }>;
116-
ws[0].ready = false;
117-
118-
// Capture taskIds as they get assigned so we can cancel a
119-
// specific queued one.
120-
const taskIds: number[] = [];
121-
const batchP = pool.lintBatch(
122-
[1, 2, 3].map((i) => ({
123-
filePath: `q${i}.ts`,
124-
text: 'const x = null;\n',
125-
rules: { 'local/no-null': { options: [] } },
126-
collectFixes: false,
127-
suggestionsMode: 'off',
128-
configKey: LOCAL_CONFIG_DIR,
129-
})),
130-
(taskId) => {
131-
taskIds.push(taskId);
132-
},
133-
);
134-
135-
// Let enqueue finish so onTaskDispatched fires for all 3.
136-
await new Promise((r) => setTimeout(r, 30));
137-
expect(taskIds).toHaveLength(3);
138-
139-
// Cancel the MIDDLE queued task — it must surface as
140-
// `cancelled: true` from the shutdown drain, while the
141-
// first/third stay as `parseError: 'shutdown'`.
142-
pool.cancelTask(taskIds[1]);
143-
144-
await pool.shutdown();
145-
const result = await batchP;
146-
expect(result).toHaveLength(3);
147-
148-
expect(result[0].parseError).toBe('shutdown');
149-
expect(result[0].cancelled).toBe(false);
150-
151-
expect(result[1].cancelled).toBe(true);
152-
expect(result[1].parseError).toBeUndefined();
153-
154-
expect(result[2].parseError).toBe('shutdown');
155-
expect(result[2].cancelled).toBe(false);
156-
}, 15_000);
157-
});
152+
const result = await batchP;
153+
expect(result).toHaveLength(3);
154+
155+
expect(result[0].parseError).toBe('shutdown');
156+
expect(result[0].cancelled).toBe(false);
157+
158+
expect(result[1].cancelled).toBe(true);
159+
expect(result[1].parseError).toBeUndefined();
160+
161+
expect(result[2].parseError).toBe('shutdown');
162+
expect(result[2].cancelled).toBe(false);
163+
}, 15_000);
164+
},
165+
);

0 commit comments

Comments
 (0)