From 4bce87f3cf3536f56da2122a2dca06a70a349e4f Mon Sep 17 00:00:00 2001 From: Peter Adams <63288215+PeterAdams-A@users.noreply.github.com> Date: Wed, 30 Oct 2024 15:22:05 +0000 Subject: [PATCH] Enable ShellCheck CI (#478) Motivation: Complete enablement of all standard GitHub Actions workflows. Modifications: Describe the modifications you've done. Result: After your change, what will change. --- .github/workflows/pull_request.yml | 2 -- FuzzTesting/do_build.sh | 10 +++++----- IntegrationTests/plugin_junit_xml.sh | 6 +++--- IntegrationTests/run-single-test.sh | 5 +++++ IntegrationTests/run-tests.sh | 14 ++++++++------ IntegrationTests/test_functions.sh | 1 + .../test_01_allocation_counts.sh | 6 ++++-- scripts/integration_tests.sh | 2 +- scripts/test_h2spec.sh | 7 ++++--- 9 files changed, 31 insertions(+), 22 deletions(-) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index e4cda5c2..9d622c9a 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -10,8 +10,6 @@ jobs: uses: swiftlang/github-workflows/.github/workflows/soundness.yml@main with: license_header_check_project_name: "SwiftNIO" - format_check_enabled: true - shell_check_enabled: false unit-tests: name: Unit tests diff --git a/FuzzTesting/do_build.sh b/FuzzTesting/do_build.sh index 485fdf4b..4a365257 100755 --- a/FuzzTesting/do_build.sh +++ b/FuzzTesting/do_build.sh @@ -24,7 +24,9 @@ set -eu -readonly FuzzTestingDir=$(dirname "$(echo $0 | sed -e "s,^\([^/]\),$(pwd)/\1,")") +# shellcheck disable=SC2001 # Too complex +FuzzTestingDir=$(dirname "$(echo "$0" | sed -e "s,^\([^/]\),$(pwd)/\1,")") +readonly FuzzTestingDir printUsage() { NAME=$(basename "${0}") @@ -95,13 +97,11 @@ cd "${FuzzTestingDir}" declare -a CMD_BASE if [ "$(uname)" == "Darwin" ]; then CMD_BASE=( - xcrun - --toolchain swift - swift build -Xswiftc -sanitize=fuzzer,address -Xswiftc -parse-as-library + xcrun --toolchain swift swift build -Xswiftc "-sanitize=fuzzer,address" -Xswiftc -parse-as-library ) else CMD_BASE=( - swift build -Xswiftc -sanitize=fuzzer,address -Xswiftc -parse-as-library + swift build -Xswiftc "-sanitize=fuzzer,address" -Xswiftc -parse-as-library ) fi diff --git a/IntegrationTests/plugin_junit_xml.sh b/IntegrationTests/plugin_junit_xml.sh index d88a51c7..f59ea28c 100644 --- a/IntegrationTests/plugin_junit_xml.sh +++ b/IntegrationTests/plugin_junit_xml.sh @@ -44,9 +44,9 @@ function junit_output_replace() { function plugin_junit_xml_test_suite_begin() { junit_testsuite_time=0 - junit_output_write "" } diff --git a/IntegrationTests/run-single-test.sh b/IntegrationTests/run-single-test.sh index 36501029..7ec19d62 100755 --- a/IntegrationTests/run-single-test.sh +++ b/IntegrationTests/run-single-test.sh @@ -20,12 +20,17 @@ set -x set -o pipefail test="$1" +# shellcheck disable=SC2034 # Used by whatever we source transpile in tmp="$2" +# shellcheck disable=SC2034 # Used by whatever we source transpile in root="$3" +# shellcheck disable=SC2034 # Used by whatever we source transpile in g_show_info="$4" here="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" +# shellcheck source=IntegrationTests/test_functions.sh source "$here/test_functions.sh" +# shellcheck source=/dev/null source "$test" wait ) diff --git a/IntegrationTests/run-tests.sh b/IntegrationTests/run-tests.sh index e8ec01dd..33325dbf 100755 --- a/IntegrationTests/run-tests.sh +++ b/IntegrationTests/run-tests.sh @@ -36,7 +36,9 @@ function plugins_do() { done } +# shellcheck source=IntegrationTests/plugin_echo.sh source "$here/plugin_echo.sh" +# shellcheck source=/dev/null source "$here/plugin_junit_xml.sh" plugins="echo" @@ -88,7 +90,7 @@ function run_test() { if $verbose; then "$@" 2>&1 | tee -a "$out" # we need to return the return value of the first command - return ${PIPESTATUS[0]} + return "${PIPESTATUS[0]}" else "$@" >> "$out" 2>&1 fi @@ -113,13 +115,13 @@ for f in tests_*; do plugins_do test_begin "$t" "$f" start=$(date +%s) if run_test "$here/run-single-test.sh" "$here/$f/$t" "$test_tmp" "$here/.." "$show_info"; then - plugins_do test_ok "$(time_diff_to_now $start)" + plugins_do test_ok "$(time_diff_to_now "$start")" suite_ok=$((suite_ok+1)) if $verbose; then cat "$out" fi else - plugins_do test_fail "$(time_diff_to_now $start)" "$out" + plugins_do test_fail "$(time_diff_to_now "$start")" "$out" suite_fail=$((suite_fail+1)) fi if ! $debug; then @@ -131,7 +133,7 @@ for f in tests_*; do cnt_ok=$((cnt_ok + suite_ok)) cnt_fail=$((cnt_fail + suite_fail)) cd .. - plugins_do test_suite_end "$(time_diff_to_now $start_suite)" "$suite_ok" "$suite_fail" + plugins_do test_suite_end "$(time_diff_to_now "$start_suite")" "$suite_ok" "$suite_fail" done if ! $debug; then @@ -142,7 +144,7 @@ fi # report -if [[ $cnt_fail > 0 ]]; then +if [[ $cnt_fail -gt 0 ]]; then # kill leftovers (the whole process group) # ignore-unacceptable-language trap '' TERM kill 0 # ignore-unacceptable-language @@ -152,7 +154,7 @@ else plugins_do summary_ok "$cnt_ok" "$cnt_fail" fi -if [[ $cnt_fail > 0 ]]; then +if [[ $cnt_fail -gt 0 ]]; then exit 1 else exit 0 diff --git a/IntegrationTests/test_functions.sh b/IntegrationTests/test_functions.sh index 2eafeb71..ee4036a1 100644 --- a/IntegrationTests/test_functions.sh +++ b/IntegrationTests/test_functions.sh @@ -64,6 +64,7 @@ function assert_greater_than_or_equal() { g_has_previously_infoed=false function info() { + # shellcheck disable=SC2154 # Defined by an include our by being source transpiled in if $g_show_info; then if ! $g_has_previously_infoed; then echo >&3 || true # echo an extra newline so it looks better diff --git a/IntegrationTests/tests_01_allocation_counters/test_01_allocation_counts.sh b/IntegrationTests/tests_01_allocation_counters/test_01_allocation_counts.sh index 0b1239fb..5689b9c2 100644 --- a/IntegrationTests/tests_01_allocation_counters/test_01_allocation_counts.sh +++ b/IntegrationTests/tests_01_allocation_counters/test_01_allocation_counts.sh @@ -13,6 +13,7 @@ ## ##===----------------------------------------------------------------------===## +# shellcheck source=IntegrationTests/tests_01_allocation_counters/defines.sh source defines.sh set -eu @@ -28,7 +29,8 @@ for file in "$here/test_01_resources/"test_*.swift; do all_tests+=( "$test_name" ) done -"$here/test_01_resources/run-nio-http2-alloc-counter-tests.sh" -t "$tmp" > "$tmp/output" +# shellcheck disable=SC2154 # Not really sure why this is flagged and not others +"$here/test_01_resources/run-nio-http2-alloc-counter-tests.sh" -t "$tmp" > "${tmp}/output" for test in "${all_tests[@]}"; do cat "$tmp/output" # helps debugging @@ -59,5 +61,5 @@ for test in "${all_tests[@]}"; do assert_less_than_or_equal "$total_allocations" "$max_allowed" assert_greater_than "$total_allocations" "$(( max_allowed - 1000))" fi - done < <(grep "^test_$test[^\W]*.total_allocations:" "$tmp/output" | cut -d: -f1 | cut -d. -f1 | sort | uniq) + done < <(grep "^test_${test}[^\W]*.total_allocations:" "$tmp/output" | cut -d: -f1 | cut -d. -f1 | sort | uniq) done diff --git a/scripts/integration_tests.sh b/scripts/integration_tests.sh index ca630bd7..496a08dc 100755 --- a/scripts/integration_tests.sh +++ b/scripts/integration_tests.sh @@ -16,4 +16,4 @@ set +ex mkdir -p .build # for the junit.xml file -./IntegrationTests/run-tests.sh --junit-xml .build/junit-sh-tests.xml -i $@ +./IntegrationTests/run-tests.sh --junit-xml .build/junit-sh-tests.xml -i "$@" diff --git a/scripts/test_h2spec.sh b/scripts/test_h2spec.sh index a4f6b04f..4e444b55 100755 --- a/scripts/test_h2spec.sh +++ b/scripts/test_h2spec.sh @@ -23,10 +23,11 @@ function stop_server() { sleep 0.5 # just to make sure all the fds could be closed kill -0 "$1" # assert server is still running # ignore-unacceptable-language kill "$1" # tell server to shut down gracefully # ignore-unacceptable-language - for f in $(seq 20); do + for _ in $(seq 20); do if ! kill -0 "$1" 2> /dev/null; then # ignore-unacceptable-language break # good, dead fi + # shellcheck disable=SC2009 # pgrep much more awkward to use for this ps auxw | grep "$1" || true sleep 0.1 done @@ -37,14 +38,14 @@ function stop_server() { # Simple thing to do. Start the server in the background. # Allow extra build arguments from the command line - eg tsan. -swift build $@ +swift build "$@" "$(swift build --show-bin-path)/NIOHTTP2Server" 127.0.0.1 8888 > /dev/null & disown SERVER_PID=$! echo "$SERVER_PID" # Wait for the server to bind a socket. worked=false -for f in $(seq 20); do +for _ in $(seq 20); do port=$(server_lsof "$SERVER_PID" | grep -Eo 'TCP .*:[0-9]+ ' | grep -Eo '[0-9]{4,5} ' | tr -d ' ' || true) if [[ -n "$port" ]]; then worked=true