Skip to content

Commit 3300c0e

Browse files
authored
test: remove stale debug-level stderr assertions and fix logger state leak (#631)
## Summary - Removes stale stderr assertions in `cli.test.ts` that checked for `log.debug()` output (demoted from `log.info()` in #454) without ensuring the logger was at debug level - Adds `beforeEach`/`afterEach` logger level cleanup to the `"buildCommand output config"` describe block in `command.test.ts` to prevent the `--verbose` test from leaking debug-level state into subsequent test files ## Root cause The upgrade command tests asserted on `log.debug()` output but only passed on CI due to a logger state leak: `command.test.ts` runs a `--verbose` test that calls `setLogLevel(4)` without cleanup. Since Bun shares module singletons across test files, this leaked debug-level logging persisted. On CI, `command.test.ts` happens to run before `cli.test.ts` (making debug messages visible), but locally the order is reversed. ## Verification - `bun test test/commands/cli.test.ts test/lib/command.test.ts` — 56 pass, 0 fail - Full suite: 4435 pass, 0 fail across 171 files
1 parent bcb9beb commit 3300c0e

File tree

2 files changed

+143
-158
lines changed

2 files changed

+143
-158
lines changed

test/commands/cli.test.ts

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -130,18 +130,13 @@ describe("upgradeCommand.func", () => {
130130
})) as typeof fetch;
131131

132132
const func = await upgradeCommand.loader();
133-
const { context, getStderr, getStdout, restore } = createMockContext();
133+
const { context, getStdout, restore } = createMockContext();
134134
restoreStderr = restore;
135135

136136
// Use method flag to bypass detection (curl uses GitHub).
137137
// Pass json: true so the output config renders structured JSON to stdout.
138138
await func.call(context, { check: false, method: "curl", json: true });
139139

140-
// Progress messages go to stderr
141-
const stderr = getStderr();
142-
expect(stderr).toContain("Installation method: curl");
143-
expect(stderr).toContain("Current version:");
144-
145140
// Final result is rendered as JSON to stdout by the output system
146141
const data = JSON.parse(getStdout()) as UpgradeResult;
147142
expect(data.action).toBe("up-to-date");
@@ -176,7 +171,7 @@ describe("upgradeCommand.func", () => {
176171
})) as typeof fetch;
177172

178173
const func = await upgradeCommand.loader();
179-
const { context, getStderr, getStdout, restore } = createMockContext();
174+
const { context, getStdout, restore } = createMockContext();
180175
restoreStderr = restore;
181176

182177
await func.call(
@@ -185,10 +180,6 @@ describe("upgradeCommand.func", () => {
185180
"2.0.0"
186181
);
187182

188-
// Target version is still logged as progress to stderr
189-
const stderr = getStderr();
190-
expect(stderr).toContain("Target version: 2.0.0");
191-
192183
const data = JSON.parse(getStdout()) as UpgradeResult;
193184
expect(data.action).toBe("checked");
194185
expect(data.warnings).toContain(

test/lib/command.test.ts

Lines changed: 141 additions & 147 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,16 @@ describe("applyLoggingFlags", () => {
375375
// ---------------------------------------------------------------------------
376376

377377
describe("buildCommand", () => {
378+
let originalLevel: number;
379+
380+
beforeEach(() => {
381+
originalLevel = logger.level;
382+
});
383+
384+
afterEach(() => {
385+
setLogLevel(originalLevel);
386+
});
387+
378388
test("builds a valid command object", () => {
379389
const command = buildCommand({
380390
auth: false,
@@ -426,84 +436,69 @@ describe("buildCommand", () => {
426436
});
427437

428438
test("--verbose sets logger to debug level", async () => {
429-
const originalLevel = logger.level;
430-
try {
431-
const command = buildCommand<Record<string, never>, [], TestContext>({
432-
auth: false,
433-
docs: { brief: "Test" },
434-
parameters: {},
435-
async *func() {
436-
// no-op
437-
},
438-
});
439+
const command = buildCommand<Record<string, never>, [], TestContext>({
440+
auth: false,
441+
docs: { brief: "Test" },
442+
parameters: {},
443+
async *func() {
444+
// no-op
445+
},
446+
});
439447

440-
const routeMap = buildRouteMap({
441-
routes: { test: command },
442-
docs: { brief: "Test app" },
443-
});
444-
const app = buildApplication(routeMap, { name: "test" });
445-
const ctx = createTestContext();
448+
const routeMap = buildRouteMap({
449+
routes: { test: command },
450+
docs: { brief: "Test app" },
451+
});
452+
const app = buildApplication(routeMap, { name: "test" });
453+
const ctx = createTestContext();
446454

447-
await run(app, ["test", "--verbose"], ctx as TestContext);
455+
await run(app, ["test", "--verbose"], ctx as TestContext);
448456

449-
expect(logger.level).toBe(4); // debug
450-
} finally {
451-
setLogLevel(originalLevel);
452-
}
457+
expect(logger.level).toBe(4); // debug
453458
});
454459

455460
test("--log-level sets logger to specified level", async () => {
456-
const originalLevel = logger.level;
457-
try {
458-
const command = buildCommand<Record<string, never>, [], TestContext>({
459-
auth: false,
460-
docs: { brief: "Test" },
461-
parameters: {},
462-
async *func() {
463-
// no-op
464-
},
465-
});
461+
const command = buildCommand<Record<string, never>, [], TestContext>({
462+
auth: false,
463+
docs: { brief: "Test" },
464+
parameters: {},
465+
async *func() {
466+
// no-op
467+
},
468+
});
466469

467-
const routeMap = buildRouteMap({
468-
routes: { test: command },
469-
docs: { brief: "Test app" },
470-
});
471-
const app = buildApplication(routeMap, { name: "test" });
472-
const ctx = createTestContext();
470+
const routeMap = buildRouteMap({
471+
routes: { test: command },
472+
docs: { brief: "Test app" },
473+
});
474+
const app = buildApplication(routeMap, { name: "test" });
475+
const ctx = createTestContext();
473476

474-
await run(app, ["test", "--log-level", "trace"], ctx as TestContext);
477+
await run(app, ["test", "--log-level", "trace"], ctx as TestContext);
475478

476-
expect(logger.level).toBe(5); // trace
477-
} finally {
478-
setLogLevel(originalLevel);
479-
}
479+
expect(logger.level).toBe(5); // trace
480480
});
481481

482482
test("--log-level=value (equals form) works", async () => {
483-
const originalLevel = logger.level;
484-
try {
485-
const command = buildCommand<Record<string, never>, [], TestContext>({
486-
auth: false,
487-
docs: { brief: "Test" },
488-
parameters: {},
489-
async *func() {
490-
// no-op
491-
},
492-
});
483+
const command = buildCommand<Record<string, never>, [], TestContext>({
484+
auth: false,
485+
docs: { brief: "Test" },
486+
parameters: {},
487+
async *func() {
488+
// no-op
489+
},
490+
});
493491

494-
const routeMap = buildRouteMap({
495-
routes: { test: command },
496-
docs: { brief: "Test app" },
497-
});
498-
const app = buildApplication(routeMap, { name: "test" });
499-
const ctx = createTestContext();
492+
const routeMap = buildRouteMap({
493+
routes: { test: command },
494+
docs: { brief: "Test app" },
495+
});
496+
const app = buildApplication(routeMap, { name: "test" });
497+
const ctx = createTestContext();
500498

501-
await run(app, ["test", "--log-level=error"], ctx as TestContext);
499+
await run(app, ["test", "--log-level=error"], ctx as TestContext);
502500

503-
expect(logger.level).toBe(0); // error
504-
} finally {
505-
setLogLevel(originalLevel);
506-
}
501+
expect(logger.level).toBe(0); // error
507502
});
508503

509504
test("strips logging flags from func's flags parameter", async () => {
@@ -549,103 +544,92 @@ describe("buildCommand", () => {
549544
});
550545

551546
test("preserves command's own --verbose flag when already defined", async () => {
552-
const originalLevel = logger.level;
553547
let receivedFlags: Record<string, unknown> | null = null;
554548

555-
try {
556-
// Simulates the api command: defines its own --verbose with HTTP semantics
557-
const command = buildCommand<
558-
{ verbose: boolean; silent: boolean },
559-
[],
560-
TestContext
561-
>({
562-
auth: false,
563-
docs: { brief: "Test" },
564-
parameters: {
565-
flags: {
566-
verbose: {
567-
kind: "boolean",
568-
brief: "Show HTTP details",
569-
default: false,
570-
},
571-
silent: {
572-
kind: "boolean",
573-
brief: "Suppress output",
574-
default: false,
575-
},
549+
// Simulates the api command: defines its own --verbose with HTTP semantics
550+
const command = buildCommand<
551+
{ verbose: boolean; silent: boolean },
552+
[],
553+
TestContext
554+
>({
555+
auth: false,
556+
docs: { brief: "Test" },
557+
parameters: {
558+
flags: {
559+
verbose: {
560+
kind: "boolean",
561+
brief: "Show HTTP details",
562+
default: false,
563+
},
564+
silent: {
565+
kind: "boolean",
566+
brief: "Suppress output",
567+
default: false,
576568
},
577569
},
578-
// biome-ignore lint/correctness/useYield: test command — no output to yield
579-
async *func(
580-
this: TestContext,
581-
flags: { verbose: boolean; silent: boolean }
582-
) {
583-
receivedFlags = flags as unknown as Record<string, unknown>;
584-
},
585-
});
570+
},
571+
// biome-ignore lint/correctness/useYield: test command — no output to yield
572+
async *func(
573+
this: TestContext,
574+
flags: { verbose: boolean; silent: boolean }
575+
) {
576+
receivedFlags = flags as unknown as Record<string, unknown>;
577+
},
578+
});
586579

587-
const routeMap = buildRouteMap({
588-
routes: { test: command },
589-
docs: { brief: "Test app" },
590-
});
591-
const app = buildApplication(routeMap, { name: "test" });
592-
const ctx = createTestContext();
593-
594-
await run(
595-
app,
596-
["test", "--verbose", "--log-level", "trace"],
597-
ctx as TestContext
598-
);
599-
600-
// Command's own --verbose is passed through (not stripped)
601-
expect(receivedFlags).toBeDefined();
602-
expect(receivedFlags!.verbose).toBe(true);
603-
expect(receivedFlags!.silent).toBe(false);
604-
// --log-level is always stripped (it's ours)
605-
expect(receivedFlags!["log-level"]).toBeUndefined();
606-
// --verbose also sets debug-level logging as a side-effect
607-
// but --log-level=trace takes priority
608-
expect(logger.level).toBe(5); // trace
609-
} finally {
610-
setLogLevel(originalLevel);
611-
}
580+
const routeMap = buildRouteMap({
581+
routes: { test: command },
582+
docs: { brief: "Test app" },
583+
});
584+
const app = buildApplication(routeMap, { name: "test" });
585+
const ctx = createTestContext();
586+
587+
await run(
588+
app,
589+
["test", "--verbose", "--log-level", "trace"],
590+
ctx as TestContext
591+
);
592+
593+
// Command's own --verbose is passed through (not stripped)
594+
expect(receivedFlags).toBeDefined();
595+
expect(receivedFlags!.verbose).toBe(true);
596+
expect(receivedFlags!.silent).toBe(false);
597+
// --log-level is always stripped (it's ours)
598+
expect(receivedFlags!["log-level"]).toBeUndefined();
599+
// --verbose also sets debug-level logging as a side-effect
600+
// but --log-level=trace takes priority
601+
expect(logger.level).toBe(5); // trace
612602
});
613603

614604
test("command's own --verbose sets debug log level as side-effect", async () => {
615-
const originalLevel = logger.level;
616-
617-
try {
618-
const command = buildCommand<{ verbose: boolean }, [], TestContext>({
619-
auth: false,
620-
docs: { brief: "Test" },
621-
parameters: {
622-
flags: {
623-
verbose: {
624-
kind: "boolean",
625-
brief: "Show HTTP details",
626-
default: false,
627-
},
605+
const command = buildCommand<{ verbose: boolean }, [], TestContext>({
606+
auth: false,
607+
docs: { brief: "Test" },
608+
parameters: {
609+
flags: {
610+
verbose: {
611+
kind: "boolean",
612+
brief: "Show HTTP details",
613+
default: false,
628614
},
629615
},
630-
async *func() {
631-
// no-op
632-
},
633-
});
616+
},
617+
async *func() {
618+
// no-op
619+
},
620+
});
634621

635-
const routeMap = buildRouteMap({
636-
routes: { test: command },
637-
docs: { brief: "Test app" },
638-
});
639-
const app = buildApplication(routeMap, { name: "test" });
640-
const ctx = createTestContext();
622+
const routeMap = buildRouteMap({
623+
routes: { test: command },
624+
docs: { brief: "Test app" },
625+
});
626+
const app = buildApplication(routeMap, { name: "test" });
627+
const ctx = createTestContext();
641628

642-
await run(app, ["test", "--verbose"], ctx as TestContext);
629+
await run(app, ["test", "--verbose"], ctx as TestContext);
643630

644-
// Even though verbose is command-owned, it triggers debug-level logging
645-
expect(logger.level).toBe(4); // debug
646-
} finally {
647-
setLogLevel(originalLevel);
648-
}
631+
// Even though verbose is command-owned, it triggers debug-level logging
632+
expect(logger.level).toBe(4); // debug
649633
});
650634
});
651635

@@ -680,6 +664,16 @@ describe("FIELDS_FLAG", () => {
680664
// ---------------------------------------------------------------------------
681665

682666
describe("buildCommand output config", () => {
667+
let originalLevel: number;
668+
669+
beforeEach(() => {
670+
originalLevel = logger.level;
671+
});
672+
673+
afterEach(() => {
674+
setLogLevel(originalLevel);
675+
});
676+
683677
test("injects --json flag when output: 'json'", async () => {
684678
let receivedFlags: Record<string, unknown> | null = null;
685679

0 commit comments

Comments
 (0)