refactor the fix based on the e2e test failure

This commit is contained in:
Cynthia Long
2026-03-17 04:43:10 +00:00
parent 2e466bd4cb
commit bd60ba9e68
5 changed files with 150 additions and 140 deletions
-2
View File
@@ -31,7 +31,6 @@ import {
ValidationRequiredError,
type AdminControlsSettings,
debugLogger,
isHeadlessMode,
} from '@google/gemini-cli-core';
import { loadCliConfig, parseArguments } from './config/config.js';
@@ -272,7 +271,6 @@ export async function main() {
const isDebugMode = cliConfig.isDebugMode(argv);
const consolePatcher = new ConsolePatcher({
stderr: true,
suppressConsoleOutput: isHeadlessMode() && !isDebugMode,
debugMode: isDebugMode,
onNewMessage: (msg) => {
coreEvents.emitConsoleLog(msg.type, msg.content);
@@ -235,30 +235,6 @@ describe('runNonInteractive', () => {
const getWrittenOutput = () =>
processStdoutSpy.mock.calls.map((c) => c[0]).join('');
it('should initialize ConsolePatcher with correct value', async () => {
const { ConsolePatcher } = await import('./ui/utils/ConsolePatcher.js');
const patcherSpy = vi.spyOn(ConsolePatcher.prototype, 'patch');
const events: ServerGeminiStreamEvent[] = [
{
type: GeminiEventType.Finished,
value: { reason: undefined, usageMetadata: undefined },
},
];
mockGeminiClient.sendMessageStream.mockReturnValueOnce(
createStreamFromEvents(events),
);
await runNonInteractive({
config: mockConfig,
settings: mockSettings,
input: 'test input',
prompt_id: 'test-prompt-id',
});
expect(patcherSpy).toHaveBeenCalled();
});
it('should process input and write text output', async () => {
const events: ServerGeminiStreamEvent[] = [
{ type: GeminiEventType.Content, value: 'Hello' },
-3
View File
@@ -27,7 +27,6 @@ import {
CoreEvent,
createWorkingStdio,
recordToolCallInteractions,
isHeadlessMode,
ToolErrorType,
Scheduler,
ROOT_SCHEDULER_ID,
@@ -64,10 +63,8 @@ export async function runNonInteractive({
resumedSessionData,
}: RunNonInteractiveParams): Promise<void> {
return promptIdContext.run(prompt_id, async () => {
const suppressConsoleOutput = isHeadlessMode() && !config.getDebugMode();
const consolePatcher = new ConsolePatcher({
stderr: true,
suppressConsoleOutput,
debugMode: config.getDebugMode(),
onNewMessage: (msg) => {
coreEvents.emitConsoleLog(msg.type, msg.content);
+147 -102
View File
@@ -6,135 +6,180 @@
/* eslint-disable no-console */
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { describe, it, expect, vi, afterEach } from 'vitest';
import { ConsolePatcher } from './ConsolePatcher.js';
describe('ConsolePatcher', () => {
let patcher: ConsolePatcher;
const onNewMessage = vi.fn();
beforeEach(() => {
vi.clearAllMocks();
});
afterEach(() => {
if (patcher) {
patcher.cleanup();
}
vi.restoreAllMocks();
vi.clearAllMocks();
});
it('should suppress output when suppressConsoleOutput is true and debugMode is false', () => {
const originalError = console.error;
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
it('should patch and restore console methods', () => {
const beforeLog = console.log;
const beforeWarn = console.warn;
const beforeError = console.error;
const beforeDebug = console.debug;
const beforeInfo = console.info;
patcher = new ConsolePatcher({
debugMode: false,
suppressConsoleOutput: true,
stderr: true,
});
patcher = new ConsolePatcher({ onNewMessage, debugMode: false });
patcher.patch();
console.log('test log');
expect(errorSpy).not.toHaveBeenCalled();
expect(console.log).not.toBe(beforeLog);
expect(console.warn).not.toBe(beforeWarn);
expect(console.error).not.toBe(beforeError);
expect(console.debug).not.toBe(beforeDebug);
expect(console.info).not.toBe(beforeInfo);
patcher.cleanup();
errorSpy.mockRestore();
console.error = originalError;
expect(console.log).toBe(beforeLog);
expect(console.warn).toBe(beforeWarn);
expect(console.error).toBe(beforeError);
expect(console.debug).toBe(beforeDebug);
expect(console.info).toBe(beforeInfo);
});
it('should NOT suppress console.error even when suppressConsoleOutput is true', () => {
const originalError = console.error;
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
describe('when stderr is false', () => {
it('should call onNewMessage for log, warn, error, and info', () => {
patcher = new ConsolePatcher({
onNewMessage,
debugMode: false,
stderr: false,
});
patcher.patch();
patcher = new ConsolePatcher({
debugMode: false,
suppressConsoleOutput: true,
stderr: true,
});
patcher.patch();
console.error('test error');
expect(errorSpy).toHaveBeenCalled();
patcher.cleanup();
errorSpy.mockRestore();
console.error = originalError;
});
it('should NOT suppress output when suppressConsoleOutput is true but debugMode is true', () => {
const originalError = console.error;
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
patcher = new ConsolePatcher({
debugMode: true,
suppressConsoleOutput: true,
stderr: true,
});
patcher.patch();
console.log('test log');
// When stderr is true, log goes to originalConsoleError
expect(errorSpy).toHaveBeenCalled();
patcher.cleanup();
errorSpy.mockRestore();
console.error = originalError;
});
it('should NOT suppress output when suppressConsoleOutput is false', () => {
const originalError = console.error;
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
patcher = new ConsolePatcher({
debugMode: false,
suppressConsoleOutput: false,
stderr: true,
});
patcher.patch();
console.error('test error');
expect(errorSpy).toHaveBeenCalled();
patcher.cleanup();
errorSpy.mockRestore();
console.error = originalError;
});
it('should call onNewMessage when stderr is false and not suppressed', () => {
patcher = new ConsolePatcher({
debugMode: false,
suppressConsoleOutput: false,
stderr: false,
onNewMessage,
});
patcher.patch();
console.log('test log');
expect(onNewMessage).toHaveBeenCalledWith(
expect.objectContaining({
console.log('test log');
expect(onNewMessage).toHaveBeenCalledWith({
type: 'log',
content: 'test log',
}),
);
count: 1,
});
patcher.cleanup();
console.warn('test warn');
expect(onNewMessage).toHaveBeenCalledWith({
type: 'warn',
content: 'test warn',
count: 1,
});
console.error('test error');
expect(onNewMessage).toHaveBeenCalledWith({
type: 'error',
content: 'test error',
count: 1,
});
console.info('test info');
expect(onNewMessage).toHaveBeenCalledWith({
type: 'info',
content: 'test info',
count: 1,
});
});
it('should not call onNewMessage for debug when debugMode is false', () => {
patcher = new ConsolePatcher({
onNewMessage,
debugMode: false,
stderr: false,
});
patcher.patch();
console.debug('test debug');
expect(onNewMessage).not.toHaveBeenCalled();
});
it('should call onNewMessage for debug when debugMode is true', () => {
patcher = new ConsolePatcher({
onNewMessage,
debugMode: true,
stderr: false,
});
patcher.patch();
console.debug('test debug');
expect(onNewMessage).toHaveBeenCalledWith({
type: 'debug',
content: 'test debug',
count: 1,
});
});
it('should format multiple arguments using util.format', () => {
patcher = new ConsolePatcher({
onNewMessage,
debugMode: false,
stderr: false,
});
patcher.patch();
console.log('test %s %d', 'string', 123);
expect(onNewMessage).toHaveBeenCalledWith({
type: 'log',
content: 'test string 123',
count: 1,
});
});
});
it('should NOT suppress output when suppressConsoleOutput is true but debugMode is true (explicit check)', () => {
const onNewMessage = vi.fn();
patcher = new ConsolePatcher({
debugMode: true,
suppressConsoleOutput: true,
stderr: false,
onNewMessage,
describe('when stderr is true', () => {
it('should redirect warn and error to originalConsoleError', () => {
const spyError = vi.spyOn(console, 'error').mockImplementation(() => {});
patcher = new ConsolePatcher({ debugMode: false, stderr: true });
patcher.patch();
console.warn('test warn');
expect(spyError).toHaveBeenCalledWith('test warn');
console.error('test error');
expect(spyError).toHaveBeenCalledWith('test error');
});
patcher.patch();
console.log('test log');
it('should ignore log and info when debugMode is false', () => {
const spyError = vi.spyOn(console, 'error').mockImplementation(() => {});
patcher = new ConsolePatcher({ debugMode: false, stderr: true });
patcher.patch();
expect(onNewMessage).toHaveBeenCalled();
patcher.cleanup();
console.log('test log');
console.info('test info');
expect(spyError).not.toHaveBeenCalled();
});
it('should redirect log and info to originalConsoleError when debugMode is true', () => {
const spyError = vi.spyOn(console, 'error').mockImplementation(() => {});
patcher = new ConsolePatcher({ debugMode: true, stderr: true });
patcher.patch();
console.log('test log');
expect(spyError).toHaveBeenCalledWith('test log');
console.info('test info');
expect(spyError).toHaveBeenCalledWith('test info');
});
it('should ignore debug when debugMode is false', () => {
const spyError = vi.spyOn(console, 'error').mockImplementation(() => {});
patcher = new ConsolePatcher({ debugMode: false, stderr: true });
patcher.patch();
console.debug('test debug');
expect(spyError).not.toHaveBeenCalled();
});
it('should redirect debug to originalConsoleError when debugMode is true', () => {
const spyError = vi.spyOn(console, 'error').mockImplementation(() => {});
patcher = new ConsolePatcher({ debugMode: true, stderr: true });
patcher.patch();
console.debug('test debug');
expect(spyError).toHaveBeenCalledWith('test debug');
});
});
});
+3 -9
View File
@@ -13,7 +13,6 @@ interface ConsolePatcherParams {
onNewMessage?: (message: Omit<ConsoleMessageItem, 'id'>) => void;
debugMode: boolean;
stderr?: boolean;
suppressConsoleOutput?: boolean;
}
export class ConsolePatcher {
@@ -50,15 +49,10 @@ export class ConsolePatcher {
private patchConsoleMethod =
(type: 'log' | 'warn' | 'error' | 'debug' | 'info') =>
(...args: unknown[]) => {
if (
this.params.suppressConsoleOutput &&
!this.params.debugMode &&
type !== 'error'
) {
return;
}
if (this.params.stderr) {
if ((type === 'info' || type === 'log') && !this.params.debugMode) {
return;
}
if (type !== 'debug' || this.params.debugMode) {
this.originalConsoleError(this.formatArgs(args));
}