-
Notifications
You must be signed in to change notification settings - Fork 7
feat: add live watch mode for kdm health Add watch mode to health command
#87
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,7 +42,7 @@ | |
|
|
||
| import { describe, it, expect, vi, beforeEach } from 'vitest'; | ||
| import { Command } from 'commander'; | ||
| import { registerHealthCommand } from '../commands/health'; | ||
| import { registerHealthCommand, showHealth } from '../commands/health'; | ||
| import { getRunningContainers } from '../docker/containers'; | ||
| import { getRunningPods } from '../kubernetes/pods'; | ||
| import { logger } from '../utils/logger'; | ||
|
|
@@ -192,4 +192,96 @@ describe('health command', () => { | |
| }), | ||
| ); | ||
| }); | ||
|
|
||
| it('should register watch and interval options', () => { | ||
| const healthCmd = program.commands.find((c) => c.name() === 'health'); | ||
| const watchOption = healthCmd?.options.find((o) => o.short === '-w'); | ||
| const intervalOption = healthCmd?.options.find((o) => o.short === '-i'); | ||
| expect(watchOption).toBeDefined(); | ||
| expect(intervalOption).toBeDefined(); | ||
| }); | ||
|
|
||
| it('should reject non-integer and malformed intervals strictly', async () => { | ||
| await program.parseAsync(['node', 'test', 'health', 'all', '--watch', '--interval', '-1']); | ||
| expect(logger.error).toHaveBeenLastCalledWith( | ||
| expect.stringContaining('Invalid interval'), | ||
| ); | ||
|
|
||
| await program.parseAsync(['node', 'test', 'health', 'all', '--watch', '--interval', '3.5']); | ||
| expect(logger.error).toHaveBeenLastCalledWith( | ||
| expect.stringContaining('Invalid interval'), | ||
| ); | ||
|
|
||
| await program.parseAsync(['node', 'test', 'health', 'all', '--watch', '--interval', '3abc']); | ||
| expect(logger.error).toHaveBeenLastCalledWith( | ||
| expect.stringContaining('Invalid interval'), | ||
| ); | ||
| }); | ||
|
|
||
| it('should poll health status periodically in watch mode', async () => { | ||
| vi.useFakeTimers(); | ||
| const writeSpy = vi.spyOn(process.stdout, 'write').mockImplementation(() => true); | ||
|
|
||
| vi.mocked(getRunningContainers).mockResolvedValue([ | ||
| { id: '1', name: 'web', image: 'nginx', state: 'running', status: 'Up 2 hours' }, | ||
| ]); | ||
| vi.mocked(getRunningPods).mockResolvedValue([]); | ||
|
|
||
| // Parse to run the watch mode command | ||
| await program.parseAsync(['node', 'test', 'health', 'all', '--watch', '--interval', '3']); | ||
|
|
||
| expect(tableUtils.renderTable).toHaveBeenCalledTimes(1); | ||
| expect(writeSpy).toHaveBeenCalledWith('\x1Bc'); | ||
|
|
||
| // Change mock status to verify the next iteration | ||
| vi.mocked(getRunningContainers).mockResolvedValue([ | ||
| { id: '1', name: 'web', image: 'nginx', state: 'exited', status: 'Exited' }, | ||
| ]); | ||
|
|
||
| // Advance timer by 3 seconds (3000ms) | ||
| await vi.advanceTimersByTimeAsync(3000); | ||
|
|
||
| expect(tableUtils.renderTable).toHaveBeenCalledTimes(2); | ||
| expect(tableUtils.renderTable).toHaveBeenLastCalledWith( | ||
| expect.objectContaining({ | ||
| rows: expect.arrayContaining([ | ||
| expect.arrayContaining(['container', 'web', expect.stringContaining('exited')]), | ||
| ]), | ||
| }), | ||
| ); | ||
|
|
||
| // Clean up | ||
| writeSpy.mockRestore(); | ||
| vi.useRealTimers(); | ||
| }); | ||
|
Comment on lines
+221
to
+256
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win Missing test coverage: both Docker and Kubernetes fail in watch mode. You have good coverage for individual failures, but no test verifies watch mode behavior when both sources fail simultaneously. This edge case should show warnings for both and render no table (or an empty state). 🧪 Suggested additional testit('should warn for both sources and render empty state in watch mode when both fail', async () => {
vi.useFakeTimers();
const writeSpy = vi.spyOn(process.stdout, 'write').mockImplementation(() => true);
vi.mocked(getRunningContainers).mockRejectedValue(new Error('Docker down'));
vi.mocked(getRunningPods).mockRejectedValue(new Error('K8s down'));
await program.parseAsync(['node', 'test', 'health', 'all', '--watch', '--interval', '2']);
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('Docker unavailable'));
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('Kubernetes unavailable'));
expect(logger.warn).toHaveBeenCalledWith('No workloads found.');
expect(tableUtils.renderTable).not.toHaveBeenCalled();
writeSpy.mockRestore();
vi.clearAllTimers();
vi.useRealTimers();
});As per coding guidelines: "Verify that tests cover both happy paths and error conditions." 🤖 Prompt for AI AgentsWatch mode test doesn't stop the polling loop — timer continues after test. The recursive Consider explicitly clearing all pending timers before restoring real timers: 🧹 Proposed cleanup expect(tableUtils.renderTable).toHaveBeenLastCalledWith(
expect.objectContaining({
rows: expect.arrayContaining([
expect.arrayContaining(['container', 'web', expect.stringContaining('exited')]),
]),
}),
);
// Clean up
writeSpy.mockRestore();
+ vi.clearAllTimers();
vi.useRealTimers();
});🤖 Prompt for AI Agents |
||
|
|
||
| it('should stop watch loop when AbortSignal is aborted', async () => { | ||
| vi.useFakeTimers(); | ||
| const writeSpy = vi.spyOn(process.stdout, 'write').mockImplementation(() => true); | ||
|
|
||
| vi.mocked(getRunningContainers).mockResolvedValue([ | ||
| { id: '1', name: 'web', image: 'nginx', state: 'running', status: 'Up 2 hours' }, | ||
| ]); | ||
| vi.mocked(getRunningPods).mockResolvedValue([]); | ||
|
|
||
| const controller = new AbortController(); | ||
|
|
||
| // Call showHealth directly with the signal and await the first poll tick | ||
| await showHealth('all', { watch: true, interval: '3', signal: controller.signal }); | ||
|
|
||
| expect(tableUtils.renderTable).toHaveBeenCalledTimes(1); | ||
|
|
||
| // Now abort the controller | ||
| controller.abort(); | ||
|
|
||
| // Advance timers by 3 seconds | ||
| await vi.advanceTimersByTimeAsync(3000); | ||
|
|
||
| // It should NOT call renderTable again because it was aborted | ||
| expect(tableUtils.renderTable).toHaveBeenCalledTimes(1); | ||
|
|
||
| // Clean up | ||
| writeSpy.mockRestore(); | ||
| vi.useRealTimers(); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Clear mocks between sequential
parseAsynccalls to prevent test pollution.Running three
parseAsyncinvocations back-to-back without clearing mocks causeslogger.errorcalls to accumulate. WhiletoHaveBeenLastCalledWithchecks only the most recent call, the test is fragile: if any call fails to trigger an error, the assertion may pass using a stale call from a previous iteration.♻️ Recommended fix: clear mocks between calls
it('should reject non-integer and malformed intervals strictly', async () => { await program.parseAsync(['node', 'test', 'health', 'all', '--watch', '--interval', '-1']); expect(logger.error).toHaveBeenLastCalledWith( expect.stringContaining('Invalid interval'), ); + vi.clearAllMocks(); await program.parseAsync(['node', 'test', 'health', 'all', '--watch', '--interval', '3.5']); expect(logger.error).toHaveBeenLastCalledWith( expect.stringContaining('Invalid interval'), ); + vi.clearAllMocks(); await program.parseAsync(['node', 'test', 'health', 'all', '--watch', '--interval', '3abc']); expect(logger.error).toHaveBeenLastCalledWith( expect.stringContaining('Invalid interval'), ); });As per coding guidelines: "Ensure mocks are clean and shared correctly."
🤖 Prompt for AI Agents