Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
94 changes: 93 additions & 1 deletion src/__tests__/health.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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'),
);
});
Comment on lines +204 to +219
Copy link
Copy Markdown
Contributor

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 parseAsync calls to prevent test pollution.

Running three parseAsync invocations back-to-back without clearing mocks causes logger.error calls to accumulate. While toHaveBeenLastCalledWith checks 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
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/__tests__/health.test.ts` around lines 204 - 219, The test calls
program.parseAsync multiple times letting logger.error mocks accumulate; add
mock clearing between each invocation to avoid cross-call pollution—either call
jest.clearAllMocks() or specifically logger.error.mockClear() after each await
program.parseAsync(...) (references: program.parseAsync and logger.error in the
'should reject non-integer and malformed intervals strictly' test) so each
assertion checks only the error produced by the immediately preceding
parseAsync.


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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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 test
it('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 Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/__tests__/health.test.ts` around lines 211 - 246, Add a test that covers
the watch-mode edge case where both sources fail: mock getRunningContainers and
getRunningPods to reject (e.g., new Error('Docker down') / 'K8s down'), use
vi.useFakeTimers(), spy process.stdout.write, call
program.parseAsync(['node','test','health','all','--watch','--interval','2'])
(or similar), advance timers as needed, then assert logger.warn was called with
messages containing 'Docker unavailable' and 'Kubernetes unavailable' and 'No
workloads found.' and assert tableUtils.renderTable was not called; finally
restore the stdout spy and reset timers/vi.useRealTimers(). Ensure you reference
the existing symbols getRunningContainers, getRunningPods, program.parseAsync,
tableUtils.renderTable, and logger.warn when adding this test.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Watch mode test doesn't stop the polling loop — timer continues after test.

The recursive setTimeout in poll() keeps scheduling new timers even after your assertions. While vi.useRealTimers() is called, any pending fake timers may still be queued. This can cause test pollution or unexpected behavior in subsequent tests.

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
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/__tests__/health.test.ts` around lines 211 - 246, The test leaves the
recursive watch polling timers queued; after advancing timers and making
assertions, explicitly clear pending fake timers before restoring real timers to
avoid test pollution — add vi.clearAllTimers() (and if the module exposes a
stop/cleanup function for the watch/poll loop, call that, e.g., stopPolling() or
health.stopWatch) after your assertions and before vi.useRealTimers(); keep the
writeSpy.mockRestore() as-is. This targets the watch/poll behavior triggered by
program.parseAsync(...) and the poll() loop.


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();
});
});
178 changes: 145 additions & 33 deletions src/commands/health.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ import { logger } from '../utils/logger';
import { createSpinner } from '../ui/spinner';
import { renderTable } from '../ui/table';

export interface HealthOptions {
watch?: boolean;
interval?: string;
signal?: AbortSignal;
}

const healthColor = (status: string): string => {
if (status === 'healthy' || status === 'running' || status === 'Running') {
return chalk.green(status);
Expand All @@ -42,66 +48,151 @@ const healthColor = (status: string): string => {
return chalk.yellow(status);
};

export const showHealth = async (target: string): Promise<void> => {
logger.info?.(`Showing health for ${target}...`);

const validTargets = ['all', 'containers', 'pods'];
if (!validTargets.includes(target)) {
logger.error?.(
`Unknown target: ${target}. Valid targets are: ${validTargets.join(', ')}.`,
);
process.exitCode = 1;
return;
}

const spinner = createSpinner(`Checking ${target} health...`).start();
const fetchHealthRows = async (target: string): Promise<(string | number)[][]> => {
const rows: (string | number)[][] = [];
const shouldFetchContainers = target === 'all' || target === 'containers';
const shouldFetchPods = target === 'all' || target === 'pods';

const [containerResult, podResult] = await Promise.allSettled([
shouldFetchContainers ? getRunningContainers() : Promise.resolve([]),
shouldFetchPods ? getRunningPods() : Promise.resolve([]),
]);

if (target === 'all' || target === 'containers') {
try {
const containers = await getRunningContainers();
if (shouldFetchContainers) {
if (containerResult.status === 'fulfilled') {
rows.push(
...containers.map((container) => [
...containerResult.value.map((container) => [
'container',
container.name,
healthColor(container.state),
container.status,
]),
);
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
} else {
const message = containerResult.reason instanceof Error
? containerResult.reason.message
: String(containerResult.reason);
logger.warn?.(`Docker unavailable: ${message}`);
}
}

if (target === 'all' || target === 'pods') {
try {
const pods = await getRunningPods();
if (shouldFetchPods) {
if (podResult.status === 'fulfilled') {
rows.push(
...pods.map((pod) => [
...podResult.value.map((pod) => [
'pod',
pod.name,
healthColor(pod.status),
`namespace: ${pod.namespace}, restarts: ${pod.restarts}`,
]),
);
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
} else {
const message = podResult.reason instanceof Error
? podResult.reason.message
: String(podResult.reason);
logger.warn?.(`Kubernetes unavailable: ${message}`);
}
}

spinner.stop();
return rows;
};

export const showHealth = async (target: string, options: HealthOptions = {}): Promise<void> => {
logger.info?.(`Showing health for ${target}...`);

if (rows.length === 0) {
logger.warn?.(`No ${target === 'all' ? 'workloads' : target} found.`);
const validTargets = ['all', 'containers', 'pods'];
if (!validTargets.includes(target)) {
logger.error?.(
`Unknown target: ${target}. Valid targets are: ${validTargets.join(', ')}.`,
);
process.exitCode = 1;
return;
}

renderTable({
head: ['TYPE', 'NAME', 'HEALTH', 'DETAILS'],
rows,
});
if (options.watch) {
const intervalStr = options.interval || '5';
if (!/^\d+$/.test(intervalStr)) {
logger.error?.('Invalid interval. Please provide a positive integer number of seconds.');
process.exitCode = 1;
return;
}
const intervalSeconds = parseInt(intervalStr, 10);
if (intervalSeconds <= 0) {
logger.error?.('Invalid interval. Please provide a positive integer number of seconds.');
process.exitCode = 1;
return;
}

let isRunning = true;
let timer: NodeJS.Timeout | undefined;

const cleanup = () => {
isRunning = false;
if (timer) {
clearTimeout(timer);
}
if (options.signal) {
options.signal.removeEventListener('abort', cleanup);
}
};

if (options.signal) {
if (options.signal.aborted) {
cleanup();
return;
}
options.signal.addEventListener('abort', cleanup);
}

const poll = async () => {
if (!isRunning || (options.signal && options.signal.aborted)) {
cleanup();
return;
}

const rows = await fetchHealthRows(target);

// Clear terminal screen
process.stdout.write('\x1Bc');

const timestamp = new Date().toLocaleTimeString();
logger.info?.(
chalk.bold.cyan(`[KDM Health] Target: ${target} | Last updated: ${timestamp} (Interval: ${intervalSeconds}s)`)
);
logger.info?.(chalk.dim('Press Ctrl+C to exit\n'));

if (rows.length === 0) {
logger.warn?.(`No ${target === 'all' ? 'workloads' : target} found.`);
} else {
renderTable({
head: ['TYPE', 'NAME', 'HEALTH', 'DETAILS'],
rows,
});
}

if (isRunning && (!options.signal || !options.signal.aborted)) {
timer = setTimeout(poll, intervalSeconds * 1000);
} else {
cleanup();
}
};
Comment thread
Rishiraj-Pathak-27 marked this conversation as resolved.

await poll();
Comment thread
Rishiraj-Pathak-27 marked this conversation as resolved.
} else {
const spinner = createSpinner(`Checking ${target} health...`).start();
const rows = await fetchHealthRows(target);
spinner.stop();

if (rows.length === 0) {
logger.warn?.(`No ${target === 'all' ? 'workloads' : target} found.`);
return;
}

renderTable({
head: ['TYPE', 'NAME', 'HEALTH', 'DETAILS'],
rows,
});
}
};

export const registerHealthCommand = (program: Command): void => {
Expand All @@ -111,5 +202,26 @@ export const registerHealthCommand = (program: Command): void => {
'Show health status for pods, containers, or all workloads.\n' +
'Valid targets: all | containers | pods',
)
.action(showHealth);
.option('-w, --watch', 'Watch mode: continuously refresh health output')
.option('-i, --interval <number>', 'Refresh interval in seconds', '5')
.action(async (target, options) => {
if (options.watch) {
const controller = new AbortController();
const sigintHandler = () => {
controller.abort();
process.exit(0);
};
process.once('SIGINT', sigintHandler);
process.once('SIGTERM', sigintHandler);

try {
await showHealth(target, { ...options, signal: controller.signal });
} finally {
process.off('SIGINT', sigintHandler);
process.off('SIGTERM', sigintHandler);
}
} else {
await showHealth(target, options);
}
});
};
Loading