Skip to content

Implement error reporter format for GitHub Actions #544

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

Open
Tonkpils wants to merge 8 commits into
base: main
Choose a base branch
from
+325 −0
Open

Implement error reporter format for GitHub Actions #544

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cc0dd1b
Implement error reporter format for GitHub Actions
Tonkpils Nov 21, 2025
ac8293a
Reduce line length
Tonkpils Nov 21, 2025
a8e39c5
Add strict checks around panic format
Tonkpils Nov 27, 2025
72b3baa
Ignore non test files log for error annotation
Tonkpils Dec 4, 2025
dc57f2b
Address multi line error message for annotation
Tonkpils Dec 4, 2025
4f3f77b
Use relative file path to fix annotation comment feature
Tonkpils Dec 4, 2025
eef5844
Update github-actions output based on relative file path
Tonkpils Dec 4, 2025
a188b31
Update linter errors
Tonkpils Dec 4, 2025
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
95 changes: 95 additions & 0 deletions testjson/format.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"fmt"
"io"
"os"
"path/filepath"
"regexp"
"sort"
"strings"

Expand Down Expand Up @@ -463,6 +465,12 @@ func githubActionsFormat(out io.Writer) EventFormatter {
}
output := map[name][]string{}

// Compile regex patterns once for parsing test failure information
// fileLinePattern matches patterns like " filename.go:123: error message"
fileLinePattern := regexp.MustCompile(`^\s+([a-zA-Z0-9_\-./]+\.go):(\d+):`)
// panicStackPattern matches panic stack trace lines like "\t/path/to/file.go:123 +0x..."
panicStackPattern := regexp.MustCompile(`^\t(.+\.go):(\d+) \+0x`)

return eventFormatterFunc(func(event TestEvent, exec *Execution) error {
key := name{Package: event.Package, Test: event.Test}

Expand All @@ -476,6 +484,11 @@ func githubActionsFormat(out io.Writer) EventFormatter {

// test case end event
if event.Test != "" && event.Action.IsTerminal() {
// Emit error annotation for failed tests
if event.Action == ActionFail {
writeGitHubActionsError(buf, event, output[key], fileLinePattern, panicStackPattern)
}

if len(output[key]) > 0 {
buf.WriteString("::group::")
} else {
Expand Down Expand Up @@ -513,3 +526,85 @@ func githubActionsFormat(out io.Writer) EventFormatter {
return buf.Flush()
})
}

// writeGitHubActionsError parses test output and emits GitHub Actions error annotations
func writeGitHubActionsError(
buf *bufio.Writer, event TestEvent, outputLines []string, fileLinePattern, panicStackPattern *regexp.Regexp,
) {
sanitize := func(s string) string {
// Percent must be escaped first
s = strings.ReplaceAll(s, "%", "%25")
// Escape newlines and carriage returns
s = strings.ReplaceAll(s, "\r", "%0D")
s = strings.ReplaceAll(s, "\n", "%0A")
return s
}

// Check if this is a panic by looking for panic: in the output
var isPanic bool
var panicMessage strings.Builder
for _, outputLine := range outputLines {
if strings.Contains(outputLine, "panic:") {
Copy link
Copy Markdown

@ccoVeille ccoVeille Nov 22, 2025

Choose a reason for hiding this comment

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

I'm a bit worried this would be true for anything returning the string panic:

The next loop and if would use the regexp to extract the info

Maybe you could check for "panic:", loop, then test with regexp, and you match once, apply the panic/test results

Copy link
Copy Markdown
Author

@Tonkpils Tonkpils Nov 23, 2025

Choose a reason for hiding this comment

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

Do you mean to do this process

"panic:", loop, then test with regexp, and you match once, apply the panic/test results

For every time we see the panic: match or only the once?

What this does is to gather up all the messages that match panic: then attributes them to the matching stack trace in the next if/loop block.

Happy to make the necessary change. Just want to make sure I'm writing it up for the correct scenario you're thinking about 😅

Copy link
Copy Markdown

@ccoVeille ccoVeille Nov 24, 2025

Choose a reason for hiding this comment

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

I'm not sure either what I meant 😬

No matter how it could implemented I just want that gotestsum doesn't report a panic if someone called

t.Errorf("checking there is no panic: just in case")

I feel like adding a test about it would help you to figure how to catch the issue I thought about

Copy link
Copy Markdown
Author

@Tonkpils Tonkpils Nov 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

That makes sense. I can make the panic check a bit more strict but unfortunately I don't believe there's much we can do to prevent someone from writing an error message that may be similar to a panic initial error message.

I can change the condition to check if the entire output line starts out with panic: which is more strict than checking for a substring of panic: in the entire message.

Does that sound reasonable?

Either way, I'll see if I can add some more specific tests to this format so we can have a better picture of the setup/expectation.

Copy link
Copy Markdown

@ccoVeille ccoVeille Nov 24, 2025

Choose a reason for hiding this comment

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

Let's wait for the feedback of maintainers here.

I feel like I'm the only one who reviewed your PR right now

isPanic = true
panicMessage.WriteString(strings.TrimSpace(outputLine))
panicMessage.WriteString(" ")
}
}

if isPanic {
// For panics, emit a single annotation with the panic location
var file string
var line string

// Look for the test file in the stack trace
// Prefer _test.go files over other files (like testing.go or runtime files)
for _, outputLine := range outputLines {
if matches := panicStackPattern.FindStringSubmatch(outputLine); len(matches) == 3 {
stackFile := filepath.Base(matches[1])
stackLine := matches[2]
isTestFile := strings.HasSuffix(stackFile, "_test.go")

if file == "" || isTestFile {
file = stackFile
line = stackLine

if isTestFile {
break
}
}
}
}

message := strings.TrimSpace(panicMessage.String())
if message == "" {
message = "Test panicked"
}

if file != "" && line != "" {
fmt.Fprintf(buf, "::error file=%s,line=%s,title=%s::%s\n",
sanitize(file), line, sanitize(event.Test), sanitize(message))
} else {
fmt.Fprintf(buf, "::error title=%s::%s\n", sanitize(event.Test), sanitize(message))
}
} else {
// For regular test failures, emit one annotation per error line
for _, outputLine := range outputLines {
if matches := fileLinePattern.FindStringSubmatch(outputLine); len(matches) == 3 {
file := matches[1]
line := matches[2]

parts := strings.SplitN(outputLine, ":", 3)
var message string
if len(parts) >= 3 {
message = strings.TrimSpace(parts[2])
}
if message == "" {
message = "Test failed"
}

fmt.Fprintf(buf, "::error file=%s,line=%s,title=%s::%s\n",
sanitize(file), line, sanitize(event.Test), sanitize(message))
}
}
}
}
17 changes: 14 additions & 3 deletions testjson/testdata/format/github-actions.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ this is a Print

::endgroup::
::group::SKIP testjson/internal/good.TestSkipped (0.00s)
good_test.go:23:
good_test.go:23:

::endgroup::
::group::SKIP testjson/internal/good.TestSkippedWitLog (0.00s)
Expand Down Expand Up @@ -74,35 +74,43 @@ this is a Print
this is stderr

::endgroup::
::error file=fails_test.go,line=50::testjson/internal/parallelfails.TestNestedParallelFailures/a: failed sub a
::group::FAIL testjson/internal/parallelfails.TestNestedParallelFailures/a (0.00s)
fails_test.go:50: failed sub a
--- FAIL: TestNestedParallelFailures/a (0.00s)

::endgroup::
::error file=fails_test.go,line=50::testjson/internal/parallelfails.TestNestedParallelFailures/d: failed sub d
::group::FAIL testjson/internal/parallelfails.TestNestedParallelFailures/d (0.00s)
fails_test.go:50: failed sub d
--- FAIL: TestNestedParallelFailures/d (0.00s)

::endgroup::
::error file=fails_test.go,line=50::testjson/internal/parallelfails.TestNestedParallelFailures/c: failed sub c
::group::FAIL testjson/internal/parallelfails.TestNestedParallelFailures/c (0.00s)
fails_test.go:50: failed sub c
--- FAIL: TestNestedParallelFailures/c (0.00s)

::endgroup::
::error file=fails_test.go,line=50::testjson/internal/parallelfails.TestNestedParallelFailures/b: failed sub b
::group::FAIL testjson/internal/parallelfails.TestNestedParallelFailures/b (0.00s)
fails_test.go:50: failed sub b
--- FAIL: TestNestedParallelFailures/b (0.00s)

::endgroup::
::error::testjson/internal/parallelfails.TestNestedParallelFailures: Test failed
FAIL testjson/internal/parallelfails.TestNestedParallelFailures (0.00s)
::error file=fails_test.go,line=29::testjson/internal/parallelfails.TestParallelTheFirst: failed the first
::group::FAIL testjson/internal/parallelfails.TestParallelTheFirst (0.01s)
fails_test.go:29: failed the first

::endgroup::
::error file=fails_test.go,line=41::testjson/internal/parallelfails.TestParallelTheThird: failed the third
::group::FAIL testjson/internal/parallelfails.TestParallelTheThird (0.00s)
fails_test.go:41: failed the third

::endgroup::
::error file=fails_test.go,line=35::testjson/internal/parallelfails.TestParallelTheSecond: failed the second
::group::FAIL testjson/internal/parallelfails.TestParallelTheSecond (0.01s)
fails_test.go:35: failed the second

Expand All @@ -119,13 +127,14 @@ this is a Print

::endgroup::
::group::SKIP testjson/internal/withfails.TestSkipped (0.00s)
fails_test.go:26:
fails_test.go:26:

::endgroup::
::group::SKIP testjson/internal/withfails.TestSkippedWitLog (0.00s)
fails_test.go:30: the skip message

::endgroup::
::error file=fails_test.go,line=34::testjson/internal/withfails.TestFailed: this failed
::group::FAIL testjson/internal/withfails.TestFailed (0.00s)
fails_test.go:34: this failed

Expand All @@ -134,6 +143,7 @@ this is a Print
this is stderr

::endgroup::
::error file=fails_test.go,line=43::testjson/internal/withfails.TestFailedWithStderr: also failed
::group::FAIL testjson/internal/withfails.TestFailedWithStderr (0.00s)
this is stderr
fails_test.go:43: also failed
Expand All @@ -155,6 +165,7 @@ this is stderr
--- PASS: TestNestedWithFailure/b (0.00s)

::endgroup::
::error file=fails_test.go,line=65::testjson/internal/withfails.TestNestedWithFailure/c: failed
::group::FAIL testjson/internal/withfails.TestNestedWithFailure/c (0.00s)
fails_test.go:65: failed
--- FAIL: TestNestedWithFailure/c (0.00s)
Expand All @@ -168,6 +179,7 @@ this is stderr
--- PASS: TestNestedWithFailure/d (0.00s)

::endgroup::
::error::testjson/internal/withfails.TestNestedWithFailure: Test failed
FAIL testjson/internal/withfails.TestNestedWithFailure (0.00s)
::group::PASS testjson/internal/withfails.TestNestedSuccess/a/sub (0.00s)
--- PASS: TestNestedSuccess/a/sub (0.00s)
Expand Down Expand Up @@ -210,4 +222,3 @@ this is stderr
PASS testjson/internal/withfails.TestParallelTheThird (0.00s)
PASS testjson/internal/withfails.TestParallelTheSecond (0.01s)
FAIL Package testjson/internal/withfails (20ms)