diff --git a/command/up.ts b/command/up.ts index 8693e54..6a23e7e 100644 --- a/command/up.ts +++ b/command/up.ts @@ -738,7 +738,7 @@ export async function runUp( }; const workspacePath = `/workspaces/${encodeURIComponent(project)}`; - const taskCtx = await new Listr( + const taskCtx = await new Listr( [ { title: "Read compose", @@ -840,12 +840,16 @@ export async function runUp( ); return operation; }; - await task - .newListr( - services.map((name) => ({ + return task.newListr( + [ + ...services.map((name) => ({ title: `Build ${name}`, rendererOptions: { outputBar: 10, persistentOutput: true }, - task: async (_ctx, child) => { + exitOnError: false, + task: async ( + _ctx: UpContext, + child: ListrTaskWrapper, + ) => { children.set(name, child); child.output = "Build queued"; if (children.size === services.length) start(); @@ -854,14 +858,20 @@ export async function runUp( child.output = "done"; }, })), - { concurrent: true, exitOnError: false }, - ) - .run(); - await operation; - if (!result) throw operationFailure ?? new Error("Build images failed"); - taskCtx.buildImages = result.images; - await config.postBuild?.(result, await getHookContext()); - task.title = `Built ${result.built.length} image${result.built.length === 1 ? "" : "s"}`; + { + task: async () => { + await Promise.all(completed.values()); + await operation; + if (!result) + throw operationFailure ?? new Error("Build images failed"); + taskCtx.buildImages = result.images; + await config.postBuild?.(result, await getHookContext()); + task.title = `Built ${result.built.length} image${result.built.length === 1 ? "" : "s"}`; + }, + }, + ], + { concurrent: true }, + ); }, }, { @@ -1072,11 +1082,10 @@ export async function runUp( }, ], { - // Build progress can remain queued across many polls. The default TTY - // spinner redraws every child on every tick, which duplicates those - // frames in captured terminal output. Simple emits only task events. - renderer: build ? "simple" : "default", - rendererOptions: { collapseErrors: false }, + renderer: "default", + // Only redraw when a task changes; a queued build should not generate + // another frame on each spinner tick. Keep the renderer's task tree. + rendererOptions: { collapseErrors: false, collapseSubtasks: false, lazy: true }, }, ).run(); diff --git a/tests/command/up-api.test.ts b/tests/command/up-api.test.ts index be00633..0ec21d4 100644 --- a/tests/command/up-api.test.ts +++ b/tests/command/up-api.test.ts @@ -1,5 +1,5 @@ import { describe, expect, spyOn, test } from "bun:test"; -import { Listr } from "listr2"; +import { DefaultRenderer, Listr } from "listr2"; import { mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -26,48 +26,81 @@ const snapshot = { }; describe("up API pipeline", () => { - test("does not append queued build frames on repeated TTY progress ticks", async () => { + test("keeps queued builds in the TTY render tree without duplicate rows", async () => { const root = await mkdtemp(join(tmpdir(), "kuber-up-api-")); const previousCwd = process.cwd(); const tty = Object.getOwnPropertyDescriptor(process.stdout, "isTTY"); - const rendered: string[] = []; + const frames: string[] = []; + const create = DefaultRenderer.prototype.create; + const renderSpy = spyOn(DefaultRenderer.prototype, "create").mockImplementation(function (this: DefaultRenderer, options) { + const frame = create.call(this, options); + frames.push(frame); + return frame; + }); const writes = spyOn(process.stdout, "write").mockImplementation(((chunk: string | Uint8Array) => { - rendered.push(String(chunk)); return true; }) as typeof process.stdout.write); - let polls = 0; + const polls = new Map(); + const builds = new Map(); try { Object.defineProperty(process.stdout, "isTTY", { configurable: true, value: true }); - await writeFile(join(root, "compose.yml"), "services:\n app:\n build: .\n"); + await writeFile(join(root, "compose.yml"), "services:\n app:\n build: .\n client:\n build:\n context: .\n args:\n ROLE: client\n"); await writeFile(join(root, ".kuberrc.ts"), 'export default { project: "shop" };\n'); process.chdir(root); const trust = await resolveTrustIdentity("shop", root); - const request: ApiRequester = async (path: string) => { + const request: ApiRequester = async (path: string, init?: ApiRequestInit) => { if (path === "/snapshots/negotiate") return { ready: true } as T; - if (path === "/builds") return { state: "queued" } as T; + if (path === "/builds") { + const build = init!.json as { id: string; service: string }; + builds.set(build.id, build.service); + return { state: "queued" } as T; + } + const id = path.split("/")[2]!; + const service = builds.get(id)!; if (path.includes("/events")) return [{ type: "status", status: { state: "queued", - phase: ["queued", "preparing", "waiting", "waiting"][polls], + phase: ["queued", "preparing", "waiting", "waiting"][polls.get(service) ?? 0], } }] as T; if (path.endsWith("/reconcile")) { - polls++; + const count = (polls.get(service) ?? 0) + 1; + polls.set(service, count); await Bun.sleep(110); - return polls < 3 + return count < 3 ? { state: "queued" } as T - : { state: "failed", error: "build stopped" } as T; + : service === "client" + ? { state: "failed", error: "build stopped\nstack detail" } as T + : { state: "succeeded" } as T; } + if (path.endsWith("/result")) return { reference: "image:app" } as T; throw new Error(path); }; await expect(provideContext(() => runUp(true, request, { trust }))).rejects.toThrow("build stopped"); - const output = rendered.join(""); - expect(polls).toBe(3); - expect(output.match(/Build app/g)).toHaveLength(1); - expect(output.match(/Build queued/g)).toHaveLength(1); - expect(output).toContain("Build preparing"); - expect(output).toContain("Build waiting"); - expect(output).toContain("build stopped"); + expect(polls.get("client")).toBe(3); + const active = frames.filter((frame) => + frame.includes("Build images") && frame.includes("Build app") && frame.includes("Build client") && + frame.split("\n").filter((row) => row.includes("Build queued")).length === 2); + expect(active.length).toBeGreaterThan(0); + for (const frame of frames.filter((frame) => frame.includes("Build images") && frame.includes("Build app") && frame.includes("Build client"))) { + const rows = frame.split("\n"); + const parent = rows.find((row) => row.includes("Build images"))!; + for (const name of ["app", "client"]) { + const children = rows.filter((row) => row.includes(`Build ${name}`)); + expect(children).toHaveLength(1); + expect(children[0]!.search(/\S/)).toBeGreaterThan(parent.search(/\S/)); + } + expect(rows.filter((row) => row.includes("Build queued")).length).toBeLessThanOrEqual(2); + } + expect(active[0]).toMatch(/Build app\n {2,}.*Build queued[\s\S]*Build client\n {2,}.*Build queued/); + const final = frames.at(-1)!; + expect(final).toMatch(/Build images[\s\S]* .*Build app[\s\S]* .*Build client/); + expect(final).toMatch(/✔.*Build app/); + expect(final).toMatch(/✖.*Build client/); + expect(frames.some((frame) => frame.includes("Build preparing") && frame.includes("Build waiting"))).toBe(true); + expect(final).toContain("build stopped"); + expect(final).toContain("stack detail"); } finally { + renderSpy.mockRestore(); writes.mockRestore(); if (tty) Object.defineProperty(process.stdout, "isTTY", tty); else Reflect.deleteProperty(process.stdout, "isTTY"); @@ -85,7 +118,8 @@ describe("up API pipeline", () => { const runSpy = spyOn(Listr.prototype, "run").mockImplementation(function (this: Listr) { if (this.tasks[0]?.title === "Build web") { for (const entry of this.tasks) { - const name = entry.title!.replace(/^Build /, ""); + if (!entry.title) continue; + const name = entry.title.replace(/^Build /, ""); const executable = entry as unknown as { taskFn: typeof entry.task.task }; const originalTask = executable.taskFn; executable.taskFn = async (ctx, child) => {