Skip to content

Commit

Permalink
Rework run function for bash tests
Browse files Browse the repository at this point in the history 
There is a tension between running tests and running linters. The fix
previously worked great for linters, but failed hard for tests. This
fixes both paths.

We now have a run_linter command that turns out the ability to not have
to fail hard when you encounter an error, but run will continue to do
so.
  • Loading branch information
@SimonRichardson
SimonRichardson committed Oct 14, 2020
1 parent bfc8c5f commit 358ac9f
Show file tree
Hide file tree
Showing 15 changed files with 74 additions and 62 deletions.
2 changes: 1 addition & 1 deletion pki/tls/sni.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type Logger interface {
// supplied authority that best matches the client hellow message.
func AuthoritySNITLSGetter(authority pki.Authority, logger Logger) func(*tls.ClientHelloInfo) (*tls.Certificate, error) {
return func(hello *tls.ClientHelloInfo) (*tls.Certificate, error) {
logger.Debugf("recieved tls client hello for server name %s", hello.ServerName)
logger.Debugf("received tls client hello for server name %s", hello.ServerName)
var cert *tls.Certificate
authority.LeafRange(func(leaf pki.Leaf) bool {
if err := hello.SupportsCertificate(leaf.TLSCertificate()); err == nil {
Expand Down
12 changes: 8 additions & 4 deletions tests/includes/check.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,20 @@ check() {

got=
while read -r d; do
got="${got}\n${d}"
if [ -z "${got}" ]; then
got="${d}"
else
got="${got}\n${d}"
fi
done

OUT=$(echo "${got}" | grep -E "${want}" || true)
if [ -z "${OUT}" ]; then
OUT=$(echo "${got}" | grep -E "${want}" || echo "(NOT FOUND)")
if [ "${OUT}" == "(NOT FOUND)" ]; then
echo "" >&2
# shellcheck disable=SC2059
printf "$(red \"Expected\"): ${want}\n" >&2
# shellcheck disable=SC2059
printf "$(red \"Recieved\"): ${got}\n" >&2
printf "$(red \"Received\"): ${got}\n" >&2
echo "" >&2
exit 1
fi
Expand Down
15 changes: 9 additions & 6 deletions tests/includes/run.sh
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# run a command and immediately terminate the script when any error occurs.
run() {
CMD="${1}"

Expand All @@ -15,13 +16,8 @@ run() {

START_TIME=$(date +%s)

# Prevent command from killing the script so we can capture its exit code
# AND output. Also, make sure to grab both STDOUT and STDERR. We should be
# using set -o pipefail here but that's unfortunately not supported by the shell.
set +e
cmd_output=$("${CMD}" "$@" 2>&1)
"${CMD}" "$@" 2>&1| OUTPUT "${TEST_DIR}/${TEST_CURRENT}.log"
cmd_status=$?
echo -e "$cmd_output" | OUTPUT "${TEST_DIR}/${TEST_CURRENT}.log"

set_verbosity
END_TIME=$(date +%s)
Expand All @@ -34,6 +30,13 @@ run() {
fi
}

# run_linter will run until the end of a pipeline even if there is a failure.
# This is different from `run` as we require the output of a linter.
run_linter() {
set +e
run "$@"
}

skip() {
CMD="${1}"
# shellcheck disable=SC2143,SC2046
Expand Down
13 changes: 9 additions & 4 deletions tests/includes/verbose.sh
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
set_verbosity() {
# There are three levels of verbosity, both 1 and 2 will fail on any error,
# the difference is that 2 will also turn juju debug statements on, but not
# the shell debug statements. Turning to 3, turns everything on, in theory
# we could also turn on trace statements for juju.
# the shell debug statements.
case "${VERBOSE}" in
1)
set -eu
set -euo pipefail
set +x
;;
2)
set -eu
set -euo pipefail
set +x
;;
11)
# You asked for it!
set -euxo pipefail
;;
*)
echo "Unexpected verbose level" >&2
Expand Down
28 changes: 14 additions & 14 deletions tests/main.sh
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/sh -e
#!/bin/bash -e
[ -n "${GOPATH:-}" ] && export "PATH=${GOPATH}/bin:${PATH}"

# Always ignore SC2230 ('which' is non-standard. Use builtin 'command -v' instead.)
Expand Down Expand Up @@ -77,16 +77,16 @@ show_help() {
echo ""
echo "cmd [-h] [-v] [-A] [-s test] [-a file] [-x file] [-r] [-l controller] [-p provider type <lxd|aws|manual|microk8s>]"
echo ""
echo " $(green 'cmd -h') Display this help message"
echo " $(green 'cmd -v') Verbose and debug messages"
echo " $(green 'cmd -A') Run all the test suites"
echo " $(green 'cmd -s') Skip tests using a comma seperated list"
echo " $(green 'cmd -a') Create an artifact file"
echo " $(green 'cmd -x') Output file from streaming the output"
echo " $(green 'cmd -r') Reuse bootstrapped controller between testing suites"
echo " $(green 'cmd -l') Local bootstrapped controller name to reuse"
echo " $(green 'cmd -p') Bootstrap provider to use when bootstrapping <lxd|aws|manual|microk8s>"
echo " $(green 'cmd -S') Bootstrap series to use <default is host>, priority over -l"
echo " $(green './main.sh -h') Display this help message"
echo " $(green './main.sh -v') Verbose and debug messages"
echo " $(green './main.sh -A') Run all the test suites"
echo " $(green './main.sh -s') Skip tests using a comma seperated list"
echo " $(green './main.sh -a') Create an artifact file"
echo " $(green './main.sh -x') Output file from streaming the output"
echo " $(green './main.sh -r') Reuse bootstrapped controller between testing suites"
echo " $(green './main.sh -l') Local bootstrapped controller name to reuse"
echo " $(green './main.sh -p') Bootstrap provider to use when bootstrapping <lxd|aws|manual|microk8s>"
echo " $(green './main.sh -S') Bootstrap series to use <default is host>, priority over -l"
echo ""
echo "Tests:"
echo "¯¯¯¯¯¯"
Expand All @@ -107,17 +107,17 @@ show_help() {
echo "¯¯¯¯¯¯¯¯¯"
echo "Run a singular test:"
echo ""
echo " $(green 'cmd static_analysis test_static_analysis_go')"
echo " $(green './main.sh static_analysis test_static_analysis_go')"
echo ""
echo "Run static analysis tests, but skip the go static analysis tests:"
echo ""
echo " $(green 'cmd -s test_static_analysis_go static_analysis')"
echo " $(green './main.sh -s test_static_analysis_go static_analysis')"
echo ""
echo "Run a more verbose output and save that to an artifact tar (it"
echo "requires piping the output from stdout and stderr into a output.log,"
echo "which is then copied into the artifact tar file on test cleanup):"
echo ""
echo " $(green 'cmd -V -a artifact.tar.gz -x output.log 2>&1|tee output.log')"
echo " $(green './main.sh -V -a artifact.tar.gz -x output.log 2>&1|tee output.log')"
exit 1
}

Expand Down
2 changes: 1 addition & 1 deletion tests/suites/examples/example.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,4 @@ test_example() {
run "run_example1"
run "run_example2"
)
}
}
2 changes: 1 addition & 1 deletion tests/suites/examples/other.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,4 @@ test_other() {

run "run_other"
)
}
}
2 changes: 1 addition & 1 deletion tests/suites/examples/task.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@ test_examples() {
test_other

destroy_controller "test-example"
}
}
4 changes: 2 additions & 2 deletions tests/suites/static_analysis/copyright.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ run_copyright() {
if [ "$LINES" != 0 ]; then
echo ""
echo "$(red 'Found some issues:')"
echo "\\nThe following files are missing copyright headers"
echo -e "\\nThe following files are missing copyright headers"
echo "${OUT}"
exit 1
fi
Expand All @@ -22,6 +22,6 @@ test_copyright() {
cd .. || exit

# Check for copyright notices
run "run_copyright"
run_linter "run_copyright"
)
}
42 changes: 21 additions & 21 deletions tests/suites/static_analysis/lint_go.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ run_go_vet() {
if [ -n "${OUT}" ]; then
echo ""
echo "$(red 'Found some issues (go vet):')"
echo "\\n${OUT}" >&2
echo -e "\\n${OUT}" >&2
exit 1
fi
}
Expand All @@ -17,7 +17,7 @@ run_go_lint() {
if [ -n "${OUT}" ]; then
echo ""
echo "$(red 'Found some issues (go lint):')"
echo "\\n${OUT}" >&2
echo -e "\\n${OUT}" >&2
exit 1
fi
}
Expand All @@ -32,7 +32,7 @@ run_go_imports() {
echo ""
echo "$(red 'Found some issues (go imports):')"
for ITEM in ${GOFILES}; do
echo "\\ngoimports -w ${ITEM}" >&2
echo -e "\\ngoimports -w ${ITEM}" >&2
done
exit 1
fi
Expand All @@ -45,19 +45,19 @@ run_deadcode() {
if [ -n "${OUT}" ]; then
echo ""
echo "$(red 'Found some issues (deadcode):')"
echo "\\n${OUT}" >&2
echo -e "\\n${OUT}" >&2
exit 1
fi
}

run_misspell() {
FILES="${2}"
# shellcheck disable=SC2086
OUT=$(misspell -source=go 2>/dev/null ${FILES} || true)
OUT=$(misspell -i=gratuitious ${FILES} 2>/dev/null || true)
if [ -n "${OUT}" ]; then
echo ""
echo "$(red 'Found some issues (misspell):')"
echo "\\n${OUT}" >&2
echo -e "\\n${OUT}" >&2
exit 1
fi
}
Expand All @@ -69,7 +69,7 @@ run_unconvert() {
if [ -n "${OUT}" ]; then
echo ""
echo "$(red 'Found some issues (unconvert):')"
echo "\\n${OUT}" >&2
echo -e "\\n${OUT}" >&2
exit 1
fi
}
Expand All @@ -81,7 +81,7 @@ run_ineffassign() {
if [ -n "${OUT}" ]; then
echo ""
echo "$(red 'Found some issues (ineffassign):')"
echo "\\n${OUT}" >&2
echo -e "\\n${OUT}" >&2
exit 1
fi
}
Expand All @@ -94,7 +94,7 @@ run_go_fmt() {
echo ""
echo "$(red 'Found some issues (gofmt):')"
for ITEM in ${OUT}; do
echo "\\ngofmt -s -w ${ITEM}" >&2
echo -e "\\ngofmt -s -w ${ITEM}" >&2
done
exit 1
fi
Expand All @@ -105,7 +105,7 @@ run_go_tidy() {
go mod tidy 2>&1
NEW_SHA=$(cat go.sum | shasum -a 1 | awk '{ print $1 }')
if [ "${CUR_SHA}" != "${NEW_SHA}" ]; then
(>&2 echo "\\nError: go mod sum is out of sync. Run 'go mod tidy' and commit source.")
(>&2 echo -e "\\nError: go mod sum is out of sync. Run 'go mod tidy' and commit source.")
exit 1
fi
}
Expand All @@ -127,11 +127,11 @@ test_static_analysis_go() {

## Functions starting by empty line
# turned off until we get approval of test suite
# run "func vet"
# run_linter "func vet"

## go vet, if it exists
if go help vet >/dev/null 2>&1; then
run "run_go_vet" "${PACKAGES}"
run_linter "run_go_vet" "${PACKAGES}"
else
echo "vet not found, vet static analysis disabled"
fi
Expand All @@ -140,14 +140,14 @@ test_static_analysis_go() {

## golint
#if which golint >/dev/null 2>&1; then
# run "run_go_lint" "${PACKAGES}"
# run_linter "run_go_lint" "${PACKAGES}"
#else
# echo "golint not found, golint static analysis disabled"
#fi

## goimports
if which goimports >/dev/null 2>&1; then
run "run_go_imports" "${FOLDERS}"
run_linter "run_go_imports" "${FOLDERS}"
else
echo "goimports not found, goimports static analysis disabled"
fi
Expand All @@ -157,34 +157,34 @@ test_static_analysis_go() {

## deadcode
#if which deadcode >/dev/null 2>&1; then
# run "run_deadcode" "${FOLDERS}"
# run_linter "run_deadcode" "${FOLDERS}"
#else
# echo "deadcode not found, deadcode static analysis disabled"
#fi

## misspell
if which misspell >/dev/null 2>&1; then
run "run_misspell" "${FILES}"
run_linter "run_misspell" "${FILES}"
else
echo "misspell not found, misspell static analysis disabled"
echo "misspell not found, misspell static analysis disabled"
fi

## unconvert
if which unconvert >/dev/null 2>&1; then
run "run_unconvert" "${PACKAGES}"
run_linter "run_unconvert" "${PACKAGES}"
else
echo "unconvert not found, unconvert static analysis disabled"
fi

## ineffassign
if which ineffassign >/dev/null 2>&1; then
run "run_ineffassign" "${FOLDERS}"
run_linter "run_ineffassign" "${FOLDERS}"
else
echo "ineffassign not found, ineffassign static analysis disabled"
fi

## go fmt
run "run_go_fmt" "${FILES}"
run "run_go_tidy"
run_linter "run_go_fmt" "${FILES}"
run_linter "run_go_tidy"
)
}
2 changes: 1 addition & 1 deletion tests/suites/static_analysis/lint_python.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ test_static_analysis_python() {

# Shell static analysis
if which python3 >/dev/null 2>&1; then
run "run_compileall"
run_linter "run_compileall"
else
echo "python3 not found, python static analysis disabled"
fi
Expand Down
6 changes: 3 additions & 3 deletions tests/suites/static_analysis/lint_shell.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,15 @@ test_static_analysis_shell() {

# Shell static analysis
if which shellcheck >/dev/null 2>&1; then
run "run_shellcheck"
run_linter "run_shellcheck"
else
echo "shellcheck not found, shell static analysis disabled"
fi

## Mixed tabs/spaces in scripts
run "run_whitespace"
run_linter "run_whitespace"

## Trailing whitespace in scripts
run "run_trailing_whitespace"
run_linter "run_trailing_whitespace"
)
}
2 changes: 1 addition & 1 deletion tests/suites/static_analysis/schema.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,6 @@ test_schema() {
cd .. || exit

# Check for schema changes and ensure they've been committed
run "run_schema"
run_linter "run_schema"
)
}
2 changes: 1 addition & 1 deletion worker/apiaddressupdater/apiaddressupdater.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func (c *APIAddressUpdater) Handle(_ <-chan struct{}) error {

// Logging to identify lp: 1888453
if len(hps) == 0 {
c.config.Logger.Warningf("empty API host ports recieved. Updating using existing entries.")
c.config.Logger.Warningf("empty API host ports received. Updating using existing entries.")
}

c.config.Logger.Debugf("updating API hostPorts to %+v", hps)
Expand Down
2 changes: 1 addition & 1 deletion worker/modelworkermanager/manifold.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (config ManifoldConfig) Validate() error {
return errors.NotValidf("empty StateName")
}
if config.MuxName == "" {
return errors.NotValidf("emtpy MuxName")
return errors.NotValidf("empty MuxName")
}
if config.NewWorker == nil {
return errors.NotValidf("nil NewWorker")
Expand Down

0 comments on commit 358ac9f

Please sign in to comment.