Skip to content

Commit adbd907

Browse files
committed
fix(test): remove stale debug-level stderr assertions and fix logger state leak
The upgrade command tests asserted on log.debug() output that was demoted from log.info() in commit 062bb74 but the test expectations were never updated. These tests only passed on CI due to a logger state leak from command.test.ts (--verbose test sets level to debug without cleanup), which happened to run first on CI but not locally. - Remove stale stderr assertions for debug-level messages in cli.test.ts (the same data is verified via the structured JSON output) - Add beforeEach/afterEach logger level cleanup to the 'buildCommand output config' describe block in command.test.ts
1 parent bcb9beb commit adbd907

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)