Skip to content

Commit

Permalink
Speed up and make happo-ci more robust when HAPPO_IS_ASYNC
Browse files Browse the repository at this point in the history
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:
actions/checkout#27
  • Loading branch information
trotzig committed Jun 8, 2020
1 parent 1b8f5f0 commit fc37835
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 27 deletions.
51 changes: 28 additions & 23 deletions bin/happo-ci
Original file line number Diff line number Diff line change
Expand Up @@ -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"}

Expand Down Expand Up @@ -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)"
Expand Down Expand Up @@ -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.
Expand Down
27 changes: 23 additions & 4 deletions test/happo-ci-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 [];
Expand Down Expand Up @@ -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 <tom@dooner.com>',
]);
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', () => {
Expand Down

0 comments on commit fc37835

Please sign in to comment.