proposal: x/tools/testfiles: provide utilities for testing tools for Go code with modules

timothy-king commented 3 months ago

Updated July 30.

Proposal Details

The proposal is to move the existing internal x/tools/internal/testfiles.CopyToTmp function for testing Go code with go.mod files into a new exported package x/tools/testfiles. This function creates a temporary testing directory from an io/fs.FS and then potentially renaming files.

// CopyToTmp copies the files and directories in src to a new temporary testing
// directory dst, and returns dst on success.
//
// After copying the files, it processes each of the 'old,new,' rename
// directives in order. Each rename directive moves the relative path "old"
// to the relative path "new" within the directory.
//
// Renaming allows tests to hide files whose names have
// special meaning, such as "go.mod" files or "testdata" directories
// from the go command, or ill-formed Go source files from gofmt.
func CopyToTmp(t testing.TB, src fs.FS, rename ...string) string

The potential renaming allows for populating the new directory with files names significant to Go, such as go.mod, without checking in files with special names. For example if we copy the directory testdata:

    testdata/
        go.mod.test
        a/a.go
        b/b.go

using `dir := testfiles.CopyToTmp(t, os.DirFS("testdata"), "go.mod.test,go.mod"), the resulting files will be:

    dir/
        go.mod
        a/a.go
        b/b.go

This package was originally created to support x/tools/analysistest.Run. From it's documentation:

func Run(t Testing, dir string, a analysis.Analyzer, patterns ...string) []Result Run applies an analysis to the packages denoted by the "go list" patterns.

It loads the packages from the specified directory using golang.org/x/tools/go/packages, runs the analysis on them, and checks that each analysis emits the expected diagnostics and facts [...] .

If the directory contains a go.mod file, Run treats it as the root of the Go module in which to work.

Adopting this proposal would resolve #37054, #46041, #53063, and #61336.

Note this proposal works well with both testdata/ directories using os.DirFS, and txtar archives using x/tools/txtar.FS (#44158).

gabyhelp commented 3 months ago

Related Issues and Documentation

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

earthboundkid commented 3 months ago

See #62484 (os.CopyFS) and #44158 (txtar fs.FS).

timothy-king commented 3 months ago

@earthboundkid Do you want to submit the CL you started for #44158? Or anything else I could do to help it get over the finish line? I think this would lead to a nice simplification to this proposal.

earthboundkid commented 3 months ago

I have been too busy to work on #44158 and I'm not sure what the best way to implement ErrModified is, so if you want to take over, feel free. 😄

gopherbot commented 3 months ago

Change https://go.dev/cl/598996 mentions this issue: internal/testfiles: consolidate API

timothy-king commented 3 months ago

Updated the proposal with the new consolidated API.

golang / go

proposal: x/tools/testfiles: provide utilities for testing tools for Go code with modules #68408

Proposal Details