From 68f8f456a056dd0665817f957b698905e097af0a Mon Sep 17 00:00:00 2001 From: Henric Trotzig Date: Tue, 25 Jun 2024 14:08:20 +0200 Subject: [PATCH] Log client version in happo-ci This commit serves two purposes: One is to output the version of the CLI in the happo-ci logs. The other purpose is to catch a misconfigured HAPPO_COMMAND. By attempting to get the version, we can catch errors and output a helpful message instead of failing with a cryptic message of this kind: 2024-06-24T21:04:09.6031947Z /some/absolute/path/node_modules/.bin/happo-ci: line 104: node_modules/happo.io/build/cli.js: No such file or directory --- bin/happo-ci | 10 ++++++++++ test/cli-mock.js | 5 +++++ test/happo-ci-test.js | 18 ++++++++++++++++++ 3 files changed, 33 insertions(+) diff --git a/bin/happo-ci b/bin/happo-ci index 44901c29..1f39ab98 100755 --- a/bin/happo-ci +++ b/bin/happo-ci @@ -40,6 +40,16 @@ echo "HAPPO_FALLBACK_SHAS_COUNT: ${HAPPO_FALLBACK_SHAS_COUNT}" echo "Using Node version: $(node -v)" +set +e +HAPPO_VERSION=$("$HAPPO_COMMAND" version) +if [ $? -eq 0 ]; then + echo "Using happo CLI version: ${HAPPO_VERSION}" +else + echo "Failed to execute ${HAPPO_COMMAND}. Are you running from a subdirectory in the repository perhaps? To resolve this, you could either run from the repository root or set the environment variable HAPPO_COMMAND to override where Happo looks for the the cli.js script." >&2 + exit 1 +fi +set -e + NPM_CLIENT="npm" NPM_CLIENT_FLAGS="--no-save" diff --git a/test/cli-mock.js b/test/cli-mock.js index 0643738c..ae9d9c17 100755 --- a/test/cli-mock.js +++ b/test/cli-mock.js @@ -25,3 +25,8 @@ if (/compare no-report bar/.test(cmd)) { if (/compare fail bar/.test(cmd)) { process.exit(1); } + +if (/version/.test(cmd)) { + console.log('v0.0.999'); + process.exit(0); +} diff --git a/test/happo-ci-test.js b/test/happo-ci-test.js index 46f447da..4dadc519 100644 --- a/test/happo-ci-test.js +++ b/test/happo-ci-test.js @@ -66,6 +66,16 @@ describe('when CURRENT_SHA is missing', () => { }); }); +describe('when HAPPO_COMMAND is pointing to a non-existant file', () => { + beforeEach(() => { + env.HAPPO_COMMAND = './foo/bar.js'; + }); + + it('fails with a message', () => { + expect(subject).toThrow(/Failed to execute \.\/foo\/bar\.js/); + }); +}); + describe('when CHANGE_URL is missing', () => { beforeEach(() => { delete env.CHANGE_URL; @@ -84,6 +94,7 @@ describe('when CURRENT_SHA and PREVIOUS_SHA is the same', () => { it('runs a single report', () => { subject(); expect(getCliLog()).toEqual([ + 'version', 'run bar --link http://foo.bar/ --message Commit message', ]); expect(getGitLog()).toEqual([ @@ -99,6 +110,7 @@ describe('when there is a report for PREVIOUS_SHA', () => { it('runs the right happo commands', () => { subject(); expect(getCliLog()).toEqual([ + 'version', 'start-job foo bar --link http://foo.bar/ --message Commit message', 'run bar --link http://foo.bar/ --message Commit message', 'compare foo bar --link http://foo.bar/ --message Commit message --author Tom Dooner ', @@ -121,6 +133,7 @@ describe('when there is a report for PREVIOUS_SHA', () => { it('runs the right happo commands', () => { subject(); expect(getCliLog()).toEqual([ + 'version', 'start-job foo bar --link http://foo.bar/ --message Commit message', 'run bar --link http://foo.bar/ --message Commit message', 'has-report foo', @@ -146,6 +159,7 @@ describe('when there is a report for PREVIOUS_SHA', () => { it('runs the right happo commands', () => { subject(); expect(getCliLog()).toEqual([ + 'version', 'run bar --link http://foo.bar/ --message Commit message', 'compare foo bar --link http://foo.bar/ --message Commit message --author Tom Dooner ', ]); @@ -220,6 +234,7 @@ describe('when there is no report for PREVIOUS_SHA', () => { it('does not checkout anything, runs a single report', () => { subject(); expect(getCliLog()).toEqual([ + 'version', 'start-job no-report bar --link http://foo.bar/ --message Commit message', 'run bar --link http://foo.bar/ --message Commit message', 'compare no-report bar --link http://foo.bar/ --message Commit message --author Tom Dooner ', @@ -241,6 +256,7 @@ describe('when there is no report for PREVIOUS_SHA', () => { it('runs the right happo commands', () => { subject(); expect(getCliLog()).toEqual([ + 'version', 'start-job no-report bar --link http://foo.bar/ --message Commit message', 'run bar --link http://foo.bar/ --message Commit message', 'has-report no-report', @@ -272,6 +288,7 @@ describe('when the compare call fails', () => { it('fails the script', () => { expect(subject).toThrow(); expect(getCliLog()).toEqual([ + 'version', 'start-job fail bar --link http://foo.bar/ --message Commit message', 'run bar --link http://foo.bar/ --message Commit message', 'compare fail bar --link http://foo.bar/ --message Commit message --author Tom Dooner ', @@ -296,6 +313,7 @@ describe('when happo.io is not installed for the PREVIOUS_SHA and HAPPO_IS_ASYNC it('runs the right happo commands', () => { subject(); expect(getCliLog()).toEqual([ + 'version', 'start-job no-happo bar --link http://foo.bar/ --message Commit message', 'run bar --link http://foo.bar/ --message Commit message', 'has-report no-happo',