Skip to content

Commit 6e78204

Browse files
committed
fix: address Copilot review comments (round 3)
- Rewrite timing-dependent AsyncSerializer tests to use deterministic deferred promise barriers and inFlight counters instead of setTimeout/Date.now, eliminating flakiness under CI load - Fix reloadMkdocsConfig to read vscode.workspace.workspaceFolders live inside the callback rather than using the stale captured variable, so multi-root workspaces and folder changes are handled correctly
1 parent 025f004 commit 6e78204

2 files changed

Lines changed: 38 additions & 35 deletions

File tree

src/extension.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,9 @@ export async function activate(context: vscode.ExtensionContext) {
5151
// Reload config and refresh diagnostics when mkdocs config changes
5252
const reloadMkdocsConfig = async () => {
5353
await configReloadSerializer.execute(async () => {
54-
if (workspaceFolders && workspaceFolders.length > 0) {
55-
await diagnosticManager.loadMkdocsConfig(workspaceFolders[0].uri.fsPath);
54+
const currentFolders = vscode.workspace.workspaceFolders;
55+
if (currentFolders && currentFolders.length > 0) {
56+
await diagnosticManager.loadMkdocsConfig(currentFolders[0].uri.fsPath);
5657

5758
// Refresh diagnostics for all open markdown files
5859
vscode.window.visibleTextEditors

src/test/unit/asyncSerializer.test.ts

Lines changed: 35 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -11,53 +11,47 @@ import { AsyncSerializer } from '../../asyncSerializer';
1111
describe('AsyncSerializer', () => {
1212
it('should prevent concurrent executions', async () => {
1313
const serializer = new AsyncSerializer();
14-
const executions: Array<{ id: number; start: number; end: number }> = [];
15-
let counter = 0;
14+
let inFlight = 0;
15+
let maxInFlight = 0;
1616

17-
const slowOperation = async () => {
18-
const id = counter++;
19-
const startTime = Date.now();
20-
executions.push({ id, start: startTime, end: -1 });
17+
// A manually controlled barrier holds the operation in-flight until released,
18+
// making this test deterministic and free of real-time delays.
19+
let release!: () => void;
20+
const barrier = new Promise<void>(resolve => { release = resolve; });
2121

22-
// Simulate async operation with artificial delay
23-
await new Promise(resolve => setTimeout(resolve, 50));
24-
25-
const endTime = Date.now();
26-
const execution = executions.find(e => e.id === id);
27-
if (execution) {
28-
execution.end = endTime;
29-
}
22+
const operation = async () => {
23+
inFlight++;
24+
if (inFlight > maxInFlight) { maxInFlight = inFlight; }
25+
await barrier;
26+
inFlight--;
3027
};
3128

32-
// Fire three operations rapidly
29+
// Fire three operations; first executes immediately, the rest are coalesced
3330
const promises = [
34-
serializer.execute(slowOperation),
35-
serializer.execute(slowOperation),
36-
serializer.execute(slowOperation),
31+
serializer.execute(operation),
32+
serializer.execute(operation),
33+
serializer.execute(operation),
3734
];
3835

36+
// Release the barrier so all executions can complete
37+
release();
3938
await Promise.all(promises);
4039

41-
// Verify no overlaps occurred
42-
// Sort by start time to ensure we check in correct order
43-
const sorted = executions.sort((a, b) => a.start - b.start);
44-
45-
for (let i = 0; i < sorted.length - 1; i++) {
46-
assert.ok(
47-
sorted[i].end <= sorted[i + 1].start,
48-
`Execution ${sorted[i].id} (ended at ${sorted[i].end}) overlapped with ` +
49-
`execution ${sorted[i + 1].id} (started at ${sorted[i + 1].start})`
50-
);
51-
}
40+
assert.strictEqual(maxInFlight, 1, 'At most one execution should be in flight at a time');
5241
});
5342

5443
it('should coalesce multiple pending calls into single execution', async () => {
5544
const serializer = new AsyncSerializer();
5645
let executionCount = 0;
5746

47+
// A manually controlled barrier holds the first execution in-flight while
48+
// the remaining calls are dispatched synchronously, ensuring coalescing occurs.
49+
let release!: () => void;
50+
const barrier = new Promise<void>(resolve => { release = resolve; });
51+
5852
const countingOperation = async () => {
5953
executionCount++;
60-
await new Promise(resolve => setTimeout(resolve, 50));
54+
await barrier;
6155
};
6256

6357
// Fire five operations rapidly while first is running
@@ -69,6 +63,7 @@ describe('AsyncSerializer', () => {
6963
serializer.execute(countingOperation),
7064
];
7165

66+
release();
7267
await Promise.all(promises);
7368

7469
// With coalescing of synchronously scheduled calls, this should be deterministic:
@@ -120,19 +115,26 @@ describe('AsyncSerializer', () => {
120115
const serializer = new AsyncSerializer();
121116
let executionCount = 0;
122117

118+
// A manually controlled barrier holds the first execution in-flight so calls
119+
// B and C are coalesced before A finishes (and throws).
120+
let release!: () => void;
121+
const barrier = new Promise<void>(resolve => { release = resolve; });
122+
123123
const operation = async () => {
124124
const id = ++executionCount;
125-
await new Promise(resolve => setTimeout(resolve, 50));
125+
await barrier;
126126
if (id === 1) {
127127
throw new Error('First execution failed');
128128
}
129129
};
130130

131-
// Start call A (will error after delay), then dispatch B and C which get coalesced
131+
// Start call A (will error after barrier), then dispatch B and C which get coalesced
132132
const promiseA = serializer.execute(operation);
133133
const promiseB = serializer.execute(operation);
134134
const promiseC = serializer.execute(operation);
135135

136+
release();
137+
136138
// A rejects; B and C already resolved immediately (coalesced)
137139
await promiseA.catch(() => {});
138140
await promiseB;
@@ -152,7 +154,7 @@ describe('AsyncSerializer', () => {
152154

153155
const operation = async (id: number) => {
154156
executions.push(id);
155-
await new Promise(resolve => setTimeout(resolve, 10));
157+
await Promise.resolve();
156158
};
157159

158160
// First batch

0 commit comments

Comments
 (0)