fix(tests): silence excessive log noise in AppContainer.test.tsx#23578
fix(tests): silence excessive log noise in AppContainer.test.tsx#23578Shyam-Raghuwanshi wants to merge 1 commit intogoogle-gemini:mainfrom
Conversation
The AppContainer test suite (107 tests) was dumping hundreds of lines of noisy output on every run, making CI and local test output hard to read. Three sources of noise, now fixed: 1. '[SessionSummary] Error finding previous session' — caused by the real generateSummary() hitting uninitialised storage. Fixed by adding a hoisted mock for generateSummary in the core vi.mock factory. 2. 'Failed to read or parse keybindings' — caused by the real useTerminalSetupPrompt() trying to read the host filesystem. Fixed by mocking ./utils/terminalSetup.js. 3. console.log/debug/warn/error noise from debugLogger and React act() warnings. Fixed by mocking debugLogger from core and adding targeted console spies in beforeEach (console.error only suppresses the known React act() message, letting real errors through).
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly reduces the excessive log noise and unhandled rejection errors generated during the execution of the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the excessive log noise in AppContainer.test.tsx by mocking several dependencies that were causing side effects like filesystem access and uninitialized storage access. The use of hoisted mocks for generateSummary and debugLogger is a good approach. I have one major concern regarding the blanket suppression of console.log, console.warn, and console.debug, which could impact the maintainability of these tests.
| vi.spyOn(console, 'log').mockImplementation(() => {}); | ||
| vi.spyOn(console, 'debug').mockImplementation(() => {}); | ||
| vi.spyOn(console, 'warn').mockImplementation(() => {}); |
There was a problem hiding this comment.
While silencing test noise is the goal, completely suppressing console.log, console.debug, and console.warn can hide important debugging information for future test failures and may mask new issues. This makes the test suite harder to maintain.
A better approach would be to identify any remaining sources of noise and mock them directly, similar to how generateSummary and useTerminalSetupPrompt were handled. If some warnings are unavoidable and known, they could be filtered out selectively, just like the React act(...) warning is handled for console.error.
Could you please remove these blanket suppressions and address any remaining noise at its source?
|
We do not accept outside contributions for |
Problem
The
AppContainer.test.tsxsuite (107 tests) was dumping hundreds of lines of noisy output on every run, drowning out actual test results and making CI logs hard to read.Every single test case produced at least these two lines of noise:
[SessionSummary] Error finding previous session: Storage must be initialized before useFailed to read or parse keybindings, assuming prompt is needed: SyntaxError: Unexpected token ...Plus ~6 lines per test of React
act(...)environment warnings on stderr.Root Causes and Fixes
1. generateSummary was not mocked
The real
generateSummary()from@google/gemini-cli-corewas called during each test, hitting uninitialised storage and logging viadebugLogger.debug().Fix: Added a hoisted mock for
generateSummary(returning a resolved Promise) in thevi.mockfactory, and re-set it inbeforeEachaftervi.clearAllMocks().2. useTerminalSetupPrompt was not mocked
The real hook tried to read keybindings files from the host filesystem, failed, and logged via
debugLogger.debug().Fix: Added
vi.mock('./utils/terminalSetup.js')that stubsuseTerminalSetupPromptas a no-op.3. debugLogger was not mocked
All remaining
debugLogger.log/warn/error/debugcalls inAppContainer.tsxwere hitting realconsole.*methods.Fix: Added a hoisted no-op
debugLoggermock in the core mock factory, plus targetedconsolespies inbeforeEach(console.erroronly suppresses the known React act() warning, letting real errors through).Result
Related Issues
#23328