mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-17 23:52:36 +00:00
* fix(lbug): wait for read stream close in splitRelCsvByLabelPair (Windows ENOTEMPTY)
The windows-latest CI job intermittently failed:
FAIL test/unit/rel-csv-split.test.ts > splitRelCsvByLabelPair > handles empty CSV (header only) without errors
Error: ENOTEMPTY: directory not empty, rmdir 'C:\Users\RUNNER~1\AppData\Local\Temp\rel-csv-test-XW5KOu'
Cause: splitRelCsvByLabelPair resolved its Promise on readline's 'close'
event, but the underlying fs.ReadStream's file descriptor is released
asynchronously after that — especially on Windows. For the empty-CSV
test the function returns so quickly that afterEach fires rmSync while
the relations.csv fd is still held, so Windows reports ENOTEMPTY on
the directory.
Fixes:
- Production: after readline 'close', wait for inputStream 'close' (or
resolve immediately if already closed/destroyed). Call inputStream
.destroy() defensively so we never hang if the fd never emits 'close'.
- Test: afterEach now retries rmSync up to 5 times on ENOTEMPTY/EBUSY/
EPERM with a brief back-off — defense-in-depth so the test doesn't
flake on slow CI runners independent of the production change.
The production fix benefits every caller, not just the test: any code
that deletes the CSV's parent directory right after the Promise
resolves previously hit the same race on Windows.
* refactor(lbug): replace custom stream state machines with stdlib primitives
Full audit of splitRelCsvByLabelPair's stream usage after the original
ENOTEMPTY fix. Replaced three hand-rolled mechanisms with their
standard-library equivalents — 147 -> 71 lines in the function, and
the caller's WriteStream closure dropped from 13 lines to 5.
- readline: 'on(line)' + pause/resume/waitingForDrain state machine
-> 'for await (const line of rl)'. Async-iterator delivery naturally
serializes line processing with our awaits, so at most one ws is in
backpressure at a time. We just 'await once(ws, "drain")' when
'write()' returns false — the custom Set, the settled flag and the
'only resume when all streams have drained' logic all go away.
- Multi-stream error coordination: hand-rolled cleanup() that had to
be entered exactly once and had to destroy the inputStream and every
pair ws -> single AbortController shared across every 'once(ws,
'drain', { signal })'. Any stream error aborts every pending wait.
- 'stream/promises.finished(inputStream)' in the 'finally' block
replaces the manual 'rl.on('close', () => inputStream.once('close',
...))' dance, and covers both the success and error paths with the
same primitive. This closes the Windows ENOTEMPTY race root cause —
we never return while the fd might still be in flight.
- Caller closure: 'new Promise((res, rej) => ws.end(cb) + remove
listener on error)' -> 'ws.end(); await finished(ws)'.
- Test 'afterEach': custom retry loop -> 'fs.rmSync(..., { maxRetries:
5, retryDelay: 50 })' (Node added these options specifically for
cross-platform tmpdir cleanup).
- Test 'destroys all streams when one errors': old code leaked
backpressure and created multiple pair streams before the first
blocked; new strict serial backpressure doesn't, so the test now
unblocks the first stream once to advance the loop and create the
second stream before triggering the error.
283 lines
8.3 KiB
TypeScript
283 lines
8.3 KiB
TypeScript
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
|
import { EventEmitter } from 'events';
|
|
import fs from 'fs';
|
|
import path from 'path';
|
|
import os from 'os';
|
|
import { splitRelCsvByLabelPair } from '../../src/core/lbug/lbug-adapter.js';
|
|
|
|
/**
|
|
* Regression tests for splitRelCsvByLabelPair (PR #818).
|
|
*
|
|
* These tests call the real exported function from lbug-adapter.ts with a
|
|
* mock WriteStream factory, exercising the actual backpressure, error
|
|
* handling, and drain-listener guard without touching LadybugDB.
|
|
*/
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// Mock WriteStream — controllable backpressure + error injection
|
|
// ---------------------------------------------------------------------------
|
|
class MockWriteStream extends EventEmitter {
|
|
public chunks: string[] = [];
|
|
public destroyed = false;
|
|
public ended = false;
|
|
public blocked = false;
|
|
public maxDrainListenersSeen = 0;
|
|
|
|
write(chunk: string): boolean {
|
|
this.chunks.push(chunk);
|
|
this._trackDrainListeners();
|
|
return !this.blocked;
|
|
}
|
|
|
|
end(cb?: (err?: Error) => void): this {
|
|
this.ended = true;
|
|
if (cb) cb();
|
|
return this;
|
|
}
|
|
|
|
destroy(): this {
|
|
this.destroyed = true;
|
|
return this;
|
|
}
|
|
|
|
unblock(): void {
|
|
this.blocked = false;
|
|
this.emit('drain');
|
|
}
|
|
|
|
triggerError(err: Error): void {
|
|
this.emit('error', err);
|
|
}
|
|
|
|
private _trackDrainListeners(): void {
|
|
const count = this.listenerCount('drain');
|
|
if (count > this.maxDrainListenersSeen) {
|
|
this.maxDrainListenersSeen = count;
|
|
}
|
|
}
|
|
}
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// Test helpers
|
|
// ---------------------------------------------------------------------------
|
|
const HEADER = '"from","to","type","confidence","reason","step"';
|
|
|
|
function csvLine(from: string, to: string, type = 'CALLS'): string {
|
|
return `"${from}","${to}","${type}",1.0,"auto",0`;
|
|
}
|
|
|
|
function getNodeLabel(id: string): string {
|
|
return id.split(':')[0];
|
|
}
|
|
|
|
/** Cast MockWriteStream factory to the real WriteStreamFactory type. */
|
|
function mockFactory(streams: MockWriteStream[], opts?: { blocked?: boolean }) {
|
|
return (() => {
|
|
const ws = new MockWriteStream();
|
|
if (opts?.blocked) ws.blocked = true;
|
|
streams.push(ws);
|
|
return ws;
|
|
}) as unknown as (filePath: string) => import('fs').WriteStream;
|
|
}
|
|
|
|
let tmpDir: string;
|
|
|
|
beforeEach(() => {
|
|
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'rel-csv-test-'));
|
|
});
|
|
|
|
afterEach(() => {
|
|
// fs.rmSync's built-in retry loop handles Windows EBUSY/ENOTEMPTY/EPERM
|
|
// when a just-closed fd hasn't been released yet (Node added this exactly
|
|
// for cross-platform tmpdir cleanup — see Node.js fs docs). The production
|
|
// function also waits for the input stream's 'close' event, so this is
|
|
// defense-in-depth.
|
|
fs.rmSync(tmpDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 });
|
|
});
|
|
|
|
function writeCsv(lines: string[]): string {
|
|
const csvPath = path.join(tmpDir, 'relations.csv');
|
|
fs.writeFileSync(csvPath, lines.join('\n') + '\n');
|
|
return csvPath;
|
|
}
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// Tests
|
|
// ---------------------------------------------------------------------------
|
|
describe('splitRelCsvByLabelPair', () => {
|
|
const validTables = new Set(['Function', 'Class', 'File', 'Method']);
|
|
|
|
it('splits lines into per-pair files with correct row counts', async () => {
|
|
const csvPath = writeCsv([
|
|
HEADER,
|
|
csvLine('Function:a', 'Class:b'),
|
|
csvLine('Function:c', 'Class:d'),
|
|
csvLine('File:e', 'Method:f'),
|
|
]);
|
|
|
|
const streams: MockWriteStream[] = [];
|
|
const result = await splitRelCsvByLabelPair(
|
|
csvPath,
|
|
tmpDir,
|
|
validTables,
|
|
getNodeLabel,
|
|
mockFactory(streams),
|
|
);
|
|
|
|
expect(result.totalValidRels).toBe(3);
|
|
expect(result.relsByPairMeta.get('Function|Class')?.rows).toBe(2);
|
|
expect(result.relsByPairMeta.get('File|Method')?.rows).toBe(1);
|
|
});
|
|
|
|
it('captures the CSV header in relHeader', async () => {
|
|
const csvPath = writeCsv([HEADER, csvLine('Function:a', 'Class:b')]);
|
|
|
|
const streams: MockWriteStream[] = [];
|
|
const result = await splitRelCsvByLabelPair(
|
|
csvPath,
|
|
tmpDir,
|
|
validTables,
|
|
getNodeLabel,
|
|
mockFactory(streams),
|
|
);
|
|
|
|
expect(result.relHeader).toBe(HEADER);
|
|
});
|
|
|
|
it('skips lines with unknown labels and counts them', async () => {
|
|
const csvPath = writeCsv([
|
|
HEADER,
|
|
csvLine('Function:a', 'Class:b'),
|
|
csvLine('Unknown:x', 'Class:y'),
|
|
csvLine('Function:c', 'Bogus:d'),
|
|
]);
|
|
|
|
const streams: MockWriteStream[] = [];
|
|
const result = await splitRelCsvByLabelPair(
|
|
csvPath,
|
|
tmpDir,
|
|
validTables,
|
|
getNodeLabel,
|
|
mockFactory(streams),
|
|
);
|
|
|
|
expect(result.totalValidRels).toBe(1);
|
|
expect(result.skippedRels).toBe(2);
|
|
});
|
|
|
|
it('ignores blank lines without counting them as skipped', async () => {
|
|
const csvPath = writeCsv([HEADER, '', csvLine('Function:a', 'Class:b'), '', '']);
|
|
|
|
const streams: MockWriteStream[] = [];
|
|
const result = await splitRelCsvByLabelPair(
|
|
csvPath,
|
|
tmpDir,
|
|
validTables,
|
|
getNodeLabel,
|
|
mockFactory(streams),
|
|
);
|
|
|
|
expect(result.totalValidRels).toBe(1);
|
|
expect(result.skippedRels).toBe(0);
|
|
});
|
|
|
|
it('registers at most 1 drain listener per stream under heavy backpressure', async () => {
|
|
const lines = [HEADER];
|
|
for (let i = 0; i < 50; i++) {
|
|
lines.push(csvLine(`Function:f${i}`, `Class:c${i}`));
|
|
}
|
|
const csvPath = writeCsv(lines);
|
|
|
|
const streams: MockWriteStream[] = [];
|
|
const promise = splitRelCsvByLabelPair(
|
|
csvPath,
|
|
tmpDir,
|
|
validTables,
|
|
getNodeLabel,
|
|
mockFactory(streams, { blocked: true }),
|
|
);
|
|
|
|
// Give readline time to buffer and fire lines
|
|
await new Promise((r) => setTimeout(r, 50));
|
|
|
|
// Unblock all streams so the Promise can resolve
|
|
for (const ws of streams) ws.unblock();
|
|
await promise;
|
|
|
|
// The guard should have kept drain listeners at 1
|
|
for (const ws of streams) {
|
|
expect(ws.maxDrainListenersSeen).toBeLessThanOrEqual(1);
|
|
}
|
|
});
|
|
|
|
it('rejects the Promise when a WriteStream emits an error', async () => {
|
|
const csvPath = writeCsv([HEADER, csvLine('Function:a', 'Class:b')]);
|
|
|
|
const streams: MockWriteStream[] = [];
|
|
const promise = splitRelCsvByLabelPair(
|
|
csvPath,
|
|
tmpDir,
|
|
validTables,
|
|
getNodeLabel,
|
|
mockFactory(streams, { blocked: true }),
|
|
);
|
|
|
|
// Wait for readline to process, then error while paused on drain
|
|
await new Promise((r) => setTimeout(r, 50));
|
|
expect(streams.length).toBeGreaterThan(0);
|
|
streams[0].triggerError(new Error('disk full'));
|
|
|
|
await expect(promise).rejects.toThrow('disk full');
|
|
});
|
|
|
|
it('destroys all streams when one errors (no lingering FDs)', async () => {
|
|
const lines = [HEADER];
|
|
for (let i = 0; i < 10; i++) {
|
|
lines.push(csvLine(`Function:f${i}`, `Class:c${i}`));
|
|
lines.push(csvLine(`File:e${i}`, `Method:m${i}`));
|
|
}
|
|
const csvPath = writeCsv(lines);
|
|
|
|
const streams: MockWriteStream[] = [];
|
|
const promise = splitRelCsvByLabelPair(
|
|
csvPath,
|
|
tmpDir,
|
|
validTables,
|
|
getNodeLabel,
|
|
mockFactory(streams, { blocked: true }),
|
|
);
|
|
|
|
// The first pair stream is created immediately and blocks on its header
|
|
// write. Unblock it once so the loop advances and creates the second
|
|
// pair stream (also blocked). Now both streams exist — trigger the error.
|
|
await new Promise((r) => setTimeout(r, 20));
|
|
expect(streams.length).toBe(1);
|
|
streams[0].unblock();
|
|
await new Promise((r) => setTimeout(r, 20));
|
|
expect(streams.length).toBeGreaterThanOrEqual(2);
|
|
streams[0].triggerError(new Error('EMFILE'));
|
|
|
|
await expect(promise).rejects.toThrow('EMFILE');
|
|
|
|
for (const ws of streams) {
|
|
expect(ws.destroyed).toBe(true);
|
|
}
|
|
});
|
|
|
|
it('handles empty CSV (header only) without errors', async () => {
|
|
const csvPath = writeCsv([HEADER]);
|
|
|
|
const streams: MockWriteStream[] = [];
|
|
const result = await splitRelCsvByLabelPair(
|
|
csvPath,
|
|
tmpDir,
|
|
validTables,
|
|
getNodeLabel,
|
|
mockFactory(streams),
|
|
);
|
|
|
|
expect(result.totalValidRels).toBe(0);
|
|
expect(result.skippedRels).toBe(0);
|
|
expect(result.relHeader).toBe(HEADER);
|
|
});
|
|
});
|