Skip to content

Suppress debug output during test runs #501

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
theturtle32 merged 2 commits into from
Oct 6, 2025
Merged

Suppress debug output during test runs #501

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d3b8ea9
Suppress debug output during test runs
Karim13014 Oct 6, 2025
89fecc3
Address Gemini code review comments
Karim13014 Oct 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion lib/WebSocketConnection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,10 @@ class WebSocketConnection extends EventEmitter {
this.emit('error', error);
}
this.socket.destroy();
this._debug.printOutput();
// Only print debug output in non-test environments to avoid cluttering test output
if (process.env.NODE_ENV !== 'test') {
this._debug.printOutput();
}
}

handleSocketEnd() {
Expand Down
17 changes: 16 additions & 1 deletion test/shared/setup.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,34 @@ import { stopAllServers } from '../helpers/test-server.mjs';
// Vitest adds exit/beforeExit listeners for each test file with spawned processes
process.setMaxListeners(30);

// Disable debug output during tests unless explicitly enabled
if (!process.env.DEBUG) {
const debug = require('debug');

@gemini-code-assist gemini-code-assist Bot Oct 6, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

debug.disable();
}

// Global test setup for each test file
beforeEach(() => {
// Clear all mocks and timers
vi.clearAllTimers();
vi.clearAllMocks();

// Note: We don't disable debug in beforeEach as some tests need to enable it
// Tests that enable DEBUG should clean up properly in their own afterEach
});

afterEach(async () => {
// Restore all mocks
vi.restoreAllMocks();

// Clean up any test servers
await stopAllServers();

// Re-disable debug after each test to prevent leakage
if (!process.env.DEBUG) {
const debug = require('debug');
debug.disable();
}
});

// Set up global test configuration
Expand Down
15 changes: 14 additions & 1 deletion test/unit/core/utils-enhanced.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,13 @@ describe('Utils Module - Enhanced Coverage', () => {
});

afterEach(() => {
const debug = require('debug');

@gemini-code-assist gemini-code-assist Bot Oct 6, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

debug.disable();
if (originalDebugEnv !== undefined) {
process.env.DEBUG = originalDebugEnv;
if (originalDebugEnv) {
debug.enable(originalDebugEnv);
}
} else {
delete process.env.DEBUG;
}
Expand Down Expand Up @@ -62,8 +67,13 @@ describe('Utils Module - Enhanced Coverage', () => {
});

afterEach(() => {
const debug = require('debug');
debug.disable();
if (originalDebugEnv !== undefined) {
process.env.DEBUG = originalDebugEnv;
if (originalDebugEnv) {
debug.enable(originalDebugEnv);
}
} else {
delete process.env.DEBUG;
}
Expand All @@ -81,9 +91,12 @@ describe('Utils Module - Enhanced Coverage', () => {

expect(mockLog).toHaveBeenCalled();
// Verify the format includes timestamp and uniqueID
// printOutput calls logFunction.apply(global, args) where args includes:
// [formatString, date, uniqueID, ...originalArgs]
const firstCall = mockLog.mock.calls[0];
expect(firstCall).toBeDefined();
expect(firstCall[0]).toContain('test-id');
// The uniqueID should be in args[2] (after formatString and date)
expect(firstCall[2]).toBe('test-id');
}
});

Expand Down
5 changes: 5 additions & 0 deletions test/unit/core/utils.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,8 +251,13 @@ describe('Utils Module', () => {
});

afterEach(() => {
const debug = require('debug');

@gemini-code-assist gemini-code-assist Bot Oct 6, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

debug.disable();
if (originalDebugEnv !== undefined) {
process.env.DEBUG = originalDebugEnv;
if (originalDebugEnv) {
debug.enable(originalDebugEnv);
}
} else {
delete process.env.DEBUG;
}
Expand Down
3 changes: 3 additions & 0 deletions vitest.config.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ export default defineConfig({
singleThread: true
}
},
// Silence debug output during tests
silent: false,
reporters: ['default'],

@gemini-code-assist gemini-code-assist Bot Oct 6, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

// Timeouts for WebSocket operations
testTimeout: 15000,
hookTimeout: 15000,
Expand Down