Go 1.10 cmd/go: build cache, test cache, go install, go vet, test vet

[Russ Cox]

It's been a busy week. After about a month of debugging and fixing prerequisites for content-based staleness, getting that submitted on Tuesday unblocked a bunch of other work that has now landed and will no doubt need fine tuning based on feedback from all of you.

Build cache

The go command now maintains a cache of built packages and other small metadata (CL 68116 and CL 75473). The cache defaults to the operating system-defined user cache directory but can be moved by setting $GOCACHE. Run "go env GOCACHE" to see the current effective setting. Right now the go command never deletes anything from the cache. If the cache gets too big, run "go clean -cache" instead of deleting the directory. That command will preserve the cache's log.txt file. In a few weeks I'll ask people to post their log.txt files to a Github issue so that we can evaluate cache size management approaches.

The main effect of the build cache is that commands like "go test" and "go build" run fast and do incremental builds always, reusing past build steps as aggressively as possible. You do not have to use "go test -i" or "go build -i" or "go install" just to get fast incremental builds. We will not have to teach new users those workarounds anymore. Everything will just be fast.

Test cache

The go command now also maintains a cache of passing test results (CL 75631). The cache only applies to "go test package-list", not to "go test" without a package list. So if you're working in a directory and just keep running "go test", none of those are cached. But if you test an explicit list of packages ("go test all", "go test mydir/...", "go test math", "go test ."), then caching kicks in. (See "go help test" for more about that command-line distinction.) The cache also only applies when all the test arguments are recognized as cacheable, which is defined as -cpu, -list, -parallel, -run, -short, and -v. If these conditions are met and the cache has a result from a previous run of an identical test binary with identical arguments, go test will redisplay that result instead of rerunning the test. When doing so, it replaces the usual time elapsed "0.123s" in the final summary line with "(cached)".

For example, if you run "go test -short std" and then you modify a file in math/big and run "go test -short std" again, all tests that don't import math/big (transitively) will show cached results without even being rebuilt. Only the tests that do depend in some way on math/big are even rebuilt. And then only the ones that produce a different test binary from last time will be rerun. We expect test caching to be a significant win for people working in large code bases.

Go install + dependencies

The "go install" command no longer installs dependencies of the named packages (CL 75850). If you run "go install foo", the only thing installed is foo. Before, it varied. If dependencies were out-of-date, "go install" also installed any dependencies. The implicit installation of dependencies during "go install" caused a lot of confusion and headaches for users, but it was previously necessary to enable incremental builds. Not anymore. We think that the new "install what I said" semantics will be much more understandable, especially since it's clear from bug reports that many users already expected them. To force installation of dependencies during "go install", use the new "go install -i", by analogy with "go build -i" and "go test -i".

The fact that "go install" used to install any rebuilt dependencies caused confusion most often in conjunction with -a, which means "force rebuild of all dependencies". Now, "go install -a myprog" will force a complete rebuild of all dependencies of myprog, as well as myprog itself, but only myprog will get installed. (All the rebuilt dependencies will still be saved in the build cache, of course.) Making this case work more understandably is especially important in conjunction with the new content-based staleness analysis, because it sees good reasons to rebuild dependencies more often than before, which would have increased the amount of "why did my dependencies get installed" confusion. For example, if you run "go install -gcflags=-N myprog", that installs a myprog built with no compiler optimizations, but it no longer also reinstalls the packages myprog uses from the standard library without compiler optimizations.

Ideally, the very idea of "installing dependencies" would be an implementation detail completely hidden from users. We're not there yet. The main reason you might still need "go install -i" is if you are using a code analysis tool that expects to read package .a files from the $GOPATH/pkg directory. We plan to fix this problem, to update the tools to find what they need more dynamically, but we have not done that yet. It's unclear whether those tools will be updated in time for Go 1.10. One exception is "go vet".

Go vet

The "go vet" command works best with full type information for the packages it is analyzing. Historically that's been problematic in a variety of situations: packages using cgo, packages using vendoring, and packages that don't have up-to-date installed dependencies have all been things that kept vet from running with full type information. Not anymore. The "go vet" command now passes to vet full information about all these things, building new .a files if needed (CL 74355 and CL 74750). Now "go vet" is guaranteed to have up-to-date type information when it analyzes a package, which will improve the accuracy of its analyses. Having the build cache to amortize the cost of building those .a files was the final piece needed to make this happen.

Only "go vet" has this guarantee. Do not use "go tool vet", which is essentially only useful to people working on vet (just like you don't typically run "go tool compile"). Previously you needed to use "go tool vet" if you wanted control over vet flags, but "go vet" now accepts all the flags that "go tool vet" does. (See "go help vet".)

Test Vet

Now that "go vet" can be guaranteed proper type information, we're able to do something Rob and I started talking about at least three years ago: run "go vet" automatically during "go test" (CL 74356). This has two key benefits. First, it avoids needing to teach users a new manual workflow step. Just as we encourage users to set up editor hooks to run gofmt automatically, go test can run go vet automatically. Second, it broadens the set of useful checks. Before, it really only made sense to add a check to vet if that mistake would have made it through routine testing. Now, if there's something that would probably cause a mysterious test failure but that vet can pinpoint much precisely and save the user a debugging session, we can do that.

It only makes sense to run the checks that are effectively 100% reliable. The exact list for Go 1.10 is not yet decided (that's #18085). Right now the list is atomic, bool, buildtags, nilfunc, and printf. You can change the list by (for example) "go test -vet=printf,shift", and you can disable vet entirely by "go test -vet=off". My hope is that being able to enable them in go test will encourage raising the bar for vet check accuracy. One eventual goal would be to retire the misc-vetall builder, because all the vet checks we want are being run during test (without any kind of whitelist of ignored reports).

We expect no perceptible slowdown to "go test" from running vet, because we run vet in parallel with linking the test binary. (There may be a ~2% slowdown when running a very large number of tests.)

Debugging

If you run into a problem that you think is being caused by the cache, you can retry the command you're running with GOCACHE=off to avoid looking at or updating the cache. Even better, you can leave GOCACHE as is and instead set GODEBUG=gocacheverify=1, which will not refuse to read from the cache but then will double-check that any writes back to the cache of data already present match bit-for-bit. If you ever see a failure with GODEBUG=gocacheverify=1 set, please file a bug. On my laptop right now, GODEBUG=gocacheverify=1 ./all.bash passes.

If you do run into problems, please file them on Github and cc @rsc. If you have questions about anything, this email thread is a good place for them.

Thanks.

Russ

[Brad Fitzpatrick]

This is all amazing, thanks!

--
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Keith Randall]

I'm having an issue, not sure if it related to this or not.

Start up a gomote:

gomote create linux-ppc64le-buildlet

gomote push user-khr-linux-ppc64le-buildlet-0

gomote run user-khr-linux-ppc64le-buildlet-0 go/src/make.bash

gomote run user-khr-linux-ppc64le-buildlet-0 go/bin/go test bytes

It works, and is all as it should be.

But if you then edit one file in your workspace (say src/bytes/boundary_test.go) and retest, it doesn't work.

... edit src/bytes/boundary_test.go

gomote push user-khr-linux-ppc64le-buildlet-0

... gomote says it is pushing one changed file.

gomote run user-khr-linux-ppc64le-buildlet-0 go/bin/go test bytes

I get the following error:

# runtime

../src/runtime/extern.go:223:9: undefined: sys.DefaultGoroot

../src/runtime/extern.go:230:9: undefined: sys.TheVersion

../src/runtime/heapdump.go:494:10: undefined: sys.Goexperiment

../src/runtime/proc.go:13:20: undefined: sys.TheVersion

../src/runtime/proc.go:1175:18: undefined: sys.StackGuardMultiplier

../src/runtime/proc.go:1520:23: undefined: sys.StackGuardMultiplier

../src/runtime/proc.go:4844:7: undefined: sys.Goexperiment

../src/runtime/stack.go:93:2: const initializer 880 * sys.StackGuardMultiplier + _StackSystem is not a constant

../src/runtime/stack.go:93:20: undefined: sys.StackGuardMultiplier

FAIL bytes [build failed]

Certainly smells like build system problems.  Maybe a bad interaction between the new build cache and gomote?

If I gomote another make.bash, all is well again, at least until the next push.

[Russ Cox]

On Sat, Nov 4, 2017 at 12:55 AM, Keith Randall <k...@google.com> wrote:

I'm having an issue, not sure if it related to this or not.

Start up a gomote:

gomote create linux-ppc64le-buildlet

gomote push user-khr-linux-ppc64le-buildlet-0

gomote run user-khr-linux-ppc64le-buildlet-0 go/src/make.bash

gomote run user-khr-linux-ppc64le-buildlet-0 go/bin/go test bytes

It works, and is all as it should be.

But if you then edit one file in your workspace (say src/bytes/boundary_test.go) and retest, it doesn't work.

... edit src/bytes/boundary_test.go

gomote push user-khr-linux-ppc64le-buildlet-0

... gomote says it is pushing one changed file.

Do you have an old gomote? Gomote used to cause this problem all the time by deleting generated files on the remote system if they weren't present on your local system as well. That bug was fixed in August (CL 53070).

    go get -u golang...gomote

Russ

[Keith Randall]

Indeed, my gomote was from June.  Updating fixed it, thanks.

[ra...@develer.com]

Thanks Russ, I like the improvements.

One question: what is the rationale of this:

> The go command now also maintains a cache of passing test results (CL 75631). The cache only applies to "go test package-list", not to "go test" without a package list. So if you're working in a directory and just keep running "go test", none of those are cached. But if you test an explicit list of packages ("go test all", "go test mydir/...", "go test math", "go test ."), then caching kicks in.

Giovanni

[Paul Jolly]

 If you do run into problems, please file them on Github and cc @rsc. If you have questions about anything, this email thread is a good place for them

Hi Russ,

A fantastically useful set of changes, thank you. I have a couple of questions.

Firstly, on test caching. We've long been users of rsc.io/gt. The one issue we've had is that things get a little painful when a test depends on something that is not a .go file. We either:

1. embed non .go dependencies in .go files 

2. simply revert to gt -f (go test -count=1 in the new world) to rule out false positive test runs

There is an overhead to option 1 (maintaining embedded versions of non .go dependencies, etc).

With option 2, we totally miss any of the benefits of test caching, despite the fact that only a small number of our tests fall into this category (of having non .go dependencies).

Following these test caching changes, won't it be the case that anyone who has a non .go test dependency will potentially fall foul of false positive test results in the package list mode of go test? 

That said, with test caching seemingly moving closer to becoming a first class citizen (unless I've misread that signal), I wonder whether there's another option: the ability to mark a test as having non .go dependencies (using an approach similar to (*testing.T).Helper). This would allow the test binary and the "regular" test results to be cached, but then when running go test in package list mode it would re-run those tests marked as having non .go dependencies. This is a half-baked thought at best though, the question above is the main point.

Secondly, the changes to go vet:

> The "go vet" command now passes to vet full information about all these things, building new .a files if needed

This is brilliant. How do you envisage allowing others writing vetters/linters etc that rely on type information to be able to tap into the power of vet.cfg or equivalent?

I suspect it will be possible to adapt github.com/kisielk/gotool but wanted to ask the question here first.

Thanks,

Paul

[Caleb Spare]

Hey Russ, this is excellent and I'm sure it's result of an enormous
amount of hard work.

I'm particularly excited for the build caching changes in 1.10 and
beyond. I have worked with a lot of new Gophers over the years and the
need to `go build -i` or `go install` dependencies -- particularly
after upgrading Go -- has been a consistent stumbling block that I'm
really happy to see fixed. It's especially sad when a user doesn't
realize that anything is wrong -- they just take it for granted that
the build/test is supposed to take a few seconds, since that seems
fast compared with other environments.

I'm concerned about test caching. I have a fair number of packages
where the behavior as described (automatically caching without taking
input files into account) is inappropriate. For instance, I have
parsers for a few different languages, and the tests -- partly
inspired by Go compiler tests -- largely consist of files of source
text in those languages with comments describing expected outputs.

Maybe having a way for a package to opt out of test caching
would be a pragmatic partial solution here.

Even better would be a way for the user to add things to the cache
key, but I'm not sure what that would look like.

Caleb

[David Crawshaw]

On Sat, Nov 4, 2017 at 10:16 PM, Caleb Spare <ces...@gmail.com> wrote:
> Hey Russ, this is excellent and I'm sure it's result of an enormous
> amount of hard work.
>
> I'm particularly excited for the build caching changes in 1.10 and
> beyond. I have worked with a lot of new Gophers over the years and the
> need to `go build -i` or `go install` dependencies -- particularly
> after upgrading Go -- has been a consistent stumbling block that I'm
> really happy to see fixed. It's especially sad when a user doesn't
> realize that anything is wrong -- they just take it for granted that
> the build/test is supposed to take a few seconds, since that seems
> fast compared with other environments.
>
> I'm concerned about test caching. I have a fair number of packages
> where the behavior as described (automatically caching without taking
> input files into account) is inappropriate. For instance, I have
> parsers for a few different languages, and the tests -- partly
> inspired by Go compiler tests -- largely consist of files of source
> text in those languages with comments describing expected outputs.

I ran into this problem earlier today. Filed https://golang.org/issue/22583.

> Maybe having a way for a package to opt out of test caching
> would be a pragmatic partial solution here.
>
> Even better would be a way for the user to add things to the cache
> key, but I'm not sure what that would look like.
>
> Caleb
>

> --
> You received this message because you are subscribed to the Google Groups "golang-dev" group.

> To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+...@googlegroups.com.

[Russ Cox]

On Sat, Nov 4, 2017 at 5:34 PM, Paul Jolly <pa...@myitcv.io> wrote:

Firstly, on test caching. We've long been users of rsc.io/gt. The one issue we've had is that things get a little painful when a test depends on something that is not a .go file. [snip] 

Yes, and we need to fix that. There's a TODO in the code, and I've hoisted it into a proper issue golang.org/issue/22593.

Secondly, the changes to go vet:

> The "go vet" command now passes to vet full information about all these things, building new .a files if needed

This is brilliant. How do you envisage allowing others writing vetters/linters etc that rely on type information to be able to tap into the power of vet.cfg or equivalent?

We want to find a way to let tools query the go command directly, especially now that the go command can use its cache to speed repeated queries. Exactly how is still TBD. The goal is to eliminate cmd/go forks like kisielk/gotool - it clearly makes no sense for all these tools to need updating every time an internal detail of the go command changes.

Russ

[fso...@arista.com]

> I'm concerned about test caching. I have a fair number of packages
> where the behavior as described (automatically caching without taking
> input files into account) is inappropriate. For instance, I have
> parsers for a few different languages, and the tests -- partly
> inspired by Go compiler tests -- largely consist of files of source
> text in those languages with comments describing expected outputs.

+1 on this point.  I think the idea of testdata input files and golden files for tests caught on, so there'll be plenty cases where this may be an issue.
Rather than a test (suite) opting out of caching, perhaps you could allow a test source file to express its dependencies on non-go files with a directive something like //go:depends-on [<path-to-file-or-dir>]+? 
If a dir is specified, the "content digest" could represent all files and subdirs recursively, and becomes part of determining if the test binary is up-to-date/needs to be re-run.

A mechanism like this would allow such tests to still benefit from test caching if the input files haven't changed, but cause them to be re-run if the input files were altered.

Thanks,
-Frank

[ma...@influxdb.com]

Will test caching apply with test binaries? That is, if I run `go test -c ./foo` will the foo.test binary use the cache?

I think the answer is no because we aren't running `go test` in that case, but I wanted to double check. 

[Russ Cox]

On Mon, Nov 6, 2017 at 7:11 PM, <ma...@influxdb.com> wrote:

Will test caching apply with test binaries? That is, if I run `go test -c ./foo` will the foo.test binary use the cache?

No, if you run foo.test by hand there is no cache involved. Note that "cd foo; go test" also has no cache involved. Only "go test ./foo".

Russ

[Russ Cox]

On Sun, Nov 5, 2017 at 4:29 PM, fsomers via golang-dev <golan...@googlegroups.com> wrote:

+1 on this point.  I think the idea of testdata input files and golden files for tests caught on, so there'll be plenty cases where this may be an issue.

Yes, absolutely, I agree 100%. The discussion of the solution has moved to golang.org/issue/22593.

Thanks.

Russ

[roger peppe]

> The cache also only applies when all the test arguments are recognized as cacheable, which is defined as -cpu, -list, -parallel, -run, -short, and -v.

You might also consider adding -test.timeout to those flags (for
passing tests only). I just ran a bunch of tests that took more than
30 minutes to run, with one package timing out after 10m. It seems a
pity that it can't use the cached results when I run the tests again
with a longer timeout.

[Russ Cox]

On Tue, Nov 7, 2017 at 11:35 AM, roger peppe <rogp...@gmail.com> wrote:

> The cache also only applies when all the test arguments are recognized as cacheable, which is defined as -cpu, -list, -parallel, -run, -short, and -v.

You might also consider adding -test.timeout to those flags (for
passing tests only). I just ran a bunch of tests that took more than
30 minutes to run, with one package timing out after 10m. It seems a
pity that it can't use the cached results when I run the tests again
with a longer timeout.

I can and should add timeout but that will not help your case. The cache only kicks in for runs with exactly the same command line. If you run with -timeout=10m and then again with -timeout=5m then it's not safe to reuse the old successful result. And I can't tell what's different about a given run from an earlier run, just that I don't have an exact match.

Russ

 

> email to golang-dev+unsubscribe@googlegroups.com.

[roger peppe]

On 8 November 2017 at 01:07, Russ Cox <r...@golang.org> wrote:
> On Tue, Nov 7, 2017 at 11:35 AM, roger peppe <rogp...@gmail.com> wrote:
>>
>> > The cache also only applies when all the test arguments are recognized
>> > as cacheable, which is defined as -cpu, -list, -parallel, -run, -short, and
>> > -v.
>>
>> You might also consider adding -test.timeout to those flags (for
>> passing tests only). I just ran a bunch of tests that took more than
>> 30 minutes to run, with one package timing out after 10m. It seems a
>> pity that it can't use the cached results when I run the tests again
>> with a longer timeout.
>
>
> I can and should add timeout but that will not help your case. The cache
> only kicks in for runs with exactly the same command line. If you run with
> -timeout=10m and then again with -timeout=5m then it's not safe to reuse the
> old successful result. And I can't tell what's different about a given run
> from an earlier run, just that I don't have an exact match.

Would it be possible to have specific logic related to the -timeout flag?
After all, if you run the tests with a longer timeout, no package
that previously passed its tests should fail. And if you run with a shorter
timeout, you *probably* still want to cache the results from when the
timeout was longer.

So if the timeout flag is given, you could use cached results for all previously
passing packages, running tests only for the packages that previously failed
(even better if we could run tests only for the packages that failed
because they
timed out).

Another possibility might be to ignore the timeout flag entirely and just
avoid caching any test results when the test times out.

[Russ Cox]

On Wed, Nov 8, 2017 at 7:38 AM, roger peppe <rogp...@gmail.com> wrote:

On 8 November 2017 at 01:07, Russ Cox <r...@golang.org> wrote:
> On Tue, Nov 7, 2017 at 11:35 AM, roger peppe <rogp...@gmail.com> wrote:
>>
>> > The cache also only applies when all the test arguments are recognized
>> > as cacheable, which is defined as -cpu, -list, -parallel, -run, -short, and
>> > -v.
>>
>> You might also consider adding -test.timeout to those flags (for
>> passing tests only). I just ran a bunch of tests that took more than
>> 30 minutes to run, with one package timing out after 10m. It seems a
>> pity that it can't use the cached results when I run the tests again
>> with a longer timeout.
>
>
> I can and should add timeout but that will not help your case. The cache
> only kicks in for runs with exactly the same command line. If you run with
> -timeout=10m and then again with -timeout=5m then it's not safe to reuse the
> old successful result. And I can't tell what's different about a given run
> from an earlier run, just that I don't have an exact match.

Would it be possible to have specific logic related to the -timeout flag?
After all, if you run the tests with a longer timeout, no package
that previously passed its tests should fail. And if you run with a shorter
timeout, you *probably* still want to cache the results from when the
timeout was longer.

What you're suggesting is effectively that the timeout applies to "run test or display cached result", so that no matter what the timeout we can always display a cached results, because that takes no time at all. It's maybe a bit of a stretch, but we have to do something for timeouts - it's no good if people who use a script that just sets a longer timeout don't get caching - so I'm willing to try this and see. 

Another possibility might be to ignore the timeout flag entirely and just
avoid caching any test results when the test times out.

FWIW, go test never caches test failures, so if a test times out, flag or no flag, that result is not cached.

Russ

[roger peppe]

Yes, that's a good way of looking at it.

> It's maybe a
> bit of a stretch, but we have to do something for timeouts - it's no good if
> people who use a script that just sets a longer timeout don't get caching -
> so I'm willing to try this and see.

Thanks.

[roger peppe]

Yes, that's a good way of looking at it.

> It's maybe a
> bit of a stretch, but we have to do something for timeouts - it's no good if
> people who use a script that just sets a longer timeout don't get caching -
> so I'm willing to try this and see.

Thanks.

[Chris Hines]

This is awesome, Russ.

One of my public packages now fails to pass tests on tip due to a vet bug. See https://github.com/golang/go/issues/22608. I've posted a CL to fix the vet behavior in question.

I also learned that the tests for vet itself rely on perl, so they get skipped on Windows. :(

Chris

[Florin Pățan]

Hi,

I'm a bit late to the party but I have a question regarding the test caching.

How will it be able to catch issues such as: https://golang.org/cl/81576. A

quick look at https://go-review.googlesource.com/q/deflake shows that there

are around 100 merged CLs containing the word "deflake".

As far as I can tell, this means you'll either be lucky on the first run on a

system or you'll have to wait until you magically touch the right code path

that triggers the again. Wouldn't this make the job of everyone a lot more

harder than before?

I've searched the thread here and I couldn't find anyone addressing this.

Thank you.

[Caleb Spare]

Probably any automated test running systems should disable caching
(with -count=1 or whatever).

> --
> You received this message because you are subscribed to the Google Groups
> "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an

> email to golang-dev+...@googlegroups.com.

[Florin Pățan]

But that means two things:

- I have to remember to disable the cache in the CI system

- I have to remember to disable the cache on the local machine when

trying to fix such a bug

As it stands, having to deal daily with people that do not have a full

grasp of the various knobs and switches for the go tool, adding one

more to remember in such a critical development path will not help.

It will also make teaching Go harder since now it will be: you can run

` go test ./... ` and it will tests your packages. However, in CI you should

run with ` GO_TEST_CACHE=0 go test ./... ` (replace GO_TEST_CACHE

with the correct switch) as this will allow you to catch flaky tests in 

your CI. And then, if you get a flaky test, remember to run the same

command in your local machine / editor as well, since you need to

disable the cache as well.

Wouldn't this feature better be an opt-in feature instead of an opt-out

feature?

[Russ Cox]

On Fri, Dec 1, 2017 at 5:10 PM, Florin Pățan <flori...@gmail.com> wrote:

How will it be able to catch issues such as: https://golang.org/cl/81576. A

quick look at https://go-review.googlesource.com/q/deflake shows that there

are around 100 merged CLs containing the word "deflake".

As far as I can tell, this means you'll either be lucky on the first run on a

system or you'll have to wait until you magically touch the right code path

that triggers the again. Wouldn't this make the job of everyone a lot more

harder than before?

You don't have to "magically touch the right code path". You just have to make any change in any dependency that results in a different test.exe file, and the test will be rerun.

Test caching makes a huge difference for large code bases. It's true that flaky tests exist and that test caching makes it a little more difficult to find flaky tests, but that's not a bad tradeoff: the benefit to good behavior should not be thrown away because it mildly exacerbates bad behavior.

On Fri, Dec 1, 2017 at 5:25 PM, Florin Pățan <flori...@gmail.com> wrote:

But that means two things:

- I have to remember to disable the cache in the CI system

- I have to remember to disable the cache on the local machine when

trying to fix such a bug

As it stands, having to deal daily with people that do not have a full

grasp of the various knobs and switches for the go tool, adding one

more to remember in such a critical development path will not help.

It will also make teaching Go harder since now it will be: you can run

` go test ./... ` and it will tests your packages. However, in CI you should

run with ` GO_TEST_CACHE=0 go test ./... ` (replace GO_TEST_CACHE

with the correct switch) as this will allow you to catch flaky tests in 

your CI. And then, if you get a flaky test, remember to run the same

command in your local machine / editor as well, since you need to

disable the cache as well.

I think it would be perfectly fine to run a CI with test caching enabled. The point of CI is to catch newly introduced test failures, not to detect flaky tests. To the extent that test caching makes flaky tests less flaky, that's actually a benefit to CI systems, because they would pin flaky tests on unrelated changes less often.

As for when you are debugging on a local machine, there is documentation here: https://tip.golang.org/cmd/go/#hdr-Test_packages. We expect that most of the time people debug by cd'ing into the relevant directory and running "go test". In that case, as the documentation explains, there is no test caching at all. When that's not true, the output from "go test" clearly says the result is cached when it is:

$ go test strings

ok  strings 0.286s

$ go test strings

ok  strings (cached)

$ 

If you do not "remember to disable the cache" when using this mode, the output should make the behavior clear. 

Finally, again as documented, "The idiomatic way to disable test caching explicitly is to use -count=1." When you have a flaky test usually the very first thing you want to do is to establish the failure rate, for which you would naturally reach for something like -count=10 or -count=100 or -count=1000, all of which disable caching. So probably you'd bypass caching without even thinking about it, either by using -count=100 or by running "go test" from within the directory where the test is. 

My suggestion would be that instead of posing hypotheticals, wait until you need to debug an actual flaky test and see how well the defaults we've chosen do or do not work. We spent a lot of time thinking about them and expect them to work well.

Russ

[land...@gmail.com]

Can you make test caching op-in instead of opt-out?  I and others use a lot of fuzzers in our tests and they stop being useful if they are cached.  Furthermore, golang's correctness guarantees in the compiler are not very strong, so intermittent errors do crop up a lot.  I don't want to see intermittent errors not get caught because of caching.

[Russ Cox]

On Mon, Dec 4, 2017 at 12:32 PM, <land...@gmail.com> wrote:

Can you make test caching op-in instead of opt-out? 

Sorry, but no. (See my previous emails on this thread.)

 

I and others use a lot of fuzzers in our tests and they stop being useful if they are cached.

Again, if you are running a test over and over it should be very obvious what is going on, because the output will literally say "(cached)". At that point I would hope you'd update your script to run with -count=1 to disable the cache in that instance.

 

  Furthermore, golang's correctness guarantees in the compiler are not very strong, so intermittent errors do crop up a lot.  I don't want to see intermittent errors not get caught because of caching.

If your goal is to spend a lot of CPU to find flaky tests, then you can still do this. Just add -count=1 or even -count=100.

Russ

[Sebastien Binet]

On Tue, Dec 5, 2017 at 2:05 AM, Russ Cox <r...@golang.org> wrote:

On Mon, Dec 4, 2017 at 12:32 PM, <land...@gmail.com> wrote:

Can you make test caching op-in instead of opt-out? 

Sorry, but no. (See my previous emails on this thread.)

 

I and others use a lot of fuzzers in our tests and they stop being useful if they are cached.

Again, if you are running a test over and over it should be very obvious what is going on, because the output will literally say "(cached)". At that point I would hope you'd update your script to run with -count=1 to disable the cache in that instance.

wrt input files that are fed to tests as part of a set of TestXYZ functions...

couldn't we somehow "magically" intercept the `os.Open` or `io.Read` calls to compute the hash of the content of the files to be part of the test cache?

or add a method to testing.T to populate a list of files whose contents need to be watched for?

I've just been biten (not too hard, though) by this while trying out go1.10beta1: adding new content to a `testdata/some-file.ng` that was supposed to test new paths in my parser.

the test was initially successful, so it was captured by the cache.

adding new statements in that `testdata/some-file.ng` didn't trigger a cache invalidation and I thought my newly added, minted, parser code was stellar from the first try. (I thought pretty good about myself until I sent the CL to travis...)

(also, as my parser is in a subdir of my repo, I used to run the tests as: `go test ./parser` instead of `(cd ./parser && go test)`)

-s

[Ian Lance Taylor]

On Mon, Dec 11, 2017 at 4:50 AM, Sebastien Binet <seb....@gmail.com> wrote:
>
> On Tue, Dec 5, 2017 at 2:05 AM, Russ Cox <r...@golang.org> wrote:
>>
>> On Mon, Dec 4, 2017 at 12:32 PM, <land...@gmail.com> wrote:
>>>
>>> Can you make test caching op-in instead of opt-out?
>>
>>
>> Sorry, but no. (See my previous emails on this thread.)
>>
>>>
>>> I and others use a lot of fuzzers in our tests and they stop being useful
>>> if they are cached.
>>
>>
>> Again, if you are running a test over and over it should be very obvious
>> what is going on, because the output will literally say "(cached)". At that
>> point I would hope you'd update your script to run with -count=1 to disable
>> the cache in that instance.
>
>
> wrt input files that are fed to tests as part of a set of TestXYZ
> functions...
>
> couldn't we somehow "magically" intercept the `os.Open` or `io.Read` calls
> to compute the hash of the content of the files to be part of the test
> cache?
> or add a method to testing.T to populate a list of files whose contents need
> to be watched for?

Yes. That is https://golang.org/cl/81895, which should go in soon.

> I've just been biten (not too hard, though) by this while trying out
> go1.10beta1: adding new content to a `testdata/some-file.ng` that was
> supposed to test new paths in my parser.
> the test was initially successful, so it was captured by the cache.
> adding new statements in that `testdata/some-file.ng` didn't trigger a cache
> invalidation and I thought my newly added, minted, parser code was stellar
> from the first try. (I thought pretty good about myself until I sent the CL
> to travis...)
> (also, as my parser is in a subdir of my repo, I used to run the tests as:
> `go test ./parser` instead of `(cd ./parser && go test)`)

Might be interesting to check that CL 81895 fixes this problem, as it should.

Ian

[Sebastien Binet]

Ian,

On Mon, Dec 11, 2017 at 7:41 PM, Ian Lance Taylor <ia...@golang.org> wrote:

On Mon, Dec 11, 2017 at 4:50 AM, Sebastien Binet <seb....@gmail.com> wrote:
>
> On Tue, Dec 5, 2017 at 2:05 AM, Russ Cox <r...@golang.org> wrote:
>>
>> On Mon, Dec 4, 2017 at 12:32 PM, <land...@gmail.com> wrote:
>>>
>>> Can you make test caching op-in instead of opt-out?
>>
>>
>> Sorry, but no. (See my previous emails on this thread.)
>>
>>>
>>> I and others use a lot of fuzzers in our tests and they stop being useful
>>> if they are cached.
>>
>>
>> Again, if you are running a test over and over it should be very obvious
>> what is going on, because the output will literally say "(cached)". At that
>> point I would hope you'd update your script to run with -count=1 to disable
>> the cache in that instance.
>
>
> wrt input files that are fed to tests as part of a set of TestXYZ
> functions...
>
> couldn't we somehow "magically" intercept the `os.Open` or `io.Read` calls
> to compute the hash of the content of the files to be part of the test
> cache?
> or add a method to testing.T to populate a list of files whose contents need
> to be watched for?

Yes.  That is https://golang.org/cl/81895, which should go in soon.

thanks for pointing me at this.

interesting CL ! (still reading through it)

 

> I've just been biten (not too hard, though) by this while trying out
> go1.10beta1: adding new content to a `testdata/some-file.ng` that was
> supposed to test new paths in my parser.
> the test was initially successful, so it was captured by the cache.
> adding new statements in that `testdata/some-file.ng` didn't trigger a cache
> invalidation and I thought my newly added, minted, parser code was stellar
> from the first try. (I thought pretty good about myself until I sent the CL
> to travis...)
> (also, as my parser is in a subdir of my repo, I used to run the tests as:
> `go test ./parser` instead of `(cd ./parser && go test)`)

Might be interesting to check that CL 81895 fixes this problem, as it should.

it fixes that particular issue:

$> go clean -cache

$> go test ./...

ok  neugram.io/ng 4.836s

ok  neugram.io/ng/eval 13.141s

?    neugram.io/ng/eval/environ [no test files]

?    neugram.io/ng/eval/gowrap [no test files]

?    neugram.io/ng/eval/gowrap/genwrap [no test files]

?    neugram.io/ng/eval/gowrap/wrapbuiltin [no test files]

?    neugram.io/ng/eval/shell [no test files]

ok  neugram.io/ng/format 0.007s

?    neugram.io/ng/frame [no test files]

?    neugram.io/ng/frame/internal/frametest [no test files]

ok  neugram.io/ng/frame/memframe 0.002s

ok  neugram.io/ng/frame/sqlframe 0.010s

ok  neugram.io/ng/gengo 15.274s

?    neugram.io/ng/internal/bigcplx [no test files]

?    neugram.io/ng/jupyter [no test files]

?    neugram.io/ng/ngcore [no test files]

ok  neugram.io/ng/parser 0.006s

ok  neugram.io/ng/syntax 0.035s

?    neugram.io/ng/syntax/expr [no test files]

?    neugram.io/ng/syntax/shell [no test files]

?    neugram.io/ng/syntax/src [no test files]

?    neugram.io/ng/syntax/stmt [no test files]

?    neugram.io/ng/syntax/tipe [no test files]

?    neugram.io/ng/syntax/token [no test files]

ok  neugram.io/ng/typecheck 0.020s

$> go test ./eval

ok  neugram.io/ng/eval 5.023s

$> go test ./eval

ok  neugram.io/ng/eval 4.983s

$> go test ./eval

ok  neugram.io/ng/eval 4.964s

but it seems it completely disables the cache for the "eval" package tests (that read eval/testdata/*.ng) even if no input file changed.

well, I can live with that, of course (I've lived with that up until now!) but perhaps that's some unintended collateral damage ?

-s

PS: testing with:

$> go version

go version devel +d1be0fd910 Tue Dec 12 04:55:56 2017 +0000 linux/amd64

[Ian Lance Taylor]

On Tue, Dec 12, 2017 at 2:28 AM, Sebastien Binet <seb....@gmail.com> wrote:
>
> $> go test ./eval
> ok neugram.io/ng/eval 5.023s
> $> go test ./eval
> ok neugram.io/ng/eval 4.983s
> $> go test ./eval
> ok neugram.io/ng/eval 4.964s
>
> but it seems it completely disables the cache for the "eval" package tests
> (that read eval/testdata/*.ng) even if no input file changed.
> well, I can live with that, of course (I've lived with that up until now!)
> but perhaps that's some unintended collateral damage ?

Try running `go test -work ./eval` and looking at the testlog.txt file
in a go-buildNNNN subdirectory. Perhaps that will point to the case
that is changing between runs. (If you are using a *BSD system make
sure you are using tip, as CL 83455 fixed a case that often failed to
cache on those systems.)

Ian

[Sebastien Binet]

looking at:

===

# test log

stat testdata

open testdata

getenv TMPDIR

open /tmp/array1.stdout.857292807

getenv PWD

open /home/binet/work/gonum/src/neugram.io/ng/eval/testdata/array1.ng

getenv PATH

stat /home/binet/work/gonum/bin/go

stat /home/binet/bin/go

stat /home/binet/dev/go/gocode/bin/go

stat /home/binet/sdk/latest/bin/go

open /dev/null

open /home/binet/sdk/latest/bin/go

getenv PWD

stat /home

stat /home/binet

stat /home/binet/sdk

stat /home/binet/sdk/latest

stat /home

stat /home/binet

stat /home/binet/sdk

stat /home/binet/sdk/go-master

stat /home

stat /home/binet

stat /home/binet/work

stat /home/binet/work/gonum

stat /home/binet/work/gonum/src

stat /home/binet/work/gonum/src/neugram.io

stat /home/binet/work/gonum/src/neugram.io/ng

stat /home/binet/work/gonum/src/neugram.io/ng/eval

stat /home/binet/work/gonum/src/neugram.io/ng/eval/vendor

stat /home/binet/work/gonum/src/neugram.io/ng/vendor

stat /home/binet/work/gonum/src/neugram.io/ng/vendor/crypto/md5

stat /home/binet/work/gonum/src/neugram.io/vendor

stat /home/binet/work/gonum/src/vendor

stat /home/binet/sdk/latest/src/crypto/md5

stat /home/binet/sdk/latest/pkg/linux_amd64/crypto/md5.a

open /home/binet/sdk/latest/pkg/linux_amd64/crypto/md5.a

[...]

getenv TMPDIR

open /tmp/ng-tmp-288250298/ng-plugin-crypto_md5.go

===

I guess it's the $TMPDIR (and/or the subsequent 'array1.stdout.xxxxx') that's invalidating the cache.

Not sure whether we want to have some special treatment of, say, io/ioutil.Temp{Dir,File}... (out of the bat, it doesn't seem easily tractable)

-s

[Ian Lance Taylor]

In general, using a temporary file should be OK, if the test is
careful to remove the file. What the test caching cares about is that
the state of the inputs is consistent. After the test completes, the
go tool is going to check the status of all the inputs. If the
temporary file was removed by the test, then the go tool will record a
non-existent file. The second time it runs the test, it will check
that the state is the same; it will observe a non-existent file, and
conclude that caching is OK.

I think a more likely problem here is the state of /home and
/home/binet and so forth. If any of those directories change in any
way--if any files are added or removed, thus changing the directory
mtime--the code will detect a possible cache mismatch. Why is your
test looking at those directories?

Ian

[Sebastien Binet]

Ian,

AFAICT, neugram.io/ng/eval's tests are not the ones looking at those directories.

it just happens that I put my Go installation under ~/sdk/go-master and set my GOPATH to ~/work/gonum.

I thought one possible reason was that the cache was confused with my ~/sdk/latest symlink to ~/sdk/go-master, but even after removing it, I still see the cache not being used between 2 subsequent "go test ./eval".

I tried in a docker container:

===

root@b64f6edc7026:/go/path/src/neugram.io/ng# go get -v . && go test . ./eval

ok  neugram.io/ng (cached)

ok  neugram.io/ng/eval 5.032s

root@b64f6edc7026:/go/path/src/neugram.io/ng# go get -v . && go test . ./eval

ok  neugram.io/ng (cached)

ok  neugram.io/ng/eval 5.098s

===

same thing.

should I file a bug report?

-s