mfridman/tparse

Pulse removal not mentioned in release notes

philippgille opened this issue ยท 5 comments

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:

  • Looks like it was removed here: #59
  • And discussed a bit here, but saying on one hand that pulse is deprecated while on the other hand that it could be useful in CI where spinners are not supported: #58

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?

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

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 #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.

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.

Alright, I took a stab at something slightly simpler (#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 hashicorp/terraform#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.