test: remove duplicate and theatrical tests (#2856)

- Replace 10x `expect(true).toBe(true)` in update-check-cov.test.ts with
  meaningful assertions: skip-condition tests now verify fetch was NOT called,
  fetch-failure tests use `resolves.toBeUndefined()`, backoff edge-case tests
  verify fetch WAS called (proving the skip was bypassed)
- Remove theatrical executor existence check (`typeof executor.execFileSync === "function"`)
  that proved nothing about behavior
- Replace structural `typeof agent.install/envVars/launchCmd === "function"` checks in
  agent-setup-cov.test.ts with assertion that agent names are non-empty strings;
  the downstream tests already prove the functions work by calling them

Co-authored-by: spawn-qa-bot <qa@openrouter.ai>

packages/cli/src/__tests__/agent-setup-cov.test.ts

@@ -88,23 +88,17 @@ describe("offerGithubAuth", () => {
 // ── createCloudAgents ──────────────────────────────────────────────────
 
 describe("createCloudAgents", () => {
-  it("returns agents map and resolveAgent function", () => {
+  it("returns agents map with all expected agent keys", () => {
     const result = createCloudAgents({
       runServer: mock(() => Promise.resolve()),
       uploadFile: mock(() => Promise.resolve()),
       downloadFile: mock(() => Promise.resolve()),
     });
-    expect(typeof result.agents).toBe("object");
-    expect(typeof result.resolveAgent).toBe("function");
     const keys = Object.keys(result.agents);
     expect(keys.length).toBeGreaterThan(0);
-    // Verify each agent has required fields
+    // All registered agents must have non-empty names
     for (const key of keys) {
-      const agent = result.agents[key];
-      expect(agent.name).toBeDefined();
-      expect(typeof agent.install).toBe("function");
-      expect(typeof agent.envVars).toBe("function");
-      expect(typeof agent.launchCmd).toBe("function");
+      expect(result.agents[key].name.length).toBeGreaterThan(0);
     }
   });
 

packages/cli/src/__tests__/update-check-cov.test.ts

@@ -70,31 +70,34 @@ describe("update-check.ts coverage", () => {
   describe("checkForUpdates skip conditions", () => {
     it("skips in test environment (NODE_ENV=test)", async () => {
       process.env.NODE_ENV = "test";
+      global.fetch = mock(async () => new Response("1.0.0"));
       const { checkForUpdates } = await import("../update-check");
       await checkForUpdates();
-      // Should return without any fetch
-      expect(true).toBe(true);
+      expect(global.fetch).not.toHaveBeenCalled();
     });
 
     it("skips when SPAWN_NO_UPDATE_CHECK=1", async () => {
       process.env.SPAWN_NO_UPDATE_CHECK = "1";
+      global.fetch = mock(async () => new Response("1.0.0"));
       const { checkForUpdates } = await import("../update-check");
       await checkForUpdates();
-      expect(true).toBe(true);
+      expect(global.fetch).not.toHaveBeenCalled();
     });
 
     it("skips when recently backed off", async () => {
       writeUpdateFailed(Date.now()); // failed just now
+      global.fetch = mock(async () => new Response("1.0.0"));
       const { checkForUpdates } = await import("../update-check");
       await checkForUpdates();
-      expect(true).toBe(true);
+      expect(global.fetch).not.toHaveBeenCalled();
     });
 
     it("skips when recently checked successfully", async () => {
       writeUpdateChecked(Date.now()); // checked just now
+      global.fetch = mock(async () => new Response("1.0.0"));
       const { checkForUpdates } = await import("../update-check");
       await checkForUpdates();
-      expect(true).toBe(true);
+      expect(global.fetch).not.toHaveBeenCalled();
     });
   });
 
@@ -119,30 +122,19 @@ describe("update-check.ts coverage", () => {
   describe("checkForUpdates fetch failure", () => {
     it("handles fetch returning null version gracefully", async () => {
       const { checkForUpdates } = await import("../update-check");
-
       global.fetch = mock(async () => new Response("not-a-version"));
-      await checkForUpdates();
-      // Should not throw, just return
-      expect(true).toBe(true);
+      // Should not throw — verify by confirming fetch was called and function completed
+      await expect(checkForUpdates()).resolves.toBeUndefined();
+      expect(global.fetch).toHaveBeenCalled();
     });
 
     it("handles fetch network error gracefully", async () => {
       const { checkForUpdates } = await import("../update-check");
-
       global.fetch = mock(async () => {
         throw new TypeError("fetch failed");
       });
-      await checkForUpdates();
-      expect(true).toBe(true);
-    });
-  });
-
-  // ── findUpdatedBinary (via executor) ──────────────────────────────────
-
-  describe("executor-based findUpdatedBinary", () => {
-    it("executor.execFileSync is accessible", async () => {
-      const { executor } = await import("../update-check");
-      expect(typeof executor.execFileSync).toBe("function");
+      // Should not throw — verify by confirming function completes without rejection
+      await expect(checkForUpdates()).resolves.toBeUndefined();
     });
   });
 
@@ -156,8 +148,8 @@ describe("update-check.ts coverage", () => {
       const pkg = await import("../../package.json");
       global.fetch = mock(async () => new Response(pkg.version));
       await checkForUpdates();
-      // Should proceed with check (not backed off)
-      expect(true).toBe(true);
+      // Should proceed with check (not backed off) — fetch was called
+      expect(global.fetch).toHaveBeenCalled();
     });
 
     it("does not skip when checked timestamp is old (>1h)", async () => {
@@ -167,7 +159,8 @@ describe("update-check.ts coverage", () => {
       const pkg = await import("../../package.json");
       global.fetch = mock(async () => new Response(pkg.version));
       await checkForUpdates();
-      expect(true).toBe(true);
+      // Should proceed with network check — fetch was called
+      expect(global.fetch).toHaveBeenCalled();
     });
 
     it("handles NaN in .update-failed file", async () => {
@@ -181,7 +174,8 @@ describe("update-check.ts coverage", () => {
       const pkg = await import("../../package.json");
       global.fetch = mock(async () => new Response(pkg.version));
       await checkForUpdates();
-      expect(true).toBe(true);
+      // NaN timestamp is not treated as recent failure — fetch proceeds
+      expect(global.fetch).toHaveBeenCalled();
     });
 
     it("handles NaN in .update-checked file", async () => {
@@ -195,7 +189,8 @@ describe("update-check.ts coverage", () => {
       const pkg = await import("../../package.json");
       global.fetch = mock(async () => new Response(pkg.version));
       await checkForUpdates();
-      expect(true).toBe(true);
+      // NaN timestamp is not treated as recent check — fetch proceeds
+      expect(global.fetch).toHaveBeenCalled();
     });
   });
 });