From 3dda4ab35c89f805d156bf07ff0bd350d2827fd9 Mon Sep 17 00:00:00 2001 From: William Chargin Date: Wed, 5 Sep 2018 12:47:54 -0700 Subject: [PATCH] test: invoke `yarn backend` only once (#784) Summary: Lots of tests need the output of `yarn backend`. Before this commit, they tended to create it themselves. This was slow and wasteful, and also could in principle have race conditions (though in practice usually tended not to). This commit updates tests to respect a `SOURCECRED_BIN` environment variable indicating the path to an existing directory of backend applications. Closes #765. Test Plan: Running `yarn test --full` passes. Prepending `echo run >>/tmp/log &&` to the `backend` script in `package.json` and running `yarn test --full` results in a log file containing only one line, indicating that the script really is run only once. wchargin-branch: deduplicate-backend --- config/test.js | 59 +++++++++++++---------- scripts/build_static_site.sh | 37 ++++++++++---- sharness/test_build_static_site.t | 4 +- src/plugins/git/loadRepositoryTest.sh | 16 +++++- src/plugins/github/fetchGithubRepoTest.sh | 13 ++++- 5 files changed, 89 insertions(+), 40 deletions(-) diff --git a/config/test.js b/config/test.js index 1c7be95..453c838 100644 --- a/config/test.js +++ b/config/test.js @@ -18,6 +18,18 @@ function main() { } function makeTasks(mode /*: "BASIC" | "FULL" */) { + const backendOutput = tmp.dirSync({ + unsafeCleanup: true, + prefix: "sourcecred-test-", + }).name; + console.log("tmpdir for backend output: " + backendOutput); + + function withSourcecredBinEnv( + invocation /*: $ReadOnlyArray */ + ) /*: string[] */ { + return ["env", "SOURCECRED_BIN=" + backendOutput, ...invocation]; + } + const basicTasks = [ { id: "ensure-flow-typing", @@ -59,16 +71,6 @@ function makeTasks(mode /*: "BASIC" | "FULL" */) { cmd: ["npm", "run", "--silent", "unit"], deps: [], }, - { - id: {BASIC: "sharness", FULL: "sharness-full"}[mode], - cmd: [ - "npm", - "run", - "--silent", - {BASIC: "sharness", FULL: "sharness-full"}[mode], - ], - deps: [], - }, { id: "backend", cmd: [ @@ -78,32 +80,37 @@ function makeTasks(mode /*: "BASIC" | "FULL" */) { "backend", "--", "--output-path", - tmp.dirSync({ - unsafeCleanup: true, - prefix: "sourcecred-backend-dry-run-", - }).name, + backendOutput, ], deps: [], }, + { + id: {BASIC: "sharness", FULL: "sharness-full"}[mode], + cmd: withSourcecredBinEnv([ + "npm", + "run", + "--silent", + {BASIC: "sharness", FULL: "sharness-full"}[mode], + ]), + deps: ["backend"], + }, ]; const extraTasks = [ - { - id: "backend-in-place", - cmd: ["npm", "run", "--silent", "backend"], - // This task depends on `check-pretty` in order to work around a - // race condition in Prettier: - // https://github.com/prettier/prettier/issues/4468 - deps: ["check-pretty"], - }, { id: "fetchGithubRepoTest", - cmd: ["./src/plugins/github/fetchGithubRepoTest.sh", "--no-build"], - deps: ["backend-in-place"], + cmd: withSourcecredBinEnv([ + "./src/plugins/github/fetchGithubRepoTest.sh", + "--no-build", + ]), + deps: ["backend"], }, { id: "loadRepositoryTest", - cmd: ["./src/plugins/git/loadRepositoryTest.sh", "--no-build"], - deps: ["backend-in-place"], + cmd: withSourcecredBinEnv([ + "./src/plugins/git/loadRepositoryTest.sh", + "--no-build", + ]), + deps: ["backend"], }, ]; switch (mode) { diff --git a/scripts/build_static_site.sh b/scripts/build_static_site.sh index e9bb334..ee23e76 100755 --- a/scripts/build_static_site.sh +++ b/scripts/build_static_site.sh @@ -6,6 +6,7 @@ usage() { printf ' [--repo OWNER/NAME [...]]\n' printf ' [--feedback-url URL]\n' printf ' [--cname DOMAIN]\n' + printf ' [--no-backend]\n' printf ' [-h|--help]\n' printf '\n' printf 'Build the static SourceCred website, including example data.\n' @@ -20,8 +21,18 @@ usage() { printf '%s\n' '--cname DOMAIN' printf '\t%s\n' 'configure DNS for a GitHub Pages site to point to' printf '\t%s\n' 'the provided custom domain' + printf '%s\n' '--no-backend' + printf '\t%s\n' 'do not run "yarn backend"; see also the SOURCECRED_BIN' + printf '\t%s\n' 'environment variable' printf '%s\n' '-h|--help' printf '\t%s\n' 'show this message' + printf '\n' + printf 'Environment variables:\n' + printf '\n' + printf '%s\n' 'SOURCECRED_BIN' + printf '\t%s\n' 'When using --no-backend, directory containing the' + printf '\t%s\n' 'SourceCred executables (output of "yarn backend").' + printf '\t%s\n' 'Default is ./bin. Ignored without --no-backend.' } main() { @@ -31,6 +42,7 @@ main() { cd "${toplevel}" sourcecred_data= + sourcecred_bin= trap cleanup EXIT build @@ -38,6 +50,7 @@ main() { parse_args() { unset SOURCECRED_FEEDBACK_URL + BACKEND=1 target= cname= repos=( ) @@ -78,6 +91,9 @@ parse_args() { die 'empty value for --cname' fi ;; + --no-backend) + BACKEND=0 + ;; -h|--help) usage exit 0 @@ -102,24 +118,26 @@ parse_args() { die "target directory is nonempty: ${target}" fi target="$(readlink -e "${target}")" + : "${SOURCECRED_BIN:=./bin}" } build() { sourcecred_data="$(mktemp -d --suffix ".sourcecred-data")" export SOURCECRED_DIRECTORY="${sourcecred_data}" - yarn - # shellcheck disable=SC2016 - printf >&2 'warn: running `yarn backend`, overwriting `bin/` in your repo\n' - printf >&2 'warn: if this offends you, please see: %s\n' \ - 'https://github.com/sourcecred/sourcecred/issues/580' - yarn backend - yarn build --output-path "${target}" + if [ "${BACKEND}" -ne 0 ]; then + sourcecred_bin="$(mktemp -d --suffix ".sourcecred-bin")" + export SOURCECRED_BIN="${sourcecred_bin}" + yarn + yarn -s backend --output-path "${SOURCECRED_BIN}" + fi + yarn -s build --output-path "${target}" if [ "${#repos[@]}" -ne 0 ]; then for repo in "${repos[@]}"; do printf >&2 'info: loading repository: %s\n' "${repo}" - node ./bin/sourcecred.js load "${repo}" + NODE_PATH="./node_modules${NODE_PATH:+:${NODE_PATH}}" \ + node "${SOURCECRED_BIN:-./bin}/sourcecred.js" load "${repo}" done fi @@ -147,7 +165,8 @@ build() { } cleanup() { - if [ -d "${sourcecred_data}" ]; then rm -rf "${sourcecred_data}"; fi + if [ -d "${sourcecred_data:-}" ]; then rm -rf "${sourcecred_data}"; fi + if [ -d "${sourcecred_bin:-}" ]; then rm -rf "${sourcecred_bin}"; fi } die() { diff --git a/sharness/test_build_static_site.t b/sharness/test_build_static_site.t index 581955a..f498165 100755 --- a/sharness/test_build_static_site.t +++ b/sharness/test_build_static_site.t @@ -152,8 +152,6 @@ run_build() { run '"${flags}"' 2>err && test_must_fail grep -vF \ -e "Removing contents of build directory: " \ - -e "warn: running \`yarn backend\`" \ - -e "warn: if this offends you" \ -e "info: loading repository" \ err && test_path_is_dir "${output_dir}" && @@ -215,6 +213,7 @@ test_pages() { run_build TWO_REPOS \ "should build the site with two repositories and a CNAME" \ + --no-backend \ --cname sourcecred.example.com \ --feedback-url http://discuss.example.com/feedback/ \ --repo sourcecred/example-git \ @@ -255,6 +254,7 @@ test_expect_success TWO_REPOS "TWO_REPOS: should have a correct CNAME record" ' SOURCECRED_FEEDBACK_URL=http://wat.com/wat \ run_build NO_REPOS \ "should build the site with no repositories and no CNAME" \ + --no-backend \ # no arguments here test_pages NO_REPOS diff --git a/src/plugins/git/loadRepositoryTest.sh b/src/plugins/git/loadRepositoryTest.sh index f156b64..1f7cf70 100755 --- a/src/plugins/git/loadRepositoryTest.sh +++ b/src/plugins/git/loadRepositoryTest.sh @@ -14,12 +14,17 @@ usage() { printf ' Default is --build.\n' printf ' --help\n' printf ' Show this message\n' + printf '\n' + printf 'Environment variables:' + printf ' SOURCECRED_BIN\n' + printf ' When using --no-build, directory containing the SourceCred\n' + printf ' executables (output of "yarn backend"). Default is ./bin.\n' } fetch() { tmpdir="$(mktemp -d)" - node bin/createExampleRepo.js "${tmpdir}" - node bin/loadAndPrintGitRepository.js "${tmpdir}" + node "${SOURCECRED_BIN:-./bin}/createExampleRepo.js" "${tmpdir}" + node "${SOURCECRED_BIN:-./bin}/loadAndPrintGitRepository.js" "${tmpdir}" rm -rf "${tmpdir}" } @@ -35,6 +40,10 @@ update() { } main() { + if [ -n "${SOURCECRED_BIN:-}" ]; then + SOURCECRED_BIN="$(readlink -f "${SOURCECRED_BIN}")" + fi + cd "$(git rev-parse --show-toplevel)" UPDATE= BUILD=1 while [ $# -gt 0 ]; do @@ -54,7 +63,10 @@ main() { shift done if [ -n "${BUILD}" ]; then + unset SOURCECRED_BIN yarn backend + else + export NODE_PATH=./node_modules"${NODE_PATH:+:${NODE_PATH}}" fi if [ -n "${UPDATE}" ]; then update diff --git a/src/plugins/github/fetchGithubRepoTest.sh b/src/plugins/github/fetchGithubRepoTest.sh index be84c26..8870cfe 100755 --- a/src/plugins/github/fetchGithubRepoTest.sh +++ b/src/plugins/github/fetchGithubRepoTest.sh @@ -16,6 +16,11 @@ usage() { printf ' Default is --build.\n' printf ' --help\n' printf ' Show this message\n' + printf '\n' + printf 'Environment variables:' + printf ' SOURCECRED_BIN\n' + printf ' When using --no-build, directory containing the SourceCred\n' + printf ' executables (output of "yarn backend"). Default is ./bin.\n' } fetch() { @@ -24,7 +29,7 @@ fetch() { printf >&2 'to a 40-character hex string API token from GitHub.\n' return 1 fi - node bin/fetchAndPrintGithubRepo.js \ + node "${SOURCECRED_BIN:-./bin}/fetchAndPrintGithubRepo.js" \ sourcecred example-github "${GITHUB_TOKEN}" } @@ -40,6 +45,9 @@ update() { } main() { + if [ -n "${SOURCECRED_BIN:-}" ]; then + SOURCECRED_BIN="$(readlink -f "${SOURCECRED_BIN}")" + fi cd "$(git rev-parse --show-toplevel)" UPDATE= BUILD=1 @@ -60,7 +68,10 @@ main() { shift done if [ -n "${BUILD}" ]; then + unset SOURCECRED_BIN yarn backend + else + export NODE_PATH="./node_modules${NODE_PATH:+:${NODE_PATH}}" fi if [ -n "${UPDATE}" ]; then update