From fc3783535935c08757b702cab1d5ee98dda5d8b2 Mon Sep 17 00:00:00 2001 From: Henric Trotzig Date: Mon, 8 Jun 2020 11:16:26 +0200 Subject: [PATCH] Speed up and make happo-ci more robust when HAPPO_IS_ASYNC Support for the HAPPO_IS_ASYNC env variable was recently added to happo.io. So far, it's only been affecting `happo run` and `happo compare`, but I'm making it affect `happo-ci` as well. In async mode, we don't need to check out the previous commit. This should help resolve issues with running happo in github actions, where (by default) the current sha is not the tip of the PR. Instead, it's a merge commit to the current master. Some context here: https://github.com/actions/checkout/issues/27 --- bin/happo-ci | 51 ++++++++++++++++++++++++------------------- test/happo-ci-test.js | 27 +++++++++++++++++++---- 2 files changed, 51 insertions(+), 27 deletions(-) diff --git a/bin/happo-ci b/bin/happo-ci index e1a4efa..ef458cf 100755 --- a/bin/happo-ci +++ b/bin/happo-ci @@ -8,6 +8,7 @@ set -eou pipefail # Initialize optional env variables INSTALL_CMD=${INSTALL_CMD:-} +HAPPO_IS_ASYNC=${HAPPO_IS_ASYNC:-} HAPPO_GIT_COMMAND=${HAPPO_GIT_COMMAND:-git} HAPPO_COMMAND=${HAPPO_COMMAND:-"node_modules/happo.io/build/cli.js"} @@ -48,15 +49,17 @@ run-install() { run-happo() { SHA=$1 - HEAD_SHA=$(${HAPPO_GIT_COMMAND} rev-parse HEAD) RUN_HAPPO_CHECK="false" - if [ "$HEAD_SHA" != "$SHA" ]; then - RUN_HAPPO_CHECK="true" - ${HAPPO_GIT_COMMAND} checkout --force --quiet "$SHA" - # Install dependencies (again, since we're in a different place in git - # history) - run-install + if [ -z "$HAPPO_IS_ASYNC" ]; then + HEAD_SHA=$(${HAPPO_GIT_COMMAND} rev-parse HEAD) + if [ "$HEAD_SHA" != "$SHA" ]; then + RUN_HAPPO_CHECK="true" + ${HAPPO_GIT_COMMAND} checkout --force --quiet "$SHA" + # Install dependencies (again, since we're in a different place in git + # history) + run-install + fi fi COMMIT_SUBJECT="$(${HAPPO_GIT_COMMAND} show -s --format=%s)" @@ -87,23 +90,25 @@ COMMIT_AUTHOR="$(${HAPPO_GIT_COMMAND} show -s --format=%ae)" run-happo "$CURRENT_SHA" -# Check if we need to generate a baseline. In some cases, the baseline is -# already there (some other PR uploaded it), and we can just use the existing -# one. -if ! "$HAPPO_COMMAND" has-report "$PREVIOUS_SHA"; then - echo "No previous report found for ${PREVIOUS_SHA}. Generating one..." - run-happo "$PREVIOUS_SHA" - - # Restore git and node_modules to their original state so that we can run - # `happo compare` on CURRENT_SHA instead of PREVIOUS_SHA. - "$HAPPO_GIT_COMMAND" checkout --force --quiet "$CURRENT_SHA" - run-install -else - echo "Re-using existing report for ${PREVIOUS_SHA}." -fi +if [ -z "$HAPPO_IS_ASYNC" ]; then + # Check if we need to generate a baseline. In some cases, the baseline is + # already there (some other PR uploaded it), and we can just use the existing + # one. + if ! "$HAPPO_COMMAND" has-report "$PREVIOUS_SHA"; then + echo "No previous report found for ${PREVIOUS_SHA}. Generating one..." + run-happo "$PREVIOUS_SHA" + + # Restore git and node_modules to their original state so that we can run + # `happo compare` on CURRENT_SHA instead of PREVIOUS_SHA. + "$HAPPO_GIT_COMMAND" checkout --force --quiet "$CURRENT_SHA" + run-install + else + echo "Re-using existing report for ${PREVIOUS_SHA}." + fi -if [ "$FIRST_RUN" = "true" ]; then - "$HAPPO_COMMAND" empty "$PREVIOUS_SHA" + if [ "$FIRST_RUN" = "true" ]; then + "$HAPPO_COMMAND" empty "$PREVIOUS_SHA" + fi fi # Compare reports from the two SHAs. diff --git a/test/happo-ci-test.js b/test/happo-ci-test.js index ed7523f..c0fa68f 100644 --- a/test/happo-ci-test.js +++ b/test/happo-ci-test.js @@ -11,10 +11,7 @@ let env; function getLog(file) { try { - return fs - .readFileSync(file, 'utf-8') - .split('\n') - .filter(Boolean); + return fs.readFileSync(file, 'utf-8').split('\n').filter(Boolean); } catch (e) { if (e.code === 'ENOENT') { return []; @@ -145,6 +142,28 @@ describe('when there is no report for PREVIOUS_SHA', () => { 'checkout --force --quiet bar', ]); }); + + describe('when HAPPO_IS_ASYNC', () => { + beforeEach(() => { + env.HAPPO_IS_ASYNC = 'true'; + }); + + it('does not checkout anything, runs a single report', () => { + subject(); + expect(getCliLog()).toEqual([ + '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 ', + ]); + expect(getGitLog()).toEqual([ + 'rev-parse no-report', + 'rev-parse bar', + 'show -s --format=%s', + 'show -s --format=%ae', + 'show -s --format=%s', + ]); + }); + }); }); describe('when the compare call fails', () => {