diff --git a/e2e/cli-e2e/tests/__snapshots__/help.e2e.test.ts.snap b/e2e/cli-e2e/tests/__snapshots__/help.e2e.test.ts.snap index dcea5545a..beb76af1f 100644 --- a/e2e/cli-e2e/tests/__snapshots__/help.e2e.test.ts.snap +++ b/e2e/cli-e2e/tests/__snapshots__/help.e2e.test.ts.snap @@ -19,7 +19,7 @@ Commands: Global Options: --progress Show progress bar in stdout. - [boolean] [default: true] + [boolean] [default: false in CI environment, otherwise true] --verbose When true creates more verbose output. This is helpful w hen debugging. You may also set CP_VERBOSE env variable instead. [boolean] [default: false] diff --git a/packages/ci/src/lib/cli/commands/collect.ts b/packages/ci/src/lib/cli/commands/collect.ts index aa873395b..d98aacf4a 100644 --- a/packages/ci/src/lib/cli/commands/collect.ts +++ b/packages/ci/src/lib/cli/commands/collect.ts @@ -12,7 +12,6 @@ export async function runCollect({ command: bin, args: [ ...(isVerbose() ? ['--verbose'] : []), - '--no-progress', ...(config ? [`--config=${config}`] : []), ...DEFAULT_PERSIST_FORMAT.map(format => `--persist.format=${format}`), ], diff --git a/packages/ci/src/lib/run.int.test.ts b/packages/ci/src/lib/run.int.test.ts index 4f2ca5d2a..5afcb43c1 100644 --- a/packages/ci/src/lib/run.int.test.ts +++ b/packages/ci/src/lib/run.int.test.ts @@ -260,12 +260,7 @@ describe('runInCI', () => { } satisfies utils.ProcessConfig); expect(utils.executeProcess).toHaveBeenNthCalledWith(2, { command: options.bin, - args: [ - '--verbose', - '--no-progress', - '--persist.format=json', - '--persist.format=md', - ], + args: ['--verbose', '--persist.format=json', '--persist.format=md'], cwd: workDir, observer: expectedObserver, } satisfies utils.ProcessConfig); @@ -347,12 +342,7 @@ describe('runInCI', () => { } satisfies utils.ProcessConfig); expect(utils.executeProcess).toHaveBeenNthCalledWith(2, { command: options.bin, - args: [ - '--verbose', - '--no-progress', - '--persist.format=json', - '--persist.format=md', - ], + args: ['--verbose', '--persist.format=json', '--persist.format=md'], cwd: workDir, observer: expectedObserver, } satisfies utils.ProcessConfig); @@ -368,12 +358,7 @@ describe('runInCI', () => { } satisfies utils.ProcessConfig); expect(utils.executeProcess).toHaveBeenNthCalledWith(4, { command: options.bin, - args: [ - '--verbose', - '--no-progress', - '--persist.format=json', - '--persist.format=md', - ], + args: ['--verbose', '--persist.format=json', '--persist.format=md'], cwd: workDir, observer: expectedObserver, } satisfies utils.ProcessConfig); @@ -450,12 +435,7 @@ describe('runInCI', () => { } satisfies utils.ProcessConfig); expect(utils.executeProcess).toHaveBeenNthCalledWith(2, { command: options.bin, - args: [ - '--verbose', - '--no-progress', - '--persist.format=json', - '--persist.format=md', - ], + args: ['--verbose', '--persist.format=json', '--persist.format=md'], cwd: workDir, observer: expectedObserver, } satisfies utils.ProcessConfig); @@ -647,12 +627,7 @@ describe('runInCI', () => { } satisfies utils.ProcessConfig); expect(utils.executeProcess).toHaveBeenCalledWith({ command: runMany, - args: [ - '--verbose', - '--no-progress', - '--persist.format=json', - '--persist.format=md', - ], + args: ['--verbose', '--persist.format=json', '--persist.format=md'], cwd: expect.stringContaining(workDir), observer: expectedObserver, } satisfies utils.ProcessConfig); @@ -814,12 +789,7 @@ describe('runInCI', () => { } satisfies utils.ProcessConfig); expect(utils.executeProcess).toHaveBeenCalledWith({ command: runMany, - args: [ - '--verbose', - '--no-progress', - '--persist.format=json', - '--persist.format=md', - ], + args: ['--verbose', '--persist.format=json', '--persist.format=md'], cwd: expect.stringContaining(workDir), observer: expectedObserver, } satisfies utils.ProcessConfig); @@ -1013,12 +983,7 @@ describe('runInCI', () => { } satisfies utils.ProcessConfig); expect(utils.executeProcess).toHaveBeenCalledWith({ command: options.bin, - args: [ - '--verbose', - '--no-progress', - '--persist.format=json', - '--persist.format=md', - ], + args: ['--verbose', '--persist.format=json', '--persist.format=md'], cwd: expect.stringContaining(workDir), observer: expectedObserver, } satisfies utils.ProcessConfig); @@ -1191,12 +1156,7 @@ describe('runInCI', () => { } satisfies utils.ProcessConfig); expect(utils.executeProcess).toHaveBeenCalledWith({ command: options.bin, - args: [ - '--verbose', - '--no-progress', - '--persist.format=json', - '--persist.format=md', - ], + args: ['--verbose', '--persist.format=json', '--persist.format=md'], cwd: expect.stringContaining(workDir), observer: expectedObserver, } satisfies utils.ProcessConfig); diff --git a/packages/cli/README.md b/packages/cli/README.md index ca75f0722..38bb77e73 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -197,7 +197,7 @@ Each example is fully tested to demonstrate best practices for plugin testing as | Option | Type | Default | Description | | ---------------- | --------- | -------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------- | -| **`--progress`** | `boolean` | `true` | Show progress bar in stdout. | +| **`--progress`** | `boolean` | `false` in CI, otherwise `true` | Show progress bar in stdout. | | **`--verbose`** | `boolean` | `false` | When true creates more verbose output. This is helpful when debugging. You may also set `CP_VERBOSE` env variable instead. | | **`--config`** | `string` | looks for `code-pushup.config.{ts\|mjs\|js}` | Path to config file. | | **`--tsconfig`** | `string` | n/a | Path to a TypeScript config, used to load config file. | diff --git a/packages/cli/src/lib/implementation/global.options.ts b/packages/cli/src/lib/implementation/global.options.ts index d541bdc7a..e0ca4b41c 100644 --- a/packages/cli/src/lib/implementation/global.options.ts +++ b/packages/cli/src/lib/implementation/global.options.ts @@ -1,4 +1,5 @@ import type { Options } from 'yargs'; +import { isCI } from '@code-pushup/utils'; import type { GeneralCliOptions } from './global.model.js'; export function yargsGlobalOptionsDefinition(): Record< @@ -9,7 +10,8 @@ export function yargsGlobalOptionsDefinition(): Record< progress: { describe: 'Show progress bar in stdout.', type: 'boolean', - default: true, + default: !isCI(), + defaultDescription: 'false in CI environment, otherwise true', }, verbose: { describe: diff --git a/packages/cli/src/lib/implementation/set-verbose.middleware.int.test.ts b/packages/cli/src/lib/implementation/set-verbose.middleware.int.test.ts index 4918f6cc4..bfe0584a4 100644 --- a/packages/cli/src/lib/implementation/set-verbose.middleware.int.test.ts +++ b/packages/cli/src/lib/implementation/set-verbose.middleware.int.test.ts @@ -14,7 +14,7 @@ describe('setVerboseMiddleware', () => { [false, false, false], ['True', undefined, true], ['TRUE', undefined, true], - [42, undefined, false], + [0, undefined, false], [undefined, 'true', true], [true, 'False', false], ])( diff --git a/packages/cli/src/lib/implementation/set-verbose.middleware.ts b/packages/cli/src/lib/implementation/set-verbose.middleware.ts index 95260d255..8d77e1175 100644 --- a/packages/cli/src/lib/implementation/set-verbose.middleware.ts +++ b/packages/cli/src/lib/implementation/set-verbose.middleware.ts @@ -1,5 +1,6 @@ import type { GlobalOptions } from '@code-pushup/core'; import type { CoreConfig } from '@code-pushup/models'; +import { coerceBooleanValue } from '@code-pushup/utils'; import type { FilterOptions } from './filter.model.js'; import type { GeneralCliOptions } from './global.model'; @@ -21,8 +22,8 @@ import type { GeneralCliOptions } from './global.model'; export function setVerboseMiddleware< T extends GeneralCliOptions & CoreConfig & FilterOptions & GlobalOptions, >(originalProcessArgs: T): T { - const envVerbose = getNormalizedOptionValue(process.env['CP_VERBOSE']); - const cliVerbose = getNormalizedOptionValue(originalProcessArgs.verbose); + const envVerbose = coerceBooleanValue(process.env['CP_VERBOSE']); + const cliVerbose = coerceBooleanValue(originalProcessArgs.verbose); const verboseEffect = cliVerbose ?? envVerbose ?? false; // eslint-disable-next-line functional/immutable-data @@ -33,20 +34,3 @@ export function setVerboseMiddleware< verbose: verboseEffect, }; } - -export function getNormalizedOptionValue(value: unknown): boolean | undefined { - if (typeof value === 'boolean') { - return value; - } - if (typeof value === 'string') { - const lowerCaseValue = value.toLowerCase(); - if (lowerCaseValue === 'true') { - return true; - } - if (lowerCaseValue === 'false') { - return false; - } - } - - return undefined; -} diff --git a/packages/cli/src/lib/implementation/set-verbose.middleware.unit.test.ts b/packages/cli/src/lib/implementation/set-verbose.middleware.unit.test.ts index 440a7c508..ada1f6cd9 100644 --- a/packages/cli/src/lib/implementation/set-verbose.middleware.unit.test.ts +++ b/packages/cli/src/lib/implementation/set-verbose.middleware.unit.test.ts @@ -1,32 +1,7 @@ import { describe, expect, it, vi } from 'vitest'; -import { - getNormalizedOptionValue, - setVerboseMiddleware, -} from './set-verbose.middleware.js'; +import { setVerboseMiddleware } from './set-verbose.middleware.js'; describe('setVerboseMiddleware', () => { - it.each([ - [true, true], - ['true', true], - ['True', true], - ['TRUE', true], - [false, false], - ['false', false], - ['False', false], - ['FALSE', false], - [undefined, undefined], - ['undefined', undefined], - ['Whatever else', undefined], - [0, undefined], - [1, undefined], - [null, undefined], - ])( - 'should return normalize value of `%j` option set as `%j`', - (value, expected) => { - expect(getNormalizedOptionValue(value)).toBe(expected); - }, - ); - it.each([ [true, undefined, true], [false, undefined, false], diff --git a/packages/cli/src/lib/yargs-cli.int.test.ts b/packages/cli/src/lib/yargs-cli.int.test.ts index 4e99a2311..7865108cc 100644 --- a/packages/cli/src/lib/yargs-cli.int.test.ts +++ b/packages/cli/src/lib/yargs-cli.int.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest'; import type { CoreConfig, Format } from '@code-pushup/models'; +import { isCI } from '@code-pushup/utils'; import { yargsHistoryOptionsDefinition } from './history/history.options.js'; import type { CompareOptions } from './implementation/compare.model.js'; import { yargsCompareOptionsDefinition } from './implementation/compare.options.js'; @@ -21,7 +22,7 @@ describe('yargsCli', () => { options, }).parseAsync(); expect(parsedArgv.verbose).toBe(false); - expect(parsedArgv.progress).toBe(true); + expect(parsedArgv.progress).toBe(!isCI()); }); it('should parse an empty array as a default onlyPlugins option', async () => { @@ -112,7 +113,6 @@ describe('yargsCli', () => { FilterOptions >( [ - '--verbose', '--persist.format=md', '--persist.outputDir=code-pushdown/output/dir', '--persist.filename=code-pushdown-report', @@ -130,9 +130,8 @@ describe('yargsCli', () => { expect(parsedArgv).toEqual( expect.objectContaining({ // default values - progress: true, + verbose: false, // overridden arguments - verbose: true, persist: expect.objectContaining({ outputDir: 'code-pushdown/output/dir', filename: 'code-pushdown-report', diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 3b6b8a556..b5cb7596b 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -9,6 +9,12 @@ export { export { filesCoverageToTree, type FileCoverage } from './lib/coverage-tree.js'; export { createRunnerFiles } from './lib/create-runner-files.js'; export { comparePairs, matchArrayItemsByKey, type Diff } from './lib/diff.js'; +export { + coerceBooleanValue, + isCI, + isEnvVarEnabled, + isVerbose, +} from './lib/env.js'; export { stringifyError } from './lib/errors.js'; export { executeProcess, @@ -69,7 +75,7 @@ export { isPromiseRejectedResult, } from './lib/guards.js'; export { logMultipleResults } from './lib/log-results.js'; -export { isVerbose, link, ui, type CliUi, type Column } from './lib/logging.js'; +export { link, ui, type CliUi, type Column } from './lib/logging.js'; export { mergeConfigs } from './lib/merge-configs.js'; export { getProgressBar, type ProgressBar } from './lib/progress.js'; export { asyncSequential, groupByStatus } from './lib/promises.js'; diff --git a/packages/utils/src/lib/env.ts b/packages/utils/src/lib/env.ts new file mode 100644 index 000000000..b77f17558 --- /dev/null +++ b/packages/utils/src/lib/env.ts @@ -0,0 +1,53 @@ +import { ui } from './logging.js'; + +export function isCI() { + return isEnvVarEnabled('CI'); +} + +export function isVerbose() { + return isEnvVarEnabled('CP_VERBOSE'); +} + +export function isEnvVarEnabled(name: string): boolean { + const value = coerceBooleanValue(process.env[name]); + + if (typeof value === 'boolean') { + return value; + } + + if (process.env[name]) { + ui().logger.warning( + `Environment variable ${name} expected to be a boolean (true/false/1/0), but received value ${process.env[name]}. Treating it as disabled.`, + ); + } + + return false; +} + +export function coerceBooleanValue(value: unknown): boolean | undefined { + if (typeof value === 'boolean') { + return value; + } + + if (typeof value === 'string') { + const booleanValuePairs = [ + ['true', 'false'], + ['on', 'off'], + ['yes', 'no'], + ]; + const lowerCaseValue = value.toLowerCase(); + // eslint-disable-next-line functional/no-loop-statements + for (const [trueValue, falseValue] of booleanValuePairs) { + if (lowerCaseValue === trueValue || lowerCaseValue === falseValue) { + return lowerCaseValue === trueValue; + } + } + + const intValue = Number.parseInt(value, 10); + if (!Number.isNaN(intValue)) { + return intValue !== 0; + } + } + + return undefined; +} diff --git a/packages/utils/src/lib/env.unit.test.ts b/packages/utils/src/lib/env.unit.test.ts new file mode 100644 index 000000000..9bff5a54d --- /dev/null +++ b/packages/utils/src/lib/env.unit.test.ts @@ -0,0 +1,66 @@ +import { coerceBooleanValue, isEnvVarEnabled } from './env.js'; +import { ui } from './logging.js'; + +describe('isEnvVarEnabled', () => { + beforeEach(() => { + vi.unstubAllEnvs(); + }); + + it('should consider missing variable disabled', () => { + expect(isEnvVarEnabled('CP_VERBOSE')).toBe(false); + }); + + it('should consider "true" enabled', () => { + vi.stubEnv('CP_VERBOSE', 'true'); + expect(isEnvVarEnabled('CP_VERBOSE')).toBe(true); + }); + + it('should consider "false" disabled', () => { + vi.stubEnv('CP_VERBOSE', 'false'); + expect(isEnvVarEnabled('CP_VERBOSE')).toBe(false); + }); + + it('should consider "1" enabled', () => { + vi.stubEnv('CP_VERBOSE', '1'); + expect(isEnvVarEnabled('CP_VERBOSE')).toBe(true); + }); + + it('should consider "0" disabled', () => { + vi.stubEnv('CP_VERBOSE', '0'); + expect(isEnvVarEnabled('CP_VERBOSE')).toBe(false); + }); + + it('should log a warning for unexpected values', () => { + vi.stubEnv('CP_VERBOSE', 'unexpected'); + expect(isEnvVarEnabled('CP_VERBOSE')).toBe(false); + expect(ui()).toHaveLogged( + 'warn', + 'Environment variable CP_VERBOSE expected to be a boolean (true/false/1/0), but received value unexpected. Treating it as disabled.', + ); + }); +}); + +describe('coerceBooleanValue', () => { + it.each([ + [true, true], + [false, false], + ['true', true], + ['false', false], + ['True', true], + ['False', false], + ['TRUE', true], + ['FALSE', false], + ['on', true], + ['off', false], + ['yes', true], + ['no', false], + ['1', true], + ['0', false], + ['42', true], + ['unknown', undefined], + [null, undefined], + [undefined, undefined], + ])('should coerce value %j to %j', (input, expected) => { + expect(coerceBooleanValue(input)).toBe(expected); + }); +}); diff --git a/packages/utils/src/lib/execute-process.ts b/packages/utils/src/lib/execute-process.ts index 9e7fdfad6..d1fa98a3f 100644 --- a/packages/utils/src/lib/execute-process.ts +++ b/packages/utils/src/lib/execute-process.ts @@ -6,8 +6,9 @@ import { spawn, } from 'node:child_process'; import type { Readable, Writable } from 'node:stream'; +import { isVerbose } from './env.js'; import { formatCommandLog } from './format-command-log.js'; -import { isVerbose, ui } from './logging.js'; +import { ui } from './logging.js'; import { calcDuration } from './reports/utils.js'; /** diff --git a/packages/utils/src/lib/logging.ts b/packages/utils/src/lib/logging.ts index f9f88ab7e..ca344fc05 100644 --- a/packages/utils/src/lib/logging.ts +++ b/packages/utils/src/lib/logging.ts @@ -19,8 +19,6 @@ export type Column = { }; export type CliUi = CliUiBase & CliExtension; -export const isVerbose = () => process.env['CP_VERBOSE'] === 'true'; - // eslint-disable-next-line functional/no-let let cliUISingleton: CliUiBase | undefined; // eslint-disable-next-line functional/no-let diff --git a/packages/utils/src/lib/reports/log-stdout-summary.ts b/packages/utils/src/lib/reports/log-stdout-summary.ts index 0b4282ab6..4e7b481a9 100644 --- a/packages/utils/src/lib/reports/log-stdout-summary.ts +++ b/packages/utils/src/lib/reports/log-stdout-summary.ts @@ -1,6 +1,7 @@ import { bold, cyan, cyanBright, green, red } from 'ansis'; import type { AuditReport } from '@code-pushup/models'; -import { isVerbose, ui } from '../logging.js'; +import { isVerbose } from '../env.js'; +import { ui } from '../logging.js'; import { CODE_PUSHUP_DOMAIN, FOOTER_PREFIX,