testing: ArtifactDir() constructs incorrect path

[ansel1]

Go version

go version go1.26.0 darwin/arm64

Output of go env in your module/workspace:

AR='ar'
CC='cc'
CGO_CFLAGS='-I/opt/homebrew/Cellar/openssl@3/3.6.1/include'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='c++'
GCCGO='gccgo'
GO111MODULE='on'
GOARCH='arm64'
GOARM64='v8.0'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/Users/regan/Library/Caches/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/Users/regan/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/jk/prr92kbj6jv4ghr4mqr_l2g46_v314/T/go-build1397540797=/tmp/go-build -gno-record-gcc-switches -fno-common'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMOD='/Users/regan/dev/tmp/go.mod'
GOMODCACHE='/Users/regan/go/pkg/mod'
GONOPROXY='gitlab.protectv.local,gitlab.gemaltocloud.com'
GONOSUMDB='gitlab.protectv.local,gitlab.gemaltocloud.com'
GOOS='darwin'
GOPATH='/Users/regan/go'
GOPRIVATE='gitlab.protectv.local,gitlab.gemaltocloud.com'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.26.0/libexec'
GOSUMDB='sum.golang.org'
GOTELEMETRY='on'
GOTELEMETRYDIR='/Users/regan/Library/Application Support/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.26.0/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.26.0'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

When running any test which uses the new ArtifactDir() method, it always ends up generating the _artifacts/<random> style of path, rather than a path derived from the test name and pkg.

Given the following files in an empty folder:

go.mod

module artifactdirtest

go 1.26

sample_test.go

package tmp

import (
	"os"
	"path/filepath"
	"testing"
)

func TestArtifacts(t *testing.T) {
	dir := t.ArtifactDir()
	// just create a simple text file with hello world, in the artifactDir,
	// then fail the test.
	err := os.WriteFile(filepath.Join(dir, "hello.txt"), []byte("hello world"), 0o644)
	if err != nil {
		t.Fatal(err)
	}

	t.Fatal("failing test to ensure artifact is saved")

}

Run:

go test -v -artifacts .

What did you see happen?

I got the following output:

=== RUN   TestArtifacts
=== ARTIFACTS TestArtifacts /Users/regan/dev/tmp/_artifacts/2595644996
    sample_test.go:18: failing test to ensure artifact is saved
--- FAIL: TestArtifacts (0.00s)
FAIL
FAIL    artifactdirtest 0.312s
FAIL

What did you expect to see?

The name of the package and test and simple, so I expected to see the artifact dir be tmp/TestArtifacts.

I think the issue is here in testing#common.makeArtifactDir():

// Join with /, not filepath.Join: the import path is /-separated,
// and we don't want removeSymbolsExcept to strip \ separators on Windows.
base := "/" + pkg + "/" + name
base = removeSymbolsExcept(base, "!#$%&()+,-.=@^_{}~ /")
base, err := filepath.Localize(base) 

base is hardcoded to start with a forward slash, but filepath.Localize() explicitly rejects any path that starts with a forward slash. That violates the rules of fs.ValidPath():

Paths must not start or end with a slash: “/x” and “x/” are invalid.

In the debugger, the Localize() call always returns an invalid path error.

[seankhliao]

want to send a CL?

[ansel1]

Love to. I'll go through the contributor process.

[dr2chase]

@adonovan @neild
Looks like there is a CL coming, but in case you have opinions.

[ansel1]

I'm not sure how flexible the original test writers intended the behavior to be. They did not assert anything about the name of the artifact folder, including whether the folder name was derived from the test package or name.

I added an assertion about how many path parts there should be...I could also assert the / pattern, but I'm not sure if I should.

[gopherbot]

Change https://go.dev/cl/748581 mentions this issue: testing: fix construction of the testing artifacts path

[seankhliao]

see #71287 (comment)

The artifact directory would be named _artifacts/// where each component would be mangled to make it path-safe if necessary. The would be a shortened representation of the Go package path of the test (exact details TBD). Existing artifact directories would never be overwritten or deleted. The mangling would be left unspecified, but would be designed such that "nice" test names would map to "nice" directory names, meaning that [a-zA-Z0-9] are left as is, / and = are escaped in a predictable way, and long test names are truncated in a way that is highly likely to be unique (e.g., using a hash of the trimmed part).