Skip to content

ci: Improve gotestsum failure detection, prevent early exit #5420

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 15, 2022
Merged

ci: Improve gotestsum failure detection, prevent early exit #5420

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
20441ca
ci: Prevent early exit on gotestsum failure
mafredri Dec 14, 2022
d0a3e50
test: Shorten timeout to cause failure
mafredri Dec 14, 2022
07b87e6
ci: Explicitly capture gotestsum edge case before cat, only output trace
mafredri Dec 14, 2022
236dbd8
ci: Multiple packages can have their own stack
mafredri Dec 14, 2022
d36d232
Revert "test: Shorten timeout to cause failure"
mafredri Dec 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions .github/workflows/coder.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,14 +338,21 @@ jobs:
else
echo ::set-output name=cover::false
fi
set +e
gotestsum --junitfile="gotests.xml" --jsonfile="gotestsum.json" --packages="./..." --debug -- -parallel=8 -timeout=5m -short -failfast $COVERAGE_FLAGS
ret=$?
if ((ret)); then
# Eternalize test timeout logs because "re-run failed" erases
# artifacts and gotestsum doesn't always capture it:
# https://github.com/gotestyourself/gotestsum/issues/292
echo "Checking gotestsum.json for panic trace:"
grep -A 999999 'panic: test timed out' gotestsum.json
Copy link
Member Author

@mafredri mafredri Dec 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: This was pretty loud (so many logs after timeout), so I decided to only capture the stack trace, and while I was at it, only when the criteria is met.

# Multiple test packages could've failed, each one may or may
# not run into the edge case. PS. Don't summon ShellCheck here.
for testWithStack in $(grep 'panic: test timed out' gotestsum.json | grep -E -o '("Test":[^,}]*)'); do
if [ -n "$testWithStack" ] && grep -q "${testWithStack}.*PASS" gotestsum.json; then
echo "Conditions met for gotestsum stack trace missing bug, outputting panic trace:"
grep -A 999999 "${testWithStack}.*panic: test timed out" gotestsum.json
fi
done
fi
exit $ret

Expand Down Expand Up @@ -423,14 +430,21 @@ jobs:

- name: Test with PostgreSQL Database
run: |
set +e
make test-postgres
ret=$?
if ((ret)); then
# Eternalize test timeout logs because "re-run failed" erases
# artifacts and gotestsum doesn't always capture it:
# https://github.com/gotestyourself/gotestsum/issues/292
echo "Checking gotestsum.json for panic trace:"
grep -A 999999 'panic: test timed out' gotestsum.json
# Multiple test packages could've failed, each one may or may
# not run into the edge case. PS. Don't summon ShellCheck here.
for testWithStack in $(grep 'panic: test timed out' gotestsum.json | grep -E -o '("Test":[^,}]*)'); do
if [ -n "$testWithStack" ] && grep -q "${testWithStack}.*PASS" gotestsum.json; then
echo "Conditions met for gotestsum stack trace missing bug, outputting panic trace:"
grep -A 999999 "${testWithStack}.*panic: test timed out" gotestsum.json
fi
done
fi
exit $ret

Expand Down