Skip to content
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

Jump to bottom

cmd/dist: runPending doesn't JSON-encode skips in -json mode #61557

Closed
dmitshur opened this issue Jul 24, 2023 · 4 comments
Closed

cmd/dist: runPending doesn't JSON-encode skips in -json mode #61557

dmitshur opened this issue Jul 24, 2023 · 4 comments
Assignees
@dmitshur
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.21

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Jul 24, 2023 •
edited

runPending needs to print valid JSON to w.out when dist is invoked in -json mode. It normally does this because it runs a go test -json command, except we missed that in the case it prints a skip message. This causes consumers of dist test -json output to encounter non-JSON in such cases, and making sense of that can be wasteful of human time.

The fix is fairly trivial, but the diff ended up a bit bigger than I'd like as a result of cleaning things up and writing down more of how work fields are used. So I'll split most of that into a separate CL for Go 1.22, and do a minimal safe fix for Go 1.21.

CC @golang/release, @aclements.
@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Jul 24, 2023
@dmitshur dmitshur added this to the Go1.21 milestone Jul 24, 2023
@dmitshur dmitshur self-assigned this Jul 24, 2023
@gopherbot
Copy link

Change https://go.dev/cl/512719 mentions this issue: cmd/dist: handle -json flag in runPending (minimal)

@gopherbot
Copy link

Change https://go.dev/cl/512115 mentions this issue: cmd/dist: handle -json flag in runPending (clean up)

@gopherbot
Copy link

Change https://go.dev/cl/513761 mentions this issue: [release-branch.go1.21] cmd/dist: handle -json flag in runPending (minimal)

gopherbot pushed a commit that referenced this issue Jul 28, 2023
@dmitshur @gopherbot
cmd/dist: handle -json flag in runPending (minimal)
68d50ca 
The -json flag is new to Go 1.21, but missed skips in runPending.
This CL adds minimal code to fix that. CL 512115 cleans up a bit.

For #37486.
Fixes (via backport) #61557.

Change-Id: I53e426c9a5585b2703f0ff6661a0470e1993f960
Reviewed-on: https://go-review.googlesource.com/c/go/+/512719
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
gopherbot pushed a commit that referenced this issue Jul 28, 2023
@dmitshur @gopherbot
[release-branch.go1.21] cmd/dist: handle -json flag in runPending (mi…
6df6e61 
…nimal)

The -json flag is new to Go 1.21, but missed skips in runPending.
This CL adds minimal code to fix that. CL 512115 cleans up a bit.

For #37486.
Fixes #61557.

Change-Id: I53e426c9a5585b2703f0ff6661a0470e1993f960
Reviewed-on: https://go-review.googlesource.com/c/go/+/513761
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
@gopherbot
Copy link

Closed by merging 6df6e61 to release-branch.go1.21.

gopherbot pushed a commit that referenced this issue Jul 28, 2023
@dmitshur @gopherbot
cmd/dist: handle -json flag in runPending (clean up)
ad90eef 
Document work fields a bit more, and move code that
synthesizes JSON-encoded skip events to testjson.go.

For #37486.
For #61557.

Change-Id: Iffc23cf990bc39696e1e3fce8ce5a6790fc44e78
Reviewed-on: https://go-review.googlesource.com/c/go/+/512115
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
bradfitz pushed a commit to tailscale/go that referenced this issue Aug 2, 2023
@dmitshur @bradfitz
[release-branch.go1.21] cmd/dist: handle -json flag in runPending (mi…
d8bf2ed 
…nimal)

The -json flag is new to Go 1.21, but missed skips in runPending.
This CL adds minimal code to fix that. CL 512115 cleans up a bit.

For golang#37486.
Fixes golang#61557.

Change-Id: I53e426c9a5585b2703f0ff6661a0470e1993f960
Reviewed-on: https://go-review.googlesource.com/c/go/+/513761
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dmitshur dmitshur

Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
Go Release
Archived in project
Milestone
Go1.21
Development

No branches or pull requests
2 participants
@dmitshur @gopherbot