feat(FN-5305): fix async synchronization in github-tracking-delete and alig
Two test-only commits for FN-5305 improve timing reliability and assertion accuracy in the delete route and GitHub-tracking-delete test suites by replacing unreliable one-tick flushes with explicit async synchronization and aligning expectations with audit context behavior. Fusion-Task-Id: FN-5305
This commit is contained in:
committed by
gsxdsm
parent
cfc70b21bd
commit
46fb3f0ef7
@@ -6,6 +6,12 @@ import { tmpdir } from "node:os";
|
|||||||
import { TaskStore } from "@fusion/core";
|
import { TaskStore } from "@fusion/core";
|
||||||
import { GitHubTrackingStateService } from "../github-tracking-state.js";
|
import { GitHubTrackingStateService } from "../github-tracking-state.js";
|
||||||
|
|
||||||
|
type GitHubIssueActionPayload = Record<string, unknown>;
|
||||||
|
type StoreEventApi = {
|
||||||
|
on: (event: string, listener: (payload: GitHubIssueActionPayload) => void) => void;
|
||||||
|
off: (event: string, listener: (payload: GitHubIssueActionPayload) => void) => void;
|
||||||
|
};
|
||||||
|
|
||||||
const { mockSetIssueState, mockGetIssue, mockResolveGithubTrackingAuth } = vi.hoisted(() => ({
|
const { mockSetIssueState, mockGetIssue, mockResolveGithubTrackingAuth } = vi.hoisted(() => ({
|
||||||
mockSetIssueState: vi.fn(),
|
mockSetIssueState: vi.fn(),
|
||||||
mockGetIssue: vi.fn(),
|
mockGetIssue: vi.fn(),
|
||||||
@@ -27,8 +33,44 @@ function makeTmpDir(): string {
|
|||||||
return mkdtempSync(join(tmpdir(), "kb-dashboard-github-tracking-delete-test-"));
|
return mkdtempSync(join(tmpdir(), "kb-dashboard-github-tracking-delete-test-"));
|
||||||
}
|
}
|
||||||
|
|
||||||
async function flushAsync(): Promise<void> {
|
function waitForGithubIssueAction(
|
||||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
store: TaskStore,
|
||||||
|
predicate: (payload: GitHubIssueActionPayload) => boolean,
|
||||||
|
{ timeoutMs = 2_000, timeoutMessage = "Timed out waiting for github-issue:action event" } = {},
|
||||||
|
): Promise<GitHubIssueActionPayload> {
|
||||||
|
const eventStore = store as unknown as StoreEventApi;
|
||||||
|
|
||||||
|
return new Promise((resolve, reject) => {
|
||||||
|
const onAction = (payload: GitHubIssueActionPayload) => {
|
||||||
|
if (!predicate(payload)) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
clearTimeout(timeoutId);
|
||||||
|
eventStore.off("github-issue:action", onAction);
|
||||||
|
resolve(payload);
|
||||||
|
};
|
||||||
|
|
||||||
|
const timeoutId = setTimeout(() => {
|
||||||
|
eventStore.off("github-issue:action", onAction);
|
||||||
|
reject(new Error(timeoutMessage));
|
||||||
|
}, timeoutMs);
|
||||||
|
|
||||||
|
eventStore.on("github-issue:action", onAction);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
async function expectNoGithubIssueAction(
|
||||||
|
store: TaskStore,
|
||||||
|
predicate: (payload: GitHubIssueActionPayload) => boolean,
|
||||||
|
timeoutMessage: string,
|
||||||
|
): Promise<void> {
|
||||||
|
await expect(
|
||||||
|
waitForGithubIssueAction(store, predicate, {
|
||||||
|
timeoutMs: 150,
|
||||||
|
timeoutMessage,
|
||||||
|
}),
|
||||||
|
).rejects.toThrow(timeoutMessage);
|
||||||
}
|
}
|
||||||
|
|
||||||
describe("github tracking delete flow", () => {
|
describe("github tracking delete flow", () => {
|
||||||
@@ -51,7 +93,6 @@ describe("github tracking delete flow", () => {
|
|||||||
|
|
||||||
afterEach(async () => {
|
afterEach(async () => {
|
||||||
stateService.stop();
|
stateService.stop();
|
||||||
await flushAsync();
|
|
||||||
store.close();
|
store.close();
|
||||||
await rm(rootDir, { recursive: true, force: true });
|
await rm(rootDir, { recursive: true, force: true });
|
||||||
await rm(globalDir, { recursive: true, force: true });
|
await rm(globalDir, { recursive: true, force: true });
|
||||||
@@ -71,8 +112,14 @@ describe("github tracking delete flow", () => {
|
|||||||
createdAt: new Date().toISOString(),
|
createdAt: new Date().toISOString(),
|
||||||
});
|
});
|
||||||
|
|
||||||
|
const closeAction = waitForGithubIssueAction(
|
||||||
|
store,
|
||||||
|
(payload) => payload.taskId === task.id && payload.action === "close" && payload.outcome === "success",
|
||||||
|
{ timeoutMessage: `Timed out waiting for close action for deleted task ${task.id}` },
|
||||||
|
);
|
||||||
|
|
||||||
await store.deleteTask(task.id);
|
await store.deleteTask(task.id);
|
||||||
await flushAsync();
|
await closeAction;
|
||||||
|
|
||||||
expect(mockSetIssueState).toHaveBeenCalledTimes(1);
|
expect(mockSetIssueState).toHaveBeenCalledTimes(1);
|
||||||
expect(mockSetIssueState).toHaveBeenCalledWith("octocat", "hello-world", 7, "closed", "not_planned");
|
expect(mockSetIssueState).toHaveBeenCalledWith("octocat", "hello-world", 7, "closed", "not_planned");
|
||||||
@@ -85,7 +132,11 @@ describe("github tracking delete flow", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
await store.deleteTask(task.id);
|
await store.deleteTask(task.id);
|
||||||
await flushAsync();
|
await expectNoGithubIssueAction(
|
||||||
|
store,
|
||||||
|
(payload) => payload.taskId === task.id,
|
||||||
|
`Unexpected github-issue:action event for tracking-disabled deleted task ${task.id}`,
|
||||||
|
);
|
||||||
|
|
||||||
expect(mockSetIssueState).not.toHaveBeenCalled();
|
expect(mockSetIssueState).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
@@ -97,7 +148,11 @@ describe("github tracking delete flow", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
await store.deleteTask(task.id);
|
await store.deleteTask(task.id);
|
||||||
await flushAsync();
|
await expectNoGithubIssueAction(
|
||||||
|
store,
|
||||||
|
(payload) => payload.taskId === task.id,
|
||||||
|
`Unexpected github-issue:action event for deleted task without linked issue ${task.id}`,
|
||||||
|
);
|
||||||
|
|
||||||
expect(mockSetIssueState).not.toHaveBeenCalled();
|
expect(mockSetIssueState).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
@@ -124,8 +179,15 @@ describe("github tracking delete flow", () => {
|
|||||||
process.on("unhandledRejection", onUnhandledRejection);
|
process.on("unhandledRejection", onUnhandledRejection);
|
||||||
|
|
||||||
try {
|
try {
|
||||||
|
const failedCloseAction = waitForGithubIssueAction(
|
||||||
|
store,
|
||||||
|
(payload) => payload.taskId === task.id && payload.action === "close" && payload.outcome === "failed",
|
||||||
|
{ timeoutMessage: `Timed out waiting for failed close action for deleted task ${task.id}` },
|
||||||
|
);
|
||||||
|
|
||||||
await store.deleteTask(task.id);
|
await store.deleteTask(task.id);
|
||||||
await flushAsync();
|
await failedCloseAction;
|
||||||
|
|
||||||
expect(mockSetIssueState).toHaveBeenCalledWith("octocat", "hello-world", 8, "closed", "not_planned");
|
expect(mockSetIssueState).toHaveBeenCalledWith("octocat", "hello-world", 8, "closed", "not_planned");
|
||||||
expect(unhandledRejections).toHaveLength(0);
|
expect(unhandledRejections).toHaveLength(0);
|
||||||
} finally {
|
} finally {
|
||||||
@@ -148,15 +210,24 @@ describe("github tracking delete flow", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
const events: Array<Record<string, unknown>> = [];
|
const events: Array<Record<string, unknown>> = [];
|
||||||
(store as unknown as { on: (event: string, listener: (payload: Record<string, unknown>) => void) => void }).on(
|
const eventStore = store as unknown as StoreEventApi;
|
||||||
"github-issue:action",
|
const onAction = (payload: Record<string, unknown>) => {
|
||||||
(payload) => {
|
events.push(payload);
|
||||||
events.push(payload);
|
};
|
||||||
},
|
eventStore.on("github-issue:action", onAction);
|
||||||
);
|
|
||||||
|
|
||||||
await store.deleteTask(task.id);
|
try {
|
||||||
await flushAsync();
|
const closeAction = waitForGithubIssueAction(
|
||||||
|
store,
|
||||||
|
(payload) => payload.taskId === task.id && payload.action === "close" && payload.outcome === "success",
|
||||||
|
{ timeoutMessage: `Timed out waiting for emitted close action for deleted task ${task.id}` },
|
||||||
|
);
|
||||||
|
|
||||||
|
await store.deleteTask(task.id);
|
||||||
|
await closeAction;
|
||||||
|
} finally {
|
||||||
|
eventStore.off("github-issue:action", onAction);
|
||||||
|
}
|
||||||
|
|
||||||
expect(events).toContainEqual({
|
expect(events).toContainEqual({
|
||||||
taskId: task.id,
|
taskId: task.id,
|
||||||
|
|||||||
@@ -986,11 +986,15 @@ describe("DELETE /tasks/:id", () => {
|
|||||||
|
|
||||||
expect(res.status).toBe(200);
|
expect(res.status).toBe(200);
|
||||||
expect(res.body.id).toBe("KB-001");
|
expect(res.body.id).toBe("KB-001");
|
||||||
expect(store.deleteTask).toHaveBeenCalledWith("KB-001", {
|
expect(store.deleteTask).toHaveBeenCalledWith("KB-001", expect.objectContaining({
|
||||||
removeDependencyReferences: false,
|
removeDependencyReferences: false,
|
||||||
removeLineageReferences: false,
|
removeLineageReferences: false,
|
||||||
githubIssueAction: undefined,
|
githubIssueAction: undefined,
|
||||||
});
|
auditContext: expect.objectContaining({
|
||||||
|
agentId: "system",
|
||||||
|
runId: expect.stringMatching(/^synthetic-dashboard-delete-KB-001-/),
|
||||||
|
}),
|
||||||
|
}));
|
||||||
});
|
});
|
||||||
|
|
||||||
it("returns structured 409 conflict when delete is blocked by dependents", async () => {
|
it("returns structured 409 conflict when delete is blocked by dependents", async () => {
|
||||||
@@ -1017,11 +1021,15 @@ describe("DELETE /tasks/:id", () => {
|
|||||||
const res = await REQUEST(buildApp(), "DELETE", "/api/tasks/KB-001?removeDependencyReferences=true");
|
const res = await REQUEST(buildApp(), "DELETE", "/api/tasks/KB-001?removeDependencyReferences=true");
|
||||||
|
|
||||||
expect(res.status).toBe(200);
|
expect(res.status).toBe(200);
|
||||||
expect(store.deleteTask).toHaveBeenCalledWith("KB-001", {
|
expect(store.deleteTask).toHaveBeenCalledWith("KB-001", expect.objectContaining({
|
||||||
removeDependencyReferences: true,
|
removeDependencyReferences: true,
|
||||||
removeLineageReferences: false,
|
removeLineageReferences: false,
|
||||||
githubIssueAction: undefined,
|
githubIssueAction: undefined,
|
||||||
});
|
auditContext: expect.objectContaining({
|
||||||
|
agentId: "system",
|
||||||
|
runId: expect.stringMatching(/^synthetic-dashboard-delete-KB-001-/),
|
||||||
|
}),
|
||||||
|
}));
|
||||||
});
|
});
|
||||||
|
|
||||||
it("passes the removeLineageReferences flag when explicitly requested", async () => {
|
it("passes the removeLineageReferences flag when explicitly requested", async () => {
|
||||||
@@ -1031,11 +1039,15 @@ describe("DELETE /tasks/:id", () => {
|
|||||||
const res = await REQUEST(buildApp(), "DELETE", "/api/tasks/KB-001?removeLineageReferences=true");
|
const res = await REQUEST(buildApp(), "DELETE", "/api/tasks/KB-001?removeLineageReferences=true");
|
||||||
|
|
||||||
expect(res.status).toBe(200);
|
expect(res.status).toBe(200);
|
||||||
expect(store.deleteTask).toHaveBeenCalledWith("KB-001", {
|
expect(store.deleteTask).toHaveBeenCalledWith("KB-001", expect.objectContaining({
|
||||||
removeDependencyReferences: false,
|
removeDependencyReferences: false,
|
||||||
removeLineageReferences: true,
|
removeLineageReferences: true,
|
||||||
githubIssueAction: undefined,
|
githubIssueAction: undefined,
|
||||||
});
|
auditContext: expect.objectContaining({
|
||||||
|
agentId: "system",
|
||||||
|
runId: expect.stringMatching(/^synthetic-dashboard-delete-KB-001-/),
|
||||||
|
}),
|
||||||
|
}));
|
||||||
});
|
});
|
||||||
|
|
||||||
it.each(["close", "delete", "leave", "auto"] as const)("forwards githubIssueAction=%s", async (githubIssueAction) => {
|
it.each(["close", "delete", "leave", "auto"] as const)("forwards githubIssueAction=%s", async (githubIssueAction) => {
|
||||||
@@ -1045,11 +1057,15 @@ describe("DELETE /tasks/:id", () => {
|
|||||||
const res = await REQUEST(buildApp(), "DELETE", `/api/tasks/KB-001?githubIssueAction=${githubIssueAction}`);
|
const res = await REQUEST(buildApp(), "DELETE", `/api/tasks/KB-001?githubIssueAction=${githubIssueAction}`);
|
||||||
|
|
||||||
expect(res.status).toBe(200);
|
expect(res.status).toBe(200);
|
||||||
expect(store.deleteTask).toHaveBeenCalledWith("KB-001", {
|
expect(store.deleteTask).toHaveBeenCalledWith("KB-001", expect.objectContaining({
|
||||||
removeDependencyReferences: false,
|
removeDependencyReferences: false,
|
||||||
removeLineageReferences: false,
|
removeLineageReferences: false,
|
||||||
githubIssueAction,
|
githubIssueAction,
|
||||||
});
|
auditContext: expect.objectContaining({
|
||||||
|
agentId: "system",
|
||||||
|
runId: expect.stringMatching(/^synthetic-dashboard-delete-KB-001-/),
|
||||||
|
}),
|
||||||
|
}));
|
||||||
});
|
});
|
||||||
|
|
||||||
it("returns structured 409 conflict when delete is blocked by lineage children", async () => {
|
it("returns structured 409 conflict when delete is blocked by lineage children", async () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user