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

cmd/go: make test caching aware of testdata directories #22583

Closed
crawshaw opened this issue Nov 5, 2017 · 7 comments
Closed

cmd/go: make test caching aware of testdata directories #22583

crawshaw opened this issue Nov 5, 2017 · 7 comments
Assignees
Milestone

Comments

@crawshaw
Copy link
Member

@crawshaw crawshaw commented Nov 5, 2017

I have a test that processes test files from a testdata directory (that it finds using a filepath.Glob). Adding a new file to the directory and running "go test mypkg" ignores the new file, and returns the cached test results.

The go tool already has special handling for testdata directories: they are ignored when "go test" searches for packages to run. Could we extend this and include a hash of the contents of the testdatadirectory in the test cache key?

@crawshaw crawshaw added this to the Go1.10 milestone Nov 5, 2017
@mvdan
Copy link
Member

@mvdan mvdan commented Nov 5, 2017

What would be included - filenames, modes and bodies?

I like this proposal better than the proposals to tell go test about extra file dependencies. Would anyone depend on an extra file outside of testdata for good reason?

@cespare
Copy link
Contributor

@cespare cespare commented Nov 5, 2017

This is a great idea. It would cover almost all the tests at my company which depend on data files.

In a few cases we have test fixtures that are shared between a few different projects (not all of which are Go). In these cases the fixtures are symlinked into testdata.

@rsc
Copy link
Contributor

@rsc rsc commented Nov 5, 2017

This is a possible approach but it is imprecise, meaning it will both rerun tests too often (when a testdata file not relevant to a particular test run changes, for example a cached -run=SpecificTest that doesn't read any testdata at all) and not enough (some tests depend on other things). If we can do a bit better, it would be nice to.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Nov 6, 2017

We could hook the os and/or syscall package and watch which filesystem operations the test binary executes, and cache that as well, keyed by the digest of the test binary.

So running a cached test involves looking up the FS operations from cache, then checking the state of those (including the result of globs), and then using that in the computation of the test result cache key.

@dlsniper
Copy link
Contributor

@dlsniper dlsniper commented Nov 6, 2017

This issue does not address the cases where people do not use testdata as folder for their test data or in which the test data is part of folders like assets or fixtures. A cursory look on Github for fixtures indicates that there are plenty of people using this name for their test data folder: https://github.com/search?l=Go&q=fixtures&type=Code&utf8=✓

@rsc
Copy link
Contributor

@rsc rsc commented Nov 8, 2017

Off topic but if people use some other directory name besides testdata to mean testdata, they should probably be reminded that the Go name is testdata. That name is known to the go command and not scanned by directory walks and so on, so there are computational benefits to using the standard name, in addition to the benefits everyone enjoys when everyone follows a single convention.

@rsc
Copy link
Contributor

@rsc rsc commented Nov 8, 2017

Closing this bug in favor of #22593.

@rsc rsc closed this as completed Nov 8, 2017
@golang golang locked and limited conversation to collaborators Nov 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants