mfridman / tparse

CLI tool for summarizing go test output. Pipe friendly. CI/CD friendly.
MIT License
948 stars 22 forks source link

Panic "index out of range" in v0.12+ #102

Closed neilotoole closed 7 months ago

neilotoole commented 10 months ago

tparse panics with index out of range in v0.12.1 and v0.13.1. It does not panic in v0.11.1. I have only verified this behavior on go1.21.1 darwin/arm64.

Steps to reproduce

# Note Go version
$ go version
go version go1.21.1 darwin/arm64

# Get the repo that produces the panic
$ git clone https://github.com/neilotoole/sq.git && cd sq

# Install panicky version
$ go install github.com/mfridman/tparse@v0.13.1
$ tparse --version
tparse version: v0.13.1

# Observe the panic
$ go test ./...  -json -cover | tparse -all -sort=elapsed
panic: runtime error: index out of range [24] with length 24

goroutine 1 [running]:
github.com/mfridman/tparse/internal/app.findCommonPackagePrefix({0x14000130500?, 0x1400012bb78?, 0x4?})
    /Users/neilotoole/work/moi/go/pkg/mod/github.com/mfridman/tparse@v0.13.1/internal/app/table_summary.go:223 +0x144
github.com/mfridman/tparse/internal/app.(*consoleWriter).testsTable(0x14001962e40, {0x14000130500, 0x4d, 0x4d}, {0x0?, 0x10?, 0x0?, 0x1000?})
    /Users/neilotoole/work/moi/go/pkg/mod/github.com/mfridman/tparse@v0.13.1/internal/app/table_tests.go:53 +0x19c
github.com/mfridman/tparse/internal/app.display({0x1043c4898?, 0x14000056028?}, 0x4?, {0x0, 0x0, 0x0, 0x2, 0x1043c3858, 0x0, {0x0, ...}, ...})
    /Users/neilotoole/work/moi/go/pkg/mod/github.com/mfridman/tparse@v0.13.1/internal/app/app.go:82 +0xb0
github.com/mfridman/tparse/internal/app.Run({0x1043c4898, 0x14000056028}, {0x0, 0x0, 0x0, 0x2, 0x1043c3858, 0x0, {0x0, 0x0}, ...})
    /Users/neilotoole/work/moi/go/pkg/mod/github.com/mfridman/tparse@v0.13.1/internal/app/app.go:56 +0x208
main.main()
    /Users/neilotoole/work/moi/go/pkg/mod/github.com/mfridman/tparse@v0.13.1/main.go:148 +0x4b4
mfridman commented 10 months ago

Ohhhh yikes! Thanks for filing an issue and providing a reproducible example.

I'll take a look at this in the next few days. Need to refresh on what #81 was trying to solve and the implementation.

neilotoole commented 9 months ago

@mfridman Any joy on resolving the panic?

mfridman commented 9 months ago

Sorry, haven't had a chance to sit down and resolve this (yet). Been on the road from gRPC conf to GopherCon so it kind of slipped.

I haven't forgotten and do plan to push a fix.

neilotoole commented 9 months ago

@mfridman No worries, I hope the confs were fun. Didn't mean to hassle you, but this is what you get for creating such a useful tool 🤓

neilotoole commented 7 months ago

@mfridman FYI, last week I experienced this panic again (on v0.13.1) on a client's private repo and had to revert, so the issue is not restricted to just the example repo I provided.

mfridman commented 7 months ago

Thanks for the reminder, let me take a look real quick.

mfridman commented 7 months ago

Alright, I think https://github.com/mfridman/tparse/pull/104 should fix this issue. The slice of packages passed to findCommonPackagePrefix function had no deterministic order, and finding a common package prefix was failing if a previously seen package had a longer name.

@neilotoole give it a go when you have a chance.

go install github.com/mfridman/tparse@d11b5c57

I'll cut a proper release sometime this week.

neilotoole commented 7 months ago

@mfridman I can confirm that the issue is resolved for me on d11b5c57. Many thanks!