fix(FN-000): stabilize failing tests and trim slow waits
This commit is contained in:
@@ -390,16 +390,14 @@ describe("PluginLoader Hot-Reload", () => {
|
||||
});
|
||||
|
||||
it("should remove plugin on total failure (reload + rollback both fail)", async () => {
|
||||
await pluginLoader.loadPlugin("hot-reload-test");
|
||||
|
||||
// Create plugin with hanging onLoad
|
||||
const manifest = makeManifest({ id: "hot-reload-test", name: "Hot Reload Test", version: "1.0.0" });
|
||||
await writePluginModule(tmpDir, "plugin.js", manifest, {
|
||||
onUnload: `async () => { throw new Error("unload error"); }`,
|
||||
onLoad: `async () => { throw new Error("load error"); }`,
|
||||
onLoad: `((() => { let count = 0; return async () => { count += 1; if (count > 1) throw new Error("rollback load error"); }; })())`,
|
||||
});
|
||||
|
||||
// Modify for reload
|
||||
await pluginLoader.loadPlugin("hot-reload-test");
|
||||
|
||||
// Modify for reload with a new failing implementation.
|
||||
const newManifest = makeManifest({ id: "hot-reload-test", name: "Hot Reload Test", version: "2.0.0" });
|
||||
await writePluginModule(tmpDir, "plugin.js", newManifest, {
|
||||
onLoad: `async () => { throw new Error("new load error"); }`,
|
||||
|
||||
@@ -9,7 +9,9 @@
|
||||
* - Error isolation (plugin crashes don't crash the loader)
|
||||
*/
|
||||
|
||||
import { isAbsolute, resolve } from "node:path";
|
||||
import { copyFile, unlink } from "node:fs/promises";
|
||||
import { isAbsolute, parse, resolve } from "node:path";
|
||||
import { pathToFileURL } from "node:url";
|
||||
import { EventEmitter } from "node:events";
|
||||
import type { TaskStore } from "./store.js";
|
||||
import { PluginStore } from "./plugin-store.js";
|
||||
@@ -85,6 +87,9 @@ export class PluginLoader extends EventEmitter<{
|
||||
/** Cache of dynamically imported modules */
|
||||
private loadedModules: Map<string, unknown> = new Map();
|
||||
|
||||
/** Monotonic counter for deterministic cache-busting import URLs */
|
||||
private importNonce = 0;
|
||||
|
||||
constructor(private options: PluginLoaderOptions) {
|
||||
super();
|
||||
}
|
||||
@@ -266,19 +271,39 @@ export class PluginLoader extends EventEmitter<{
|
||||
return this.loadedModules.get(path)!;
|
||||
}
|
||||
|
||||
// Dynamic import - use cache-busting for reload scenarios
|
||||
let mod: unknown;
|
||||
let importPath = path;
|
||||
let tempPath: string | null = null;
|
||||
|
||||
if (bypassCache) {
|
||||
// Use a query parameter for cache differentiation.
|
||||
// Vite/Vitest's module resolver treats hash fragments as part of the
|
||||
// filesystem path in some environments (causing ERR_MODULE_NOT_FOUND).
|
||||
const bustedPath = `${path}?t=${Date.now()}`;
|
||||
mod = await import(bustedPath);
|
||||
} else {
|
||||
mod = await import(path);
|
||||
const parsed = parse(path);
|
||||
tempPath = resolve(
|
||||
parsed.dir,
|
||||
`${parsed.name}.fusion-import-${process.pid}-${++this.importNonce}${parsed.ext || ".js"}`,
|
||||
);
|
||||
await copyFile(path, tempPath);
|
||||
importPath = tempPath;
|
||||
}
|
||||
|
||||
const fileUrl = pathToFileURL(importPath);
|
||||
|
||||
// Dynamic import - use a unique search param for reload scenarios.
|
||||
// Using file: URLs avoids Vite/Vitest resolver edge cases with bare
|
||||
// absolute filesystem paths plus query params.
|
||||
if (bypassCache) {
|
||||
fileUrl.searchParams.set("t", `${Date.now()}-${this.importNonce}`);
|
||||
}
|
||||
|
||||
try {
|
||||
const mod = await import(fileUrl.href);
|
||||
this.loadedModules.set(path, mod);
|
||||
return mod;
|
||||
} finally {
|
||||
if (tempPath) {
|
||||
void unlink(tempPath).catch(() => {
|
||||
// Best-effort cleanup; a stale temp import file is non-fatal.
|
||||
});
|
||||
}
|
||||
}
|
||||
this.loadedModules.set(path, mod);
|
||||
return mod;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -389,7 +414,6 @@ export class PluginLoader extends EventEmitter<{
|
||||
await this.options.pluginStore.updatePluginState(pluginId, "started");
|
||||
|
||||
log.warn(`Rollback successful for ${pluginId}`);
|
||||
throw err; // Still throw the original error
|
||||
} catch (rollbackErr) {
|
||||
// Rollback also failed - remove plugin and set error state
|
||||
log.error(
|
||||
@@ -416,6 +440,8 @@ export class PluginLoader extends EventEmitter<{
|
||||
|
||||
throw err; // Throw original error
|
||||
}
|
||||
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -589,7 +615,11 @@ export class PluginLoader extends EventEmitter<{
|
||||
|
||||
try {
|
||||
// Call onUnload hook
|
||||
await this.safeCallHook(plugin, "onUnload", []);
|
||||
await this.withTimeout(
|
||||
this.safeCallHook(plugin, "onUnload", []),
|
||||
5000,
|
||||
`onUnload timeout for ${pluginId}`,
|
||||
);
|
||||
} catch (err) {
|
||||
log.error(`Error in onUnload for ${pluginId}:`, err);
|
||||
}
|
||||
|
||||
@@ -8049,9 +8049,10 @@ Task with acceptance criteria
|
||||
expect(mockOnSummarize).toHaveBeenCalledWith(longDescription);
|
||||
|
||||
// 3. Wait for async summarization and verify title was set
|
||||
await new Promise((resolve) => setTimeout(resolve, 10));
|
||||
const updatedTask = await store.getTask(task.id);
|
||||
expect(updatedTask.title).toBe("AI Title From Saturation Test");
|
||||
await vi.waitFor(async () => {
|
||||
const updatedTask = await store.getTask(task.id);
|
||||
expect(updatedTask.title).toBe("AI Title From Saturation Test");
|
||||
});
|
||||
|
||||
// Reset maxConcurrent to normal value
|
||||
await store.updateSettings({ maxConcurrent: 2 });
|
||||
@@ -8061,11 +8062,9 @@ Task with acceptance criteria
|
||||
// Simulate a slow/stalled onSummarize callback to prove there's no
|
||||
// semaphore that would block task creation. The core store has no
|
||||
// dependency on any concurrency limiter.
|
||||
const slowOnSummarize = vi.fn().mockImplementation(async () => {
|
||||
// Simulate a very slow AI response
|
||||
await new Promise((resolve) => setTimeout(resolve, 1000));
|
||||
return "Slow AI Title";
|
||||
});
|
||||
const slowOnSummarize = vi.fn().mockImplementation(
|
||||
async () => new Promise<string>(() => {}),
|
||||
);
|
||||
|
||||
const taskPromise = store.createTask(
|
||||
{ description: "a".repeat(201) },
|
||||
@@ -8281,9 +8280,6 @@ Task with acceptance criteria
|
||||
// Create a task to trigger the poll cycle
|
||||
await store.createTask({ description: "fast poll test" });
|
||||
|
||||
// Wait for poll interval
|
||||
await new Promise((resolve) => setTimeout(resolve, 1100));
|
||||
|
||||
// Manually call checkForChanges - should be fast
|
||||
await storeAny.checkForChanges();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user