diff --git a/packages/cli-exec/src/exec.js b/packages/cli-exec/src/exec.js index 874bc7b82..8d0634590 100644 --- a/packages/cli-exec/src/exec.js +++ b/packages/cli-exec/src/exec.js @@ -21,6 +21,10 @@ export const exec = command('exec', { name: 'partial', description: 'Marks the build as a partial build', parse: () => !!(process.env.PERCY_PARTIAL_BUILD ||= '1') + }, { + name: 'fail-on-error', + description: 'Exit non-zero when Percy fails to start, even if the command succeeds', + parse: () => !!(process.env.PERCY_FAIL_ON_ERROR ||= 'true') }, { name: 'archive-dir', description: 'Save snapshot data to an archive directory for deferred upload', @@ -65,6 +69,12 @@ export const exec = command('exec', { exit(127, `Command not found "${command}"`, false); } + // Tracks a Percy startup failure so `--fail-on-error` can surface it as a non-zero + // exit after the wrapped command has run. Keyed off the thrown error rather than + // scraped error logs, which also carry non-fatal diagnostics (e.g. the error-analysis + // and build-log upload side channels) that must not fail a CI run. + let startError = null; + // attempt to start percy if enabled if (!percy) { log.warn('Percy is disabled'); @@ -101,6 +111,7 @@ export const exec = command('exec', { yield* percy.yield.start(); } catch (error) { if (error.name === 'AbortError') throw error; + startError = error; log.warn('Skipping visual tests'); log.error(error); } @@ -124,9 +135,20 @@ export const exec = command('exec', { await percy?.stop(force); log.info(`Command "${[command, ...args].join(' ')}" exited with status: ${status}`); - // forward any returned status code + // forward any returned status code — the wrapped command's own failure takes + // priority over a Percy startup failure, since it is the more specific signal if (status) exit(status, error, false); + // Opt-in: fail the run when Percy never started, so a CI pipeline does not report + // success for a build that produced no visual coverage. Default behavior is + // unchanged — without the flag the wrapped command's status is still the only + // thing that determines the exit code. + let failOnError = flags.failOnError || process.env.PERCY_FAIL_ON_ERROR === 'true'; + + if (failOnError && startError) { + exit(1, 'Percy failed to start and --fail-on-error was set; no visual tests ran', false); + } + // force exit post timeout await waitForTimeout(10000); process.exit(status); diff --git a/packages/cli-exec/test/exec.test.js b/packages/cli-exec/test/exec.test.js index b29cac633..ba772c4a1 100644 --- a/packages/cli-exec/test/exec.test.js +++ b/packages/cli-exec/test/exec.test.js @@ -212,6 +212,68 @@ describe('percy exec', () => { ])); }); + describe('--fail-on-error', () => { + afterEach(() => { + delete process.env.PERCY_FAIL_ON_ERROR; + }); + + it('exits non-zero when percy fails to start', async () => { + delete process.env.PERCY_TOKEN; + + await expectAsync( + exec(['--fail-on-error', '--', 'node', '--eval', '']) + ).toBeRejectedWithError( + 'Percy failed to start and --fail-on-error was set; no visual tests ran' + ); + + // the wrapped command still runs — only the exit code changes + expect(logger.stdout).toEqual(jasmine.arrayContaining([ + '[percy] Running "node --eval "' + ])); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] Skipping visual tests' + ])); + }); + + it('exits non-zero when PERCY_FAIL_ON_ERROR is set without the flag', async () => { + delete process.env.PERCY_TOKEN; + process.env.PERCY_FAIL_ON_ERROR = 'true'; + + await expectAsync( + exec(['--', 'node', '--eval', '']) + ).toBeRejectedWithError( + 'Percy failed to start and --fail-on-error was set; no visual tests ran' + ); + }); + + it('exits zero when percy starts successfully', async () => { + await exec(['--fail-on-error', '--', 'node', '--eval', '']); + + expect(logger.stdout).toEqual(jasmine.arrayContaining([ + '[percy] Command "node --eval " exited with status: 0' + ])); + }); + + it('forwards the command status ahead of a percy start failure', async () => { + delete process.env.PERCY_TOKEN; + + // the wrapped command's own status is the more specific signal, so it wins + await expectAsync( + exec(['--fail-on-error', '--', 'node', '--eval', 'process.exit(3)']) + ).toBeRejectedWithError('EEXIT: 3'); + }); + + it('does not affect the exit code when the flag is not set', async () => { + delete process.env.PERCY_TOKEN; + + await exec(['--', 'node', '--eval', '']); + + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] Skipping visual tests' + ])); + }); + }); + it('forwards the command status', async () => { await expectAsync( exec(['--', 'node', '--eval', 'process.exit(3)'])