Skip to content

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

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
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
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.21

Comments

@dmitshur
Copy link
Member

dmitshur commented Jul 24, 2023 •
edited
Loading

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
@dmitshur dmitshur moved this to In Progress in Go Release Jul 24, 2023
@gopherbot
Copy link
Contributor

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

@gopherbot
Copy link
Contributor

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

@gopherbot
Copy link
Contributor

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 <[email protected]>
Run-TryBot: Dmitri Shuralyov <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Reviewed-by: Austin Clements <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
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 <[email protected]>
Reviewed-by: Austin Clements <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Run-TryBot: Dmitri Shuralyov <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
@gopherbot
Copy link
Contributor

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

@github-project-automation github-project-automation bot moved this from In Progress to Done in Go Release Jul 28, 2023
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 <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
Run-TryBot: Dmitri Shuralyov <[email protected]>
Reviewed-by: Austin Clements <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
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 <[email protected]>
Reviewed-by: Austin Clements <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Run-TryBot: Dmitri Shuralyov <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
@golang golang locked and limited conversation to collaborators Jul 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@dmitshur dmitshur

Labels
FrozenDueToAge 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