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.

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

proposal: testing: store test artifacts #71287

Open
neild opened this issue Jan 15, 2025 · 9 comments
Open

proposal: testing: store test artifacts #71287

neild opened this issue Jan 15, 2025 · 9 comments
Labels
LibraryProposal Issues describing a requested change to the Go standard library or x/ libraries, but not to a tool Proposal
Milestone

Comments

@neild
Copy link
Contributor

neild commented Jan 15, 2025

This is an offshoot of #43936.

Some tests produce output files which the user may want to examine. For example, a test might produce files which are compared to some reference. If the comparison fails, the user will want to examine the generated files. Or a test might produce a packet capture or similar trace which can be used to debug failures.

We call these output files "test artifacts".

This is a proposal to add support for storing test artifacts to the testing package.

We add a new method to testing.TB:

package testing

// OutputDir returns a directory for the test to store output files in.
// When the -outputdir flag is provided, this directory will be located
// under that directory. Otherwise, OutputDir returns a temporary directory
// which is removed after the test completes.
//
// Each test or subtest has a unique artifact directory.
// Repeated calls to OutputDir in the same test or subtest return the same directory.
// Subtest outputs are not located under the parent test's output directory.
func (t *testing.T) OutputDir() string

The -outputdir flag already exists, and is currently used to specify a location to put output files from profiling. We're adding an additional meaning to it here: It's now where all your saved test outputs go.

When -outputdir is specified, the first call to OutputDir in a test or subtest will emit a line to the test output consisting of "=== ARTIFACTS ", the test name, and the test artifact directory, separated by spaces:

=== ARTIFACTS TestName/subtest_name /path/to/root/artifact/dir/TestName__subtest_name

When -json is specified, this will appear as an entry with an Action of "artifacts", the usual Time, Package, and Test keys, and a "Path" key containing the artifact directory:

{"Time":"2025-01-15T13:39:27.75235-08:00","Action":"artifacts","Package":"path","Test":"TestName","Path":"/path/to/root/artifact/dir/TestName"}

That's the proposal.

A few points on the design:

  • I'm reusing the existing -outputdir flag, on the theory that output files from profiling are just another test artifact. If we don't like that reuse, then we could add a new -artifactdir flag and rename TB.OutputDir to TB.ArtifactDir for consistency.

  • The test output uses the word "ARTIFACTS" because the JSON already has "output" events.

  • TB.OutputDir returns a directory, same as TB.TempDir. This seems simpler than asking the testing package to copy files around.

  • TB.OutputDir returns a directory even if we aren't saving artifacts so test behavior doesn't change depending on the presence or absence of the -outputdir flag.

In simple interactive use, users can pass -outputdir to store test artifacts when debugging a failing test.

Test frameworks that collect artifacts can arrange to pass -outputdir to the tests they run and collect any artifacts after the fact.

As a concrete use case, within Google our testing infrastructure sets an environment variable to the location of a directory. Tests can write files into this directory, and those files will be stored and associated with the test run. If we implement this proposal, we can arrange for the test infrastructure to also pass this directory as an -outputdir flag, and any test using TB.OutputDir will automatically use the right location.

@mknyszek
Copy link
Contributor

mknyszek commented Jan 15, 2025

Speaking from the perspective of Go's CI infrastructure, it would be helpful to have some standard convention to indicate output artifacts at the standard library level, so I'm in support of this proposal (or something like it).

We share the tooling for processing Go tests with several other teams, and without a standard for this sort of thing, we would be stuck having to define our own amongst ourselves. That may turn out to be OK in practice, but having a single way to do it would save us all the time of trying to define our own convention, since it wouldn't even be a question. As far as test metadata goes, I think this would be one of the most valuable forms, for the Go team anyway, and IMO more valuable than Attr on its own due the well-defined convention. I can think of several use-cases for exactly this:

  • Execution trace tests and pprof tests would emit artifacts when they encounter broken traces produced by the runtime, for debugging.
  • Crashes in CI would result in core dumps that can be easily downloaded and dissected. (So many more issues would be possible to debug!)
  • Integration tests for binary format serialization/deserialization could dump intermediate artifacts when things go wrong.

I don't have many thoughts on the specifics of how this should look in the output or in the testing package, but from my perspective this doesn't seem like much of an overreach into framework territory. It's a fairly common and simple piece of functionality, and again, essentially just defining a convention.

@gabyhelp gabyhelp added the LibraryProposal Issues describing a requested change to the Go standard library or x/ libraries, but not to a tool label Jan 15, 2025
@seankhliao
Copy link
Member

How would this interact with fuzzing which currently stores new inputs that cause crashes in testdata/fuzz/ ?

@neild
Copy link
Contributor Author

neild commented Jan 15, 2025

No changes to fuzzing, unless I'm missing something. Is there some interaction that should happen there?

@seankhliao
Copy link
Member

I was thinking they're similar in that they generate test artifacts, and if you're running in some remote CI system, you may want a (relatively) easy way to extract the new additions from the other fuzz inputs, such as redirecting them to a specified directory as in this proposal.

@neild
Copy link
Contributor Author

neild commented Jan 16, 2025

I don't think we can have -outputdir stop us from writing fuzz crashes to testdata/fuzz; anyone using fuzzing now is presumably depending on the current behavior and won't want it changed by an orthogonal feature.

Perhaps we could write fuzz crashes to both testdata/fuzz and -outputdir. But I think simpler would be to have a separate -fuzzoutputdir flag to set where new fuzz crashes get written. If you want everything in the same place, set both -fuzzoutputdir and -outputdir.

I think a -fuzzoutputdir flag is probably a separate proposal, though.

@earthboundkid
Copy link
Contributor

Instead of returning a string, could it return an os.Root?

@neild
Copy link
Contributor Author

neild commented Jan 16, 2025

Instead of returning a string, could it return an os.Root?

It could, but that'd be a gratuitous divergence from TB.TempDir. If you want a Root, it's easy enough to open the directory as one. And sometimes you do want just the directory name (when passing it as a flag to some other process, say).

@ianlancetaylor ianlancetaylor moved this to Incoming in Proposals Jan 16, 2025
@willfaught
Copy link
Contributor

willfaught commented Jan 16, 2025

If I run such tests for an unfamiliar package with just go test, will my system accrue these output artifacts permanently without my knowing? How do I clean these, or prevent them entirely?

Where will the paths likely be? Under /tmp, under the package, under GOPATH?

Do tests get unique paths over multiple test runs, or does one test run overwrite the artifacts of another for the same test? If overwrite, are all previous artifacts deleted first?

I was thinking they're similar in that they generate test artifacts, and if you're running in some remote CI system, you may want a (relatively) easy way to extract the new additions from the other fuzz inputs, such as redirecting them to a specified directory as in this proposal.

Perhaps fuzz artifacts should be thought of as generated inputs, not outputs, so they shouldn't be included in this.

@neild
Copy link
Contributor Author

neild commented Jan 16, 2025

If I run such tests for an unfamiliar package with just go test, will my system accrue these output artifacts permanently without my knowing? How do I clean these, or prevent them entirely?

If you don't pass the --outputdir flag to "go test", artifacts get written to a temporary directory (exactly the same as if they were written to a t.TempDir) and are deleted after the test finishes.

If you do pass --outputdir, artifacts get written to that directory and you can decide what to do with them from there. Persistent artifacts are only produced when you ask for them, in the location you specify.

Do tests get unique paths over multiple test runs, or does one test run overwrite the artifacts of another for the same test? If overwrite, are all previous artifacts deleted first?

That is an excellent question. I'm going to have to think about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LibraryProposal Issues describing a requested change to the Go standard library or x/ libraries, but not to a tool Proposal
Projects
Status: Incoming
Development

No branches or pull requests

7 participants