mfridman / tparse

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

Pulse removal not mentioned in release notes #86

Open philippgille opened 1 year ago

philippgille commented 1 year ago

Hello 👋 , thank you very much for creating and maintaining this open source project!

I recently updated to a newer version of tparse (now v0.11.1) and some commands I ran in the past don't work anymore. Specifically it seems the flag -pulse is not supported anymore. But I don't see this mentioned in the release notes.

As the project is a v0, breaking changes are perfectly fine, so my suggestion is just to add a note in the release notes here on GitHub where -pulse was removed.


Additional info:

mfridman commented 1 year ago

Hey there, glad you found the project useful. And I'll try to include a better changelog for breaking changes!

Indeed, I removed the -pulse flag as I didn't have the bandwidth to think through a nice UX after the large refactor.

Question, would you be using this in CI or locally when running tests?

mfridman commented 1 year ago

Note to self, I was debugging another issue and noticed it's quite annoying not to see any output. So, a reminder to actually do this.

From https://github.com/cybertec-postgresql/pg_timetable

go1.20 test ./... -json -v -count=1 | tparse -all -smallscreen

┌───────────────────────────────────────────────────────────────────────────────────┐
│  STATUS │ ELAPSED │             PACKAGE             │ COVER │ PASS │ FAIL │ SKIP  │
│─────────┼─────────┼─────────────────────────────────┼───────┼──────┼──────┼───────│
│  PASS   │  0.19s  │ pg_timetable/internal/api       │  --   │  2   │  0   │  0    │
│  PASS   │  0.12s  │ pg_timetable/internal/config    │  --   │  6   │  0   │  0    │
│  PASS   │  0.26s  │ pg_timetable/internal/log       │  --   │  15  │  0   │  0    │
│  FAIL   │ 270.17s │ pg_timetable/internal/migrator  │  --   │  6   │  3   │  0    │
│  FAIL   │ 322.12s │ pg_timetable/internal/pgengine  │  --   │  37  │  10  │  0    │
│  FAIL   │ 22.34s  │ pg_timetable/internal/scheduler │  --   │  14  │  1   │  0    │
│  PASS   │  0.16s  │ pg_timetable/internal/tasks     │  --   │  2   │  0   │  0    │
└───────────────────────────────────────────────────────────────────────────────────┘

The tests ran for ~5 min, and it would have been nice to see some output, maybe with an incrementing counter of tests per package or something..

ti-mo commented 1 year ago

I'd like to get this behaviour back in some form. To me, it's rather surprising that no lines are emitted until go test has returned. Travis CI kills jobs that don't generate any output for 10 minutes, so currently the only option is to run with -follow, but our tests are really verbose on stdout. (I suggested splitting tparse and go test output between stdout and stderr in https://github.com/mfridman/tparse/issues/77)

It would be nice if tparse's default behaviour would emit a PASS for a package as soon as it sees a line with Action, Package, Elapsed, but without Test., e.g. "Action":"pass","Package":"github.com/cilium/cilium/pkg/bpf","Elapsed":2.14, as that means go test finished testing a package.

This is similar to the default behaviour of 'go test', it emits ok github.com/cilium/cilium/pkg/bpf 2.134s lines as soon as a package is done testing.

mfridman commented 1 year ago

Yep, that's a good point. As currently implemented, it's either all (-follow) or nothing until go test is done.

The former can get very verbose, which is partially what this tool aims to solve, and the latter isn't great because there's no feedback to either the user (local development) or CI.

Iirc the default output for go test is stdout, is it odd that tparse would redirect the output to stderr and use stdout for its own purpose? Probably not, but figured it's worth raising.

mfridman commented 1 year ago

Alright, I took a stab at something slightly simpler (https://github.com/mfridman/tparse/pull/95). I believe this will get you 90% of the way toward satisfying the CI use case. Although there is still an edge case where the last package could take longer than the CI timeout since the last output.

For these cases, I think a solution may be to add a -periodic flag which can be set to an arbitrary duration 1m and it'll guarantee to output something to inform CI that tests are still running. Maybe this could default to 1m when -progress is true. This is similar to what https://github.com/hashicorp/terraform/pull/6163 does.


This will print a package-level summary as the tests are running. This will include FAIL, PASS, SKIP and no tests to run.

Example:

CleanShot 2023-05-28 at 22 02 30@2x

If this is acceptable, some nice-to-have would be to support color.