Skip to content

Commit f18416a

Browse files
authored
Allow go-leak linter to fail CI (#5316)
## Which problem is this PR solving? - Part of #5006 - We have only two packages remaining without goroutine leak checks, both integration tests, which are being worked on - Meanwhile new code / packages do not benefit from enforcement of goleak checks because the linter does not fail CI ## Description of the changes - Add the two remaining packages into exclusion list and force a failure of the CI if anything else is missing goleak checks ## How was this change tested? Successful exit: ```shell $ make goleak Verifying that all packages with tests have goleak in their TestMain 🔴 Error(check-goleak): no goleak check in package ./cmd/jaeger/internal/integration/ this package is temporarily allowed and will not cause linter failure 🔴 Error(check-goleak): no goleak check in package ./plugin/storage/integration/ this package is temporarily allowed and will not cause linter failure 🐞 Warning(check-goleak): no goleak check in 2 package(s). See https://github.com/jaegertracing/jaeger/pull/5010/files for examples of adding the checks. ``` Unsuccessful exit (forced by removing one of the existing checks): ```shell $ make goleak Verifying that all packages with tests have goleak in their TestMain 🔴 Error(check-goleak): no goleak check in package ./cmd/jaeger/internal/integration/ this package is temporarily allowed and will not cause linter failure 🔴 Error(check-goleak): no goleak check in package ./internal/tracegen/ 🔴 Error(check-goleak): no goleak check in package ./plugin/storage/integration/ this package is temporarily allowed and will not cause linter failure ⛔ Fatal(check-goleak): no goleak check in 3 package(s), 1 of which not allowed. See https://github.com/jaegertracing/jaeger/pull/5010/files for examples of adding the checks. make: *** [goleak] Error 1 ``` Signed-off-by: Yuri Shkuro <[email protected]>
1 parent 727bf18 commit f18416a

File tree

1 file changed

+20
-7
lines changed

1 file changed

+20
-7
lines changed

scripts/check-goleak-files.sh

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
set -euo pipefail
44

55
bad_pkgs=0
6+
failed_pkgs=0
67

78
# shellcheck disable=SC2048
89
for dir in $*; do
@@ -12,23 +13,35 @@ for dir in $*; do
1213
testFiles=$(find "${dir}" -maxdepth 1 -name '*_test.go')
1314
if [[ -z "$testFiles" ]]; then
1415
continue
15-
fi
16+
fi
1617
good=0
1718
for test in ${testFiles}; do
18-
if grep -q "TestMain" "${test}" | grep -q "testutils.VerifyGoLeaks" "${test}"; then
19+
if grep -q "TestMain" "${test}" && grep -q "testutils.VerifyGoLeaks" "${test}"; then
1920
good=1
2021
break
2122
fi
2223
done
2324
if ((good == 0)); then
2425
echo "🔴 Error(check-goleak): no goleak check in package ${dir}"
2526
((bad_pkgs+=1))
27+
if [[ "${dir}" == "./cmd/jaeger/internal/integration/" || "${dir}" == "./plugin/storage/integration/" ]]; then
28+
echo " this package is temporarily allowed and will not cause linter failure"
29+
else
30+
((failed_pkgs+=1))
31+
fi
2632
fi
2733
done
2834

29-
if ((bad_pkgs > 0)); then
30-
echo "Error(check-goleak): no goleak check in ${bad_pkgs} package(s)."
31-
echo "See https://github.com/jaegertracing/jaeger/pull/5010/files for example of adding the checks."
32-
echo "In the future this will be a fatal error in the CI."
33-
exit 0 # TODO change to 1 in the future
35+
function help() {
36+
echo " See https://github.com/jaegertracing/jaeger/pull/5010/files"
37+
echo " for examples of adding the checks."
38+
}
39+
40+
if ((failed_pkgs > 0)); then
41+
echo "⛔ Fatal(check-goleak): no goleak check in ${bad_pkgs} package(s), ${failed_pkgs} of which not allowed."
42+
help
43+
exit 1
44+
elif ((bad_pkgs > 0)); then
45+
echo "🐞 Warning(check-goleak): no goleak check in ${bad_pkgs} package(s)."
46+
help
3447
fi

0 commit comments

Comments
 (0)