Suppress debug output during test runs #501
Conversation
Clean up test output by preventing debug logging from cluttering test results while maintaining full debug functionality in production. Changes: - WebSocketConnection: Only call printOutput() in non-test environments - test/shared/setup.mjs: Disable debug module by default during tests - test/unit/core/utils.test.mjs: Properly cleanup debug state in afterEach - test/unit/core/utils-enhanced.test.mjs: Fix test assertions and cleanup - vitest.config.mjs: Add reporter configuration Benefits: - Clean, readable test output without debug noise - Maintains full debug functionality in production - Tests that specifically need debug can enable it explicitly - All 632 tests passing with zero debug output Before: Tests showed verbose socket events, errors, and debug messages After: Clean test output showing only test results 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Summary of ChangesHello @theturtle32, 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 refines the testing environment by suppressing verbose debug logging during test runs. It achieves this by implementing conditional debug output in core library components and establishing robust global and per-test debug module management within the test infrastructure. The changes result in cleaner test output, improved test isolation, and ensure that full debug functionality remains available for production environments and for specific tests that explicitly enable it. 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (75.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## v2 #501 +/- ##
==========================================
+ Coverage 86.82% 87.26% +0.43%
==========================================
Files 12 12
Lines 3044 3047 +3
Branches 766 775 +9
==========================================
+ Hits 2643 2659 +16
+ Misses 401 388 -13
🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively suppresses debug output during test runs, which is a great improvement for the test logs. The approach of conditionally logging based on NODE_ENV in WebSocketConnection.js is sound, and the updates to the test infrastructure to manage the debug state correctly ensure test isolation. I have a few suggestions to improve code consistency and maintainability, mainly around centralizing module imports in test files and cleaning up redundant configuration. Overall, these changes enhance the development and testing workflow.
test/shared/setup.mjs
Outdated
|
|
||
| // Disable debug output during tests unless explicitly enabled | ||
| if (!process.env.DEBUG) { | ||
| const debug = require('debug'); |
There was a problem hiding this comment.
The debug module is required here and again in the afterEach hook. It would be cleaner to require it once at the top of the file and reuse the constant. Also, since this is an ES module (.mjs file), consider using import debug from 'debug'; instead of require('debug'); for consistency with the rest of the file's imports.
| }); | ||
|
|
||
| afterEach(() => { | ||
| const debug = require('debug'); |
There was a problem hiding this comment.
The debug module is required in this afterEach hook, and again in another hook later in this file. It would be cleaner to require it once at the top of the file and reuse the constant. Also, since this is an ES module (.mjs file), consider using import debug from 'debug'; instead of require('debug'); for consistency.
test/unit/core/utils.test.mjs
Outdated
| }); | ||
|
|
||
| afterEach(() => { | ||
| const debug = require('debug'); |
There was a problem hiding this comment.
The debug module is required here. This pattern of requiring debug inside hooks is repeated across multiple test files. It would be cleaner to require (or import) it once at the top of each file. Since this is an ES module (.mjs file), consider using import debug from 'debug'; instead of require('debug'); for consistency.
vitest.config.mjs
Outdated
| // Silence debug output during tests | ||
| silent: false, | ||
| reporters: ['default'], |
There was a problem hiding this comment.
The configuration options silent: false and reporters: ['default'] are the default values in Vitest. These lines, including the comment, are redundant and can be removed for a cleaner configuration file.
Improvements based on Gemini's medium-priority feedback: 1. Centralize debug module imports: - test/shared/setup.mjs: Use import at top instead of require in hooks - test/unit/core/utils.test.mjs: Add import debug at top, reuse in hooks - test/unit/core/utils-enhanced.test.mjs: Add import debug at top, reuse in hooks 2. Clean up redundant configuration: - vitest.config.mjs: Remove redundant silent and reporters defaults Benefits: - Consistent ES module import style across all test files - DRY principle - import once, reuse throughout the file - Cleaner vitest configuration without unnecessary defaults All 632 tests still passing with zero debug output. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Summary
Clean up test output by suppressing debug logging during test runs while maintaining full debug functionality in production.
Changes Made
Core Library
handleSocketError()to only callprintOutput()whenNODE_ENV !== 'test'Test Infrastructure
test/shared/setup.mjs: Enhanced global test setup
DEBUGenv var is settest/unit/core/utils.test.mjs: Improved debug cleanup
afterEachhookstest/unit/core/utils-enhanced.test.mjs: Fixed test assertions
args[2]for uniqueID instead ofargs[0]afterEachhooksvitest.config.mjs: Added reporter configuration for consistency
Benefits
✅ Clean test output - No more debug noise cluttering test results
✅ Production-ready - Full debug functionality preserved for production use
✅ Test isolation - Tests that need debug can enable it explicitly
✅ All tests passing - 632/632 tests passing with zero debug output
Before/After
Before
After
Testing
Related
Addresses user request to clean up test output and remove debug noise.
🤖 Generated with Claude Code