Compare commits

...
Author SHA1 Message Date
Tabish Bidiwale 6b3f64e3a0 test: add unit tests for interactive utilities and CLI flag
- Export resolveNoInteractive() helper for reuse
- Add InteractiveOptions type export for testing
- Refactor validate.ts to use resolveNoInteractive()
- Add 17 unit tests for isInteractive() and resolveNoInteractive()
- Add CLI integration test for --no-interactive flag

This prevents future regressions where Commander.js --no-* flag
parsing is not properly handled.
2025-12-23 12:30:23 +11:00
Tabish Bidiwale b2c2799939 fix(cli): respect --no-interactive flag in validate command
The validate command's spinner was starting regardless of the
--no-interactive flag, causing hangs in pre-commit hooks.

Changes:
- Pass noInteractive option to runBulkValidation
- Handle Commander.js --no-* flag syntax (sets interactive=false)
- Only start ora spinner when in interactive mode
- Add CI environment variable check to isInteractive() for industry
  standard compliance
2025-12-23 12:14:51 +11:00
4 changed files with 153 additions and 6 deletions
+5 -4
View File
@@ -1,7 +1,7 @@
import ora from 'ora';
import path from 'path';
import { Validator } from '../core/validation/validator.js';
import { isInteractive } from '../utils/interactive.js';
import { isInteractive, resolveNoInteractive } from '../utils/interactive.js';
import { getActiveChangeIds, getSpecIds } from '../utils/item-discovery.js';
import { nearestMatches } from '../utils/match.js';
@@ -15,6 +15,7 @@ interface ExecuteOptions {
strict?: boolean;
json?: boolean;
noInteractive?: boolean;
interactive?: boolean; // Commander sets this to false when --no-interactive is used
concurrency?: string;
}
@@ -35,7 +36,7 @@ export class ValidateCommand {
await this.runBulkValidation({
changes: !!options.all || !!options.changes,
specs: !!options.all || !!options.specs,
}, { strict: !!options.strict, json: !!options.json, concurrency: options.concurrency });
}, { strict: !!options.strict, json: !!options.json, concurrency: options.concurrency, noInteractive: resolveNoInteractive(options) });
return;
}
@@ -180,8 +181,8 @@ export class ValidateCommand {
bullets.forEach(b => console.error(` ${b}`));
}
private async runBulkValidation(scope: { changes: boolean; specs: boolean }, opts: { strict: boolean; json: boolean; concurrency?: string }): Promise<void> {
const spinner = !opts.json ? ora('Validating...').start() : undefined;
private async runBulkValidation(scope: { changes: boolean; specs: boolean }, opts: { strict: boolean; json: boolean; concurrency?: string; noInteractive?: boolean }): Promise<void> {
const spinner = !opts.json && !opts.noInteractive ? ora('Validating...').start() : undefined;
const [changeIds, specIds] = await Promise.all([
scope.changes ? getActiveChangeIds() : Promise.resolve<string[]>([]),
scope.specs ? getSpecIds() : Promise.resolve<string[]>([]),
+9 -2
View File
@@ -1,4 +1,4 @@
type InteractiveOptions = {
export type InteractiveOptions = {
/**
* Explicit "disable prompts" flag passed by internal callers.
*/
@@ -9,7 +9,12 @@ type InteractiveOptions = {
interactive?: boolean;
};
function resolveNoInteractive(value?: boolean | InteractiveOptions): boolean {
/**
* Resolves whether non-interactive mode is requested.
* Handles both explicit `noInteractive: true` and Commander.js style `interactive: false`.
* Use this helper instead of manually checking options.noInteractive to avoid bugs.
*/
export function resolveNoInteractive(value?: boolean | InteractiveOptions): boolean {
if (typeof value === 'boolean') return value;
return value?.noInteractive === true || value?.interactive === false;
}
@@ -17,6 +22,8 @@ function resolveNoInteractive(value?: boolean | InteractiveOptions): boolean {
export function isInteractive(value?: boolean | InteractiveOptions): boolean {
if (resolveNoInteractive(value)) return false;
if (process.env.OPEN_SPEC_INTERACTIVE === '0') return false;
// Respect the standard CI environment variable (set by GitHub Actions, GitLab CI, Travis, etc.)
if ('CI' in process.env) return false;
return !!process.stdin.isTTY;
}
+14
View File
@@ -130,4 +130,18 @@ describe('top-level validate command', () => {
const result = await runCLI(['validate', changeId], { cwd: testDir });
expect(result.exitCode).toBe(0);
});
it('respects --no-interactive flag passed via CLI', async () => {
// This test ensures Commander.js --no-interactive flag is correctly parsed
// and passed to the validate command. The flag sets options.interactive = false
// (not options.noInteractive = true) due to Commander.js convention.
const result = await runCLI(['validate', '--specs', '--no-interactive'], {
cwd: testDir,
// Don't set OPEN_SPEC_INTERACTIVE to ensure we're testing the flag itself
env: { ...process.env, OPEN_SPEC_INTERACTIVE: undefined },
});
expect(result.exitCode).toBe(0);
// Should complete without hanging and without prompts
expect(result.stderr).not.toContain('What would you like to validate?');
});
});
+125
View File
@@ -0,0 +1,125 @@
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { isInteractive, resolveNoInteractive, InteractiveOptions } from '../../src/utils/interactive.js';
describe('interactive utilities', () => {
let originalOpenSpecInteractive: string | undefined;
let originalCI: string | undefined;
let originalStdinIsTTY: boolean | undefined;
beforeEach(() => {
// Save original environment
originalOpenSpecInteractive = process.env.OPEN_SPEC_INTERACTIVE;
originalCI = process.env.CI;
originalStdinIsTTY = process.stdin.isTTY;
// Clear environment for clean testing
delete process.env.OPEN_SPEC_INTERACTIVE;
delete process.env.CI;
});
afterEach(() => {
// Restore original environment
if (originalOpenSpecInteractive !== undefined) {
process.env.OPEN_SPEC_INTERACTIVE = originalOpenSpecInteractive;
} else {
delete process.env.OPEN_SPEC_INTERACTIVE;
}
if (originalCI !== undefined) {
process.env.CI = originalCI;
} else {
delete process.env.CI;
}
// Restore stdin.isTTY
Object.defineProperty(process.stdin, 'isTTY', {
value: originalStdinIsTTY,
writable: true,
configurable: true,
});
});
describe('resolveNoInteractive', () => {
it('should return true when noInteractive is true', () => {
expect(resolveNoInteractive({ noInteractive: true })).toBe(true);
});
it('should return true when interactive is false (Commander.js style)', () => {
// This is how Commander.js handles --no-interactive flag
expect(resolveNoInteractive({ interactive: false })).toBe(true);
});
it('should return false when noInteractive is false', () => {
expect(resolveNoInteractive({ noInteractive: false })).toBe(false);
});
it('should return false when interactive is true', () => {
expect(resolveNoInteractive({ interactive: true })).toBe(false);
});
it('should return false for empty options object', () => {
expect(resolveNoInteractive({})).toBe(false);
});
it('should return false for undefined', () => {
expect(resolveNoInteractive(undefined)).toBe(false);
});
it('should handle boolean value true', () => {
expect(resolveNoInteractive(true)).toBe(true);
});
it('should handle boolean value false', () => {
expect(resolveNoInteractive(false)).toBe(false);
});
it('should prioritize noInteractive over interactive when both set', () => {
// noInteractive: true should win
expect(resolveNoInteractive({ noInteractive: true, interactive: true })).toBe(true);
// If noInteractive is false, check interactive
expect(resolveNoInteractive({ noInteractive: false, interactive: false })).toBe(true);
});
});
describe('isInteractive', () => {
it('should return false when noInteractive is true', () => {
expect(isInteractive({ noInteractive: true })).toBe(false);
});
it('should return false when interactive is false (Commander.js --no-interactive)', () => {
expect(isInteractive({ interactive: false })).toBe(false);
});
it('should return false when OPEN_SPEC_INTERACTIVE env var is 0', () => {
process.env.OPEN_SPEC_INTERACTIVE = '0';
Object.defineProperty(process.stdin, 'isTTY', { value: true, writable: true, configurable: true });
expect(isInteractive({})).toBe(false);
});
it('should return false when CI env var is set', () => {
process.env.CI = 'true';
Object.defineProperty(process.stdin, 'isTTY', { value: true, writable: true, configurable: true });
expect(isInteractive({})).toBe(false);
});
it('should return false when CI env var is set to any value', () => {
// CI can be set to any value, not just "true"
process.env.CI = '1';
Object.defineProperty(process.stdin, 'isTTY', { value: true, writable: true, configurable: true });
expect(isInteractive({})).toBe(false);
});
it('should return false when stdin is not a TTY', () => {
Object.defineProperty(process.stdin, 'isTTY', { value: false, writable: true, configurable: true });
expect(isInteractive({})).toBe(false);
});
it('should return true when stdin is TTY and no flags disable it', () => {
Object.defineProperty(process.stdin, 'isTTY', { value: true, writable: true, configurable: true });
expect(isInteractive({})).toBe(true);
});
it('should return true when stdin is TTY and options are undefined', () => {
Object.defineProperty(process.stdin, 'isTTY', { value: true, writable: true, configurable: true });
expect(isInteractive(undefined)).toBe(true);
});
});
});