Skip to content

Commit 800b01e

Browse files
committed
[CI] Add shellcheck and fix up warnings
# Motivation We have quite a lot of shell scripts in our repo and want to make sure that they all pass `shellcheck`. # Modification This PR adds a GH action workflow to the soundness script for `shellcheck` and fixes up all errors and warnings. # Result No more shell/bash discussions
1 parent f938b6a commit 800b01e

File tree

53 files changed

+180
-332
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

53 files changed

+180
-332
lines changed

.github/workflows/soundness.yml

+16
Original file line numberDiff line numberDiff line change
@@ -130,3 +130,19 @@ jobs:
130130
run: |
131131
apt-get -qq update && apt-get -qq -y install curl
132132
curl -s https://raw.githubusercontent.com/apple/swift-nio/main/scripts/check-swift-format.sh | bash
133+
134+
shell-check:
135+
name: Shell check
136+
if: ${{ inputs.format_check_enabled }}
137+
runs-on: ubuntu-latest
138+
container:
139+
image: swiftlang/swift:nightly-6.0-jammy
140+
timeout-minutes: 5
141+
steps:
142+
- name: Checkout repository
143+
uses: actions/checkout@v4
144+
- name: Mark the workspace as safe
145+
# https://github.com/actions/checkout/issues/766
146+
run: git config --global --add safe.directory ${GITHUB_WORKSPACE}
147+
- name: Run shellcheck
148+
run: git ls-files -z '*.sh' | xargs -0 shellcheck

IntegrationTests/allocation-counter-tests-framework/run-allocation-counter.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ while getopts "ns:p:m:d:t:" opt; do
226226
do_hooking=false
227227
;;
228228
s)
229-
shared_files+=( $(abs_path "$OPTARG") )
229+
shared_files+=( "$(abs_path "$OPTARG")" )
230230
;;
231231
p)
232232
pkg_root=$(abs_path "$OPTARG")

IntegrationTests/plugin_junit_xml.sh

+3-3
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ function junit_output_replace() {
4444

4545
function plugin_junit_xml_test_suite_begin() {
4646
junit_testsuite_time=0
47-
junit_output_write "<testsuite name='$1' hostname='$(hostname)' "\
48-
"timestamp='$(date -u +"%Y-%m-%dT%H:%M:%S")' tests='XXX-TESTS-XXX' "\
49-
"failures='XXX-FAILURES-XXX' time='XXX-TIME-XXX' errors='0' id='$(date +%s)'"\
47+
junit_output_write "<testsuite name='$1' hostname='$(hostname)' " \
48+
"timestamp='$(date -u +"%Y-%m-%dT%H:%M:%S")' tests='XXX-TESTS-XXX' " \
49+
"failures='XXX-FAILURES-XXX' time='XXX-TIME-XXX' errors='0' id='$(date +%s)'" \
5050
" package='NIOIntegrationTests.$1'>"
5151
}
5252

IntegrationTests/run-single-test.sh

+2-3
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,11 @@ set -x
2020
set -o pipefail
2121

2222
test="$1"
23-
tmp="$2"
24-
root="$3"
25-
g_show_info="$4"
2623
here="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
2724

25+
# shellcheck source=/dev/null
2826
source "$here/test_functions.sh"
27+
# shellcheck source=/dev/null
2928
source "$test"
3029
wait
3130
)

IntegrationTests/run-tests.sh

+8-6
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,9 @@ function plugins_do() {
3636
done
3737
}
3838

39+
# shellcheck source=/dev/null
3940
source "$here/plugin_echo.sh"
41+
# shellcheck source=/dev/null
4042
source "$here/plugin_junit_xml.sh"
4143

4244
plugins="echo"
@@ -88,7 +90,7 @@ function run_test() {
8890
if $verbose; then
8991
"$@" 2>&1 | tee -a "$out"
9092
# we need to return the return value of the first command
91-
return ${PIPESTATUS[0]}
93+
return "${PIPESTATUS[0]}"
9294
else
9395
"$@" >> "$out" 2>&1
9496
fi
@@ -113,13 +115,13 @@ for f in tests_*; do
113115
plugins_do test_begin "$t" "$f"
114116
start=$(date +%s)
115117
if run_test "$here/run-single-test.sh" "$here/$f/$t" "$test_tmp" "$here/.." "$show_info"; then
116-
plugins_do test_ok "$(time_diff_to_now $start)"
118+
plugins_do test_ok "$(time_diff_to_now "$start")"
117119
suite_ok=$((suite_ok+1))
118120
if $verbose; then
119121
cat "$out"
120122
fi
121123
else
122-
plugins_do test_fail "$(time_diff_to_now $start)" "$out"
124+
plugins_do test_fail "$(time_diff_to_now "$start")" "$out"
123125
suite_fail=$((suite_fail+1))
124126
fi
125127
if ! $debug; then
@@ -131,7 +133,7 @@ for f in tests_*; do
131133
cnt_ok=$((cnt_ok + suite_ok))
132134
cnt_fail=$((cnt_fail + suite_fail))
133135
cd ..
134-
plugins_do test_suite_end "$(time_diff_to_now $start_suite)" "$suite_ok" "$suite_fail"
136+
plugins_do test_suite_end "$(time_diff_to_now "$start_suite")" "$suite_ok" "$suite_fail"
135137
done
136138

137139
if ! $debug; then
@@ -142,7 +144,7 @@ fi
142144

143145

144146
# report
145-
if [[ $cnt_fail > 0 ]]; then
147+
if [[ $cnt_fail -gt 0 ]]; then
146148
# terminate leftovers (the whole process group)
147149
trap '' TERM
148150
kill 0 # ignore-unacceptable-language
@@ -152,7 +154,7 @@ else
152154
plugins_do summary_ok "$cnt_ok" "$cnt_fail"
153155
fi
154156

155-
if [[ $cnt_fail > 0 ]]; then
157+
if [[ $cnt_fail -gt 0 ]]; then
156158
exit 1
157159
else
158160
exit 0

IntegrationTests/test_functions.sh

-12
Original file line numberDiff line numberDiff line change
@@ -61,18 +61,6 @@ function assert_greater_than_or_equal() {
6161
fi
6262
}
6363

64-
g_has_previously_infoed=false
65-
66-
function info() {
67-
if $g_show_info; then
68-
if ! $g_has_previously_infoed; then
69-
echo >&3 || true # echo an extra newline so it looks better
70-
g_has_previously_infoed=true
71-
fi
72-
echo >&3 "info: $*" || true
73-
fi
74-
}
75-
7664
function warn() {
7765
echo >&4 "warning: $*"
7866
}

IntegrationTests/tests_01_http/defines.sh

+21-9
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ function do_netstat() {
5858
}
5959

6060
function create_token() {
61-
mktemp "$tmp/server_token_XXXXXX"
61+
mktemp "${tmp:?}/server_token_XXXXXX"
6262
}
6363

6464
function start_server() {
@@ -84,7 +84,7 @@ function start_server() {
8484

8585
mkdir "$tmp/htdocs"
8686
swift build
87-
"$(swift build --show-bin-path)/NIOHTTP1Server" $extra_args $maybe_nio_host "$port" "$tmp/htdocs" &
87+
"$(swift build --show-bin-path)/NIOHTTP1Server" "$extra_args" $maybe_nio_host "$port" "$tmp/htdocs" &
8888
tmp_server_pid=$!
8989
case "$type" in
9090
inet)
@@ -114,9 +114,11 @@ function start_server() {
114114
esac
115115
echo "port: $port"
116116
echo "curl port: $curl_port"
117-
echo "local token_port; local token_htdocs; local token_pid;" >> "$token"
118-
echo " token_port='$port'; token_htdocs='$tmp/htdocs'; token_pid='$!';" >> "$token"
119-
echo " token_type='$type'; token_server_ip='$maybe_nio_host'" >> "$token"
117+
{
118+
echo "local token_port; local token_htdocs; local token_pid;"
119+
echo " token_port='$port'; token_htdocs='$tmp/htdocs'; token_pid='$!';"
120+
echo " token_type='$type'; token_server_ip='$maybe_nio_host'"
121+
} >> "$token"
120122
tmp_server_pid=$(get_server_pid "$token")
121123
echo "local token_open_fds" >> "$token"
122124
echo "token_open_fds='$(get_number_of_open_fds_for_pid "$tmp_server_pid")'" >> "$token"
@@ -125,21 +127,25 @@ function start_server() {
125127
}
126128

127129
function get_htdocs() {
130+
# shellcheck source=/dev/null
128131
source "$1"
129-
echo "$token_htdocs"
132+
# shellcheck disable=SC2154
133+
echo "${token_htdocs:?}"
130134
}
131135

132136
function get_socket() {
137+
# shellcheck source=/dev/null
133138
source "$1"
134-
echo "$token_port"
139+
echo "${token_port:?}"
135140
}
136141

137142
function stop_server() {
143+
# shellcheck source=/dev/null
138144
source "$1"
139145
sleep 0.5 # just to make sure all the fds could be closed
140146
if command -v lsof > /dev/null 2> /dev/null; then
141-
do_netstat "$token_type"
142-
assert_number_of_open_fds_for_pid_equals "$token_pid" "$token_open_fds"
147+
do_netstat "${token_type:?}"
148+
assert_number_of_open_fds_for_pid_equals "${token_pid:?}" "${token_open_fds:?}"
143149
fi
144150
# assert server is still running
145151
kill -0 "$token_pid" # ignore-unacceptable-language
@@ -149,6 +155,7 @@ function stop_server() {
149155
if ! kill -0 "$token_pid" 2> /dev/null; then # ignore-unacceptable-language
150156
break # good, dead
151157
fi
158+
# shellcheck disable=SC2009
152159
ps auxw | grep "$token_pid" || true
153160
sleep 0.1
154161
done
@@ -158,21 +165,26 @@ function stop_server() {
158165
}
159166

160167
function get_server_pid() {
168+
# shellcheck source=/dev/null
161169
source "$1"
162170
echo "$token_pid"
163171
}
164172

165173
function get_server_port() {
174+
# shellcheck source=/dev/null
166175
source "$1"
167176
echo "$token_port"
168177
}
169178

170179
function get_server_ip() {
180+
# shellcheck source=/dev/null
171181
source "$1"
182+
# shellcheck disable=SC2154
172183
echo "$token_server_ip"
173184
}
174185

175186
function do_curl() {
187+
# shellcheck source=/dev/null
176188
source "$1"
177189
shift
178190
case "$token_type" in

IntegrationTests/tests_01_http/test_01_get_file.sh

+2-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
##
1414
##===----------------------------------------------------------------------===##
1515

16+
# shellcheck source=IntegrationTests/tests_01_http/defines.sh
1617
source defines.sh
1718

1819
token=$(create_token)
@@ -22,7 +23,7 @@ echo FOO BAR > "$htdocs/some_file.txt"
2223
touch "$htdocs/empty"
2324
for file in some_file.txt empty; do
2425
for method in sendfile fileio; do
25-
do_curl "$token" "http://foobar.com/$method/$file" > "$tmp/out.txt"
26+
do_curl "$token" "http://foobar.com/$method/$file" > "${tmp:?}/out.txt"
2627
assert_equal_files "$htdocs/$file" "$tmp/out.txt"
2728
done
2829
done

IntegrationTests/tests_01_http/test_02_get_random_bytes.sh

+2-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
##
1414
##===----------------------------------------------------------------------===##
1515

16+
# shellcheck source=IntegrationTests/tests_01_http/defines.sh
1617
source defines.sh
1718

1819
token=$(create_token)
@@ -22,7 +23,7 @@ base="s/o/m/e/r/a/n/d/o/m/f/o/l/d/e/r"
2223
mkdir -p "$htdocs/$base"
2324
dd if=/dev/urandom of="$htdocs/$base/random.bytes" bs=$((1024 * 1024)) count=2
2425
for method in sendfile fileio; do
25-
do_curl "$token" "http://foobar.com/$method/$base/random.bytes" > "$tmp/random.bytes"
26+
do_curl "$token" "http://foobar.com/$method/$base/random.bytes" > "${tmp:?}/random.bytes"
2627
assert_equal_files "$htdocs/$base/random.bytes" "$tmp/random.bytes"
2728
done
2829
stop_server "$token"

IntegrationTests/tests_01_http/test_03_post_random_bytes.sh

+2-1
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,12 @@
1313
##
1414
##===----------------------------------------------------------------------===##
1515

16+
# shellcheck source=IntegrationTests/tests_01_http/defines.sh
1617
source defines.sh
1718

1819
token=$(create_token)
1920
start_server "$token"
20-
dd if=/dev/urandom of="$tmp/random.bytes" bs=$((64*1024)) count=1
21+
dd if=/dev/urandom of="${tmp:?}/random.bytes" bs=$((64*1024)) count=1
2122
do_curl "$token" --data-binary "@$tmp/random.bytes" \
2223
"http://foobar.com/dynamic/echo" > "$tmp/random.bytes.out"
2324
cmp "$tmp/random.bytes" "$tmp/random.bytes.out"

IntegrationTests/tests_01_http/test_04_keep_alive_works.sh

+2-1
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,15 @@
1313
##
1414
##===----------------------------------------------------------------------===##
1515

16+
# shellcheck source=IntegrationTests/tests_01_http/defines.sh
1617
source defines.sh
1718

1819
token=$(create_token)
1920
start_server "$token"
2021
server_pid=$(get_server_pid "$token")
2122
echo -n \
2223
"$server_pid$server_pid$server_pid$server_pid$server_pid$server_pid$server_pid$server_pid$server_pid$server_pid" \
23-
> "$tmp/out_expected"
24+
> "${tmp:?}/out_expected"
2425
do_curl "$token" \
2526
"http://foobar.com/dynamic/pid" \
2627
"http://foobar.com/dynamic/pid" \

IntegrationTests/tests_01_http/test_05_repeated_reqs_work.sh

+3-2
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,13 @@
1313
##
1414
##===----------------------------------------------------------------------===##
1515

16+
# shellcheck source=IntegrationTests/tests_01_http/defines.sh
1617
source defines.sh
1718

1819
token=$(create_token)
1920
start_server "$token"
20-
do_curl "$token" "http://foobar.com/dynamic/pid" > "$tmp/out"
21-
for f in $(seq 100); do
21+
do_curl "$token" "http://foobar.com/dynamic/pid" > "${tmp:?}/out"
22+
for _ in $(seq 100); do
2223
do_curl "$token" "http://foobar.com/dynamic/pid" > "$tmp/out2"
2324
assert_equal_files "$tmp/out" "$tmp/out2"
2425
done

IntegrationTests/tests_01_http/test_06_http_1.0.sh

+2-1
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,12 @@
1313
##
1414
##===----------------------------------------------------------------------===##
1515

16+
# shellcheck source=IntegrationTests/tests_01_http/defines.sh
1617
source defines.sh
1718

1819
token=$(create_token)
1920
start_server "$token"
20-
echo foo > "$tmp/out_expected"
21+
echo foo > "${tmp:?}/out_expected"
2122
do_curl "$token" --data-binary "@$tmp/out_expected" --http1.0 \
2223
"http://foobar.com/dynamic/echo_balloon" > "$tmp/out_actual"
2324
assert_equal_files "$tmp/out_expected" "$tmp/out_actual"

IntegrationTests/tests_01_http/test_07_headers_work.sh

+2-1
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,13 @@
1313
##
1414
##===----------------------------------------------------------------------===##
1515

16+
# shellcheck source=IntegrationTests/tests_01_http/defines.sh
1617
source defines.sh
1718

1819
token=$(create_token)
1920
start_server "$token"
2021
do_curl "$token" -H "foo: bar" --http1.0 \
21-
"http://foobar.com/dynamic/info" > "$tmp/out"
22+
"http://foobar.com/dynamic/info" > "${tmp:?}/out"
2223
if ! grep -q '("foo", "bar")' "$tmp/out"; then
2324
fail "couldn't find header in response"
2425
fi

IntegrationTests/tests_01_http/test_08_survive_signals.sh

+4-3
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
##
1414
##===----------------------------------------------------------------------===##
1515

16+
# shellcheck source=IntegrationTests/tests_01_http/defines.sh
1617
source defines.sh
1718

1819
token=$(create_token)
@@ -21,16 +22,16 @@ htdocs=$(get_htdocs "$token")
2122
server_pid=$(get_server_pid "$token")
2223
echo FOO BAR > "$htdocs/some_file.txt"
2324

24-
for f in $(seq 20); do
25+
for _ in $(seq 20); do
2526
# send some signals that are usually discarded
2627
kill -CHLD "$server_pid" # ignore-unacceptable-language
2728
kill -URG "$server_pid" # ignore-unacceptable-language
2829
kill -CONT "$server_pid" # ignore-unacceptable-language
2930
kill -WINCH "$server_pid" # ignore-unacceptable-language
3031

31-
do_curl "$token" "http://foobar.com/fileio/some_file.txt" > "$tmp/out.txt" &
32+
do_curl "$token" "http://foobar.com/fileio/some_file.txt" > "${tmp:?}/out.txt" &
3233
curl_pid=$!
33-
for g in $(seq 20); do
34+
for _ in $(seq 20); do
3435
kill -URG "$server_pid" # ignore-unacceptable-language
3536
done
3637
wait $curl_pid

IntegrationTests/tests_01_http/test_09_curl_happy_with_trailers.sh

+3-2
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
##
1414
##===----------------------------------------------------------------------===##
1515

16+
# shellcheck source=IntegrationTests/tests_01_http/defines.sh
1617
source defines.sh
1718

1819
token=$(create_token)
@@ -31,6 +32,6 @@ do_curl "$token" \
3132
"http://foobar.com/dynamic/trailers" \
3233
"http://foobar.com/dynamic/trailers" \
3334
"http://foobar.com/dynamic/trailers" \
34-
> "$tmp/out.txt"
35-
assert_equal_files "$htdocs/some_file.txt" "$tmp/out.txt"
35+
> "${tmp:?}/out.txt"
36+
assert_equal_files "$htdocs/some_file.txt" "${tmp:?}/out.txt"
3637
stop_server "$token"

0 commit comments

Comments
 (0)