Skip to content

test: add JSON output support to the run.go program #56844

Closed
@dmitshur

Description

@dmitshur

This is a tracking issue for adding JSON output support to the test/run.go program, which is invoked by cmd/dist to run tests in the GOROOT/test directory. This is a subset of #37486, broken out into a smaller tracking issue.

This can ideally be implemented by refactoring this program into a normal Go test that can be invoked with go test (which will then automatically have support JSON output via go test's -json flag), or possibly adding a lightweight Go test on top of the existing test/run.go program. It is in principle also possible to add JSON output support to test/run.go directly, without making it a normal Go test. These are implementation details, need to investigate what approach works out better.

CC @aclements, @golang/release.

Activity

added
NeedsInvestigationSomeone must examine and confirm this is a valid issue and not a duplicate of an existing one.
on Nov 18, 2022
added this to the Backlog milestone on Nov 18, 2022
aclements

aclements commented on Nov 21, 2022

@aclements
Member

One complication to this I just discovered is that some of the misc/cgo tests (at least misc/cgo/stdio and misc/cgo/life) actually use the run.go runner. We'll need to rewrite those to not use run.go either by 1) just doing the tests directly because they're really not complicated or 2) exporting the functionality of run.go so tests can use it directly. For other tests, having some of run.go's functionality would actually be really nice, but I don't think it's necessary for these tests.

self-assigned this
on Dec 7, 2022
moved this to In Progress in Go Releaseon Dec 20, 2022
dmitshur

dmitshur commented on Jan 20, 2023

@dmitshur
MemberAuthor

I've started working on this more actively recently and made enough progress to post an update. I think I have a good idea for the general implementation plan here: as expected, it'll be to migrate to using go test for running these test cases, though leave test cases in GOROOT/test as they are. Details follow.

The test/run.go runner handles tests, one for each .go file in the directories listed here (relative to GOROOT/test):

go/test/run.go

Line 103 in 27b6ace

dirs = []string{".", "ken", "chan", "interface", "syntax", "dwarf", "fixedbugs", "codegen", "runtime", "abi", "typeparam", "typeparam/mdempsky"}

For example, using a recent tip commit, there's a total of 2424 test cases:

~/gotip/test $ go run run.go -summary
 2424 ok  

(That number is before any skips are applied due to build constraints or other factors, so it doesn't depend on the GOOS/GOARCH you're trying it on.)

For each of those test cases, I plan to make a subtest in a new internal Go package. For prototyping I've used internal/issue56844 as a temporary placeholder, and so far my best idea for the real thing is probably internal/testdir. We'll see. (Edit: Added cmd/ prefix in #60059.) Assuming that's used, the diff to test/README.md would be something like:

 The test directory contains tests of the Go tool chain and runtime.
 It includes black box tests, regression tests, and error output tests.
 They are run as part of all.bash.
 
 To run just these tests, execute:
 
-    ../bin/go run run.go
+    ../bin/go test internal/testdir
 
 To run just tests from specified files in this directory, execute:
 
-    ../bin/go run run.go -- file1.go file2.go ...
+    ../bin/go test internal/testdir -run='Test/(file1.go|file2.go|...)'
 
 Standard library tests should be written as regular Go tests in the appropriate package.
 
 [...]

I've done some basic timing, and by using the aforementioned subtest arrangement with testing.T.Parallel, all 2424 test cases run in approximately 60-80 seconds on my machine, both before and after. So no regression in test timing. (And, repeated test runs benefit from go test's test caching!)

It's likely important to keep -shard and -shards flags operational for purposes of sharded test execution, but that turns up to be trivial. There are more flags in run.go, and I plan to keep most of them as they are. A few will go away because they're obsolete (e.g., -n is obsolete given go test's own parallelism controls; -showSkips will likely get replaced by t.Skip, etc.).

I haven't yet decided how I'll deal with misc/cgo/stdio and misc/cgo/life, but I'm aiming to just simplify them.

Given that GOROOT/test/run.go is fairly large (2139 lines), I'm looking to do this in a way that makes it easier to review and be confident we're not dropping some tests on the floor. At least one of the CLs will be large to do migration atomically, but fortunately I should be able to make the diff between GOROOT/test/run.go and GOROOT/src/internal/testdir/main_test.go quite small and readable in it. The rest can be split off into smaller preparatory and clean up changes.

added
NeedsFixThe path to resolution is known, but the work has not been done.
and removed
NeedsInvestigationSomeone must examine and confirm this is a valid issue and not a duplicate of an existing one.
on Jan 20, 2023
gopherbot

gopherbot commented on Jan 24, 2023

@gopherbot
Contributor

Change https://go.dev/cl/463276 mentions this issue: cmd/dist, test: convert test/run.go runner to a cmd/go test

dmitshur

dmitshur commented on Feb 6, 2023

@dmitshur
MemberAuthor

A few more things for me not to forget to update:

    • in main repo: test/codegen directory README has instructions for running all codegen tests beyond that what all.bash runs on linux-amd64 builder; the command shown ../bin/go run run.go needs to be update to the new one (CL 463276)
    • in x/website: the go.dev/doc/contribute contribution guide describes the top-level test suite in GOROOT/test and shows the command to run, it'll need to be updated (CL 471895)
    • in x/build: x/build/internal/logparser/parse.go has some special handling of the GOROOT/test portion of output from all.bash; I think it's used by watchflakes and may need some attention afterwards
gopherbot

gopherbot commented on Feb 7, 2023

@gopherbot
Contributor

Change https://go.dev/cl/466155 mentions this issue: internal/testdir: simplify and clean up

gopherbot

gopherbot commented on Feb 7, 2023

@gopherbot
Contributor

Change https://go.dev/cl/465755 mentions this issue: misc/cgo/{life,stdio}: remove reliance on test/run.go

3 remaining items

moved this from In Progress to Done in Go Releaseon Feb 28, 2023
gopherbot

gopherbot commented on Feb 28, 2023

@gopherbot
Contributor

Change https://go.dev/cl/471895 mentions this issue: _content/doc: update GOROOT/test invocation in contribution guide

gopherbot

gopherbot commented on May 9, 2023

@gopherbot
Contributor

Change https://go.dev/cl/493876 mentions this issue: internal/testdir: move to cmd/internal/testdir

gopherbot

gopherbot commented on May 9, 2023

@gopherbot
Contributor

Change https://go.dev/cl/493915 mentions this issue: _content/doc: update testdir import path in contribution guide

gopherbot

gopherbot commented on May 12, 2023

@gopherbot
Contributor

Change https://go.dev/cl/494656 mentions this issue: cmd/dist: use registerStdTestSpecially for normal Go tests only

gopherbot

gopherbot commented on May 26, 2023

@gopherbot
Contributor

Change https://go.dev/cl/498271 mentions this issue: cmd/internal/testdir: stop manually adding GOROOT/bin to PATH

locked and limited conversation to collaborators on May 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

FrozenDueToAgeNeedsFixThe path to resolution is known, but the work has not been done.

Type

No type

Projects

Status

Done

Relationships

None yet

    Development

    No branches or pull requests

      Participants

      @dmitshur@aclements@gopherbot

      Issue actions

        test: add JSON output support to the run.go program · Issue #56844 · golang/go