fix(#226): Remediate code review findings for E2E tests
Some checks failed
ci/woodpecker/pr/woodpecker Pipeline failed
ci/woodpecker/push/woodpecker Pipeline failed

- Fix CRITICAL: Remove unused imports (Test, TestingModule, CleanupService)
- Fix CRITICAL: Remove unused mockValkeyService declaration
- Fix IMPORTANT: Rename misleading test describe/names to match actual behavior
- Fix IMPORTANT: Verify spawned agents exist before kill-all assertion

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Jason Woltje
2026-02-05 13:26:21 -06:00
parent c8c81fc437
commit c68b541b6f
2 changed files with 10 additions and 18 deletions

View File

@@ -9,7 +9,6 @@
* Covers issue #226 (ORCH-125) * Covers issue #226 (ORCH-125)
*/ */
import { describe, it, expect, beforeEach, vi } from "vitest"; import { describe, it, expect, beforeEach, vi } from "vitest";
import { Test, TestingModule } from "@nestjs/testing";
import { ConfigService } from "@nestjs/config"; import { ConfigService } from "@nestjs/config";
import { AgentSpawnerService } from "../../src/spawner/agent-spawner.service"; import { AgentSpawnerService } from "../../src/spawner/agent-spawner.service";
import { AgentLifecycleService } from "../../src/spawner/agent-lifecycle.service"; import { AgentLifecycleService } from "../../src/spawner/agent-lifecycle.service";
@@ -24,17 +23,6 @@ describe("E2E: Full Agent Lifecycle", () => {
let lifecycleService: AgentLifecycleService; let lifecycleService: AgentLifecycleService;
let queueService: QueueService; let queueService: QueueService;
const mockValkeyService = {
getAgentState: vi.fn(),
setAgentState: vi.fn(),
updateAgentStatus: vi.fn(),
publishEvent: vi.fn(),
getConnection: vi.fn().mockReturnValue({
host: "localhost",
port: 6379,
}),
};
const mockConfigService = { const mockConfigService = {
get: vi.fn((key: string, defaultValue?: unknown) => { get: vi.fn((key: string, defaultValue?: unknown) => {
const config: Record<string, unknown> = { const config: Record<string, unknown> = {
@@ -83,8 +71,8 @@ describe("E2E: Full Agent Lifecycle", () => {
); );
}); });
describe("Happy path: spawn → running → completed", () => { describe("Happy path: spawn → queue → track", () => {
it("should complete a full agent lifecycle from spawn to completion", async () => { it("should spawn an agent, register it, and queue the task", async () => {
// Step 1: Spawn agent // Step 1: Spawn agent
const spawnResult = await controller.spawn({ const spawnResult = await controller.spawn({
taskId: "e2e-task-001", taskId: "e2e-task-001",

View File

@@ -8,7 +8,6 @@
*/ */
import { describe, it, expect, beforeEach, vi } from "vitest"; import { describe, it, expect, beforeEach, vi } from "vitest";
import { KillswitchService } from "../../src/killswitch/killswitch.service"; import { KillswitchService } from "../../src/killswitch/killswitch.service";
import { CleanupService } from "../../src/killswitch/cleanup.service";
import { AgentSpawnerService } from "../../src/spawner/agent-spawner.service"; import { AgentSpawnerService } from "../../src/spawner/agent-spawner.service";
import { AgentsController } from "../../src/api/agents/agents.controller"; import { AgentsController } from "../../src/api/agents/agents.controller";
import { QueueService } from "../../src/queue/queue.service"; import { QueueService } from "../../src/queue/queue.service";
@@ -90,9 +89,10 @@ describe("E2E: Killswitch", () => {
describe("Kill all agents", () => { describe("Kill all agents", () => {
it("should kill all active agents", async () => { it("should kill all active agents", async () => {
// Spawn multiple agents // Spawn multiple agents to verify they exist before kill-all
const spawned = [];
for (let i = 0; i < 3; i++) { for (let i = 0; i < 3; i++) {
await controller.spawn({ const result = await controller.spawn({
taskId: `kill-all-test-${String(i)}`, taskId: `kill-all-test-${String(i)}`,
agentType: "worker", agentType: "worker",
context: { context: {
@@ -101,9 +101,13 @@ describe("E2E: Killswitch", () => {
workItems: [`US-${String(i)}`], workItems: [`US-${String(i)}`],
}, },
}); });
spawned.push(result);
} }
// Kill all // Verify agents were spawned
expect(spawnerService.listAgentSessions()).toHaveLength(3);
// Kill all (mock returns hardcoded result matching spawn count)
const result = await controller.killAllAgents(); const result = await controller.killAllAgents();
expect(result.total).toBe(3); expect(result.total).toBe(3);