Skip to content

Commit

Permalink
check: Start a check script, in Bash
Browse files Browse the repository at this point in the history
This provides the guts of an implementation of #60.

I'd have liked to write this in Dart, rather than in shell.
But when running this script interactively (vs. in CI),
a key ingredient for a good developer experience is that the
child processes like `flutter test` should have their stdout
and stderr connected directly to those of the parent script,
i.e. to the developer's terminal.  Crucially, that lets those
processes know to print their output in a form that takes advantage
of terminal features to get a good interactive experience.
The gold standard for a CLI tool is to have a way to control
that choice directly, but `flutter test` and some other
Dart-based tools lack that capability.

And in Dart, as far as I can tell, there simply is no way to
start a child process that inherits any of the parent's file
handles; instead the child's output will always be wired up to
pipes for the parent to consume.  There's advice for how the
parent can copy the output, chunk by chunk, to its own output:
  dart-lang/sdk#44573
  dart-lang/sdk#5607
  https://stackoverflow.com/questions/72183086/dart-dont-buffer-process-stdout-tell-process-that-it-is-running-from-a-termina

but that doesn't solve the problem of the child's behavior
changing when it sees a pipe instead of a terminal.

The nastiest consequence of this behavior is that the output of
`flutter test` is hundreds of lines long, even for our small codebase,
even when only a single test case fails or none at all. That's
fine in CI, but pretty much intolerable for a development workflow.

So, shell it is.  (Python, or Javascript with Node, or many other
languages would also handle this just fine, but would mean an extra
system of dependencies to manage.)  Specifically, Bash.

---

Fortunately, using Bash does mean we get to reuse the work that's
already gone into the `tools/test` script in zulip-mobile.
This commit introduces a bare-bones version, with most of the
features (notably --diff and its friends) stripped out,
and the test suites replaced with a first two for our codebase.

This version also uses `set -u` and `set -o pipefail`, unlike
the one in zulip-mobile, in addition to `set -e`.
  • Loading branch information
gnprice committed Oct 3, 2023
1 parent e5d1e3b commit 477fe67
Showing 1 changed file with 88 additions and 0 deletions.
88 changes: 88 additions & 0 deletions tools/check
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
#!/usr/bin/env bash

# Careful! `set -e` doesn't do everything you'd think it does. In
# fact, we don't get its benefit in any of the `run_foo` functions.
#
# This is because it has an effect only when it can exit the whole shell.
# (Its full name is `set -o errexit`, and it means "exit" literally.) See:
# https://www.gnu.org/software/bash/manual/bash.html#The-Set-Builtin
#
# When one test suite fails, we want to go on to run the other suites, so
# we use `||` to prevent the whole script from exiting there, and that
# defeats `set -e`.
#
# For now our workaround is to put `|| return` in the `run_foo` just
# after each nontrivial command that isn't the final command in the
# function.
set -euo pipefail


## CLI PARSING

default_suites=(analyze test)
extra_suites=(
)

usage() {
cat >&2 <<EOF
usage: tools/check [SUITE]...
Run our tests.
By default, run ${#default_suites[@]} suite(s):
${default_suites[*]}
and skip ${#extra_suites[@]} suite(s):
${extra_suites[*]}
EOF
exit 2
}

opt_suites=()
while (( $# )); do
case "$1" in
analyze|test)
opt_suites+=("$1"); shift;;
*) usage;;
esac
done

if (( ! "${#opt_suites[@]}" )); then
opt_suites=( "${default_suites[@]}" )
fi


## EXECUTION

rootdir=$(git rev-parse --show-toplevel)
cd "$rootdir"

run_analyze() {
flutter analyze
}

run_test() {
flutter test
}

failed=()
for suite in "${opt_suites[@]}"; do
echo "Running $suite..."
case "$suite" in
analyze) run_analyze ;;
test) run_test ;;
*) echo >&2 "Internal error: unknown suite $suite" ;;
esac || failed+=( "$suite" )
done

if (( ${#failed[@]} )); then
cat >&2 <<EOF
FAILED: ${failed[*]}
To rerun the suites that failed, run:
$ tools/check ${failed[*]}
EOF
exit 1
fi

echo "Passed!"

0 comments on commit 477fe67

Please sign in to comment.