Skip to content

Commit

Permalink
Allow go-leak linter to fail CI (#5316)
Browse files Browse the repository at this point in the history
## 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 <github@ysh.us>
  • Loading branch information
yurishkuro committed Apr 1, 2024
1 parent 727bf18 commit f18416a
Showing 1 changed file with 20 additions and 7 deletions.
27 changes: 20 additions & 7 deletions scripts/check-goleak-files.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
set -euo pipefail

bad_pkgs=0
failed_pkgs=0

# shellcheck disable=SC2048
for dir in $*; do
Expand All @@ -12,23 +13,35 @@ for dir in $*; do
testFiles=$(find "${dir}" -maxdepth 1 -name '*_test.go')
if [[ -z "$testFiles" ]]; then
continue
fi
fi
good=0
for test in ${testFiles}; do
if grep -q "TestMain" "${test}" | grep -q "testutils.VerifyGoLeaks" "${test}"; then
if grep -q "TestMain" "${test}" && grep -q "testutils.VerifyGoLeaks" "${test}"; then
good=1
break
fi
done
if ((good == 0)); then
echo "🔴 Error(check-goleak): no goleak check in package ${dir}"
((bad_pkgs+=1))
if [[ "${dir}" == "./cmd/jaeger/internal/integration/" || "${dir}" == "./plugin/storage/integration/" ]]; then
echo " this package is temporarily allowed and will not cause linter failure"
else
((failed_pkgs+=1))
fi
fi
done

if ((bad_pkgs > 0)); then
echo "Error(check-goleak): no goleak check in ${bad_pkgs} package(s)."
echo "See https://github.com/jaegertracing/jaeger/pull/5010/files for example of adding the checks."
echo "In the future this will be a fatal error in the CI."
exit 0 # TODO change to 1 in the future
function help() {
echo " See https://github.com/jaegertracing/jaeger/pull/5010/files"
echo " for examples of adding the checks."
}

if ((failed_pkgs > 0)); then
echo "⛔ Fatal(check-goleak): no goleak check in ${bad_pkgs} package(s), ${failed_pkgs} of which not allowed."
help
exit 1
elif ((bad_pkgs > 0)); then
echo "🐞 Warning(check-goleak): no goleak check in ${bad_pkgs} package(s)."
help
fi

0 comments on commit f18416a

Please sign in to comment.