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: cache test results in go test #11193

Closed
rsc opened this Issue Jun 12, 2015 · 18 comments

Comments

Projects
None yet
@rsc
Contributor

rsc commented Jun 12, 2015

It would be nice to have a mode for go test that cached test results, so that you could change a package and then do 'go test ...' and have it only actually rerun the tests that depend on that package.

This comes up especially in larger trees.

This is not trivial.

@thockin @joeshaw

@rsc rsc self-assigned this Jun 12, 2015

@rsc rsc added this to the Go1.6 milestone Jun 12, 2015

@natefinch

This comment has been minimized.

Contributor

natefinch commented Jun 12, 2015

This would be amazing for juju. Our tests take on the order of 5-10 minutes depending on the speed of your hardware.

@thockin

This comment has been minimized.

thockin commented Jun 12, 2015

Worth adding another request at the same time: some tests have test data -
tests should re-run when that data changes.

For a fun but complex example, one of our tests parses example JSON and
YAML from our docs in the source tree and tries to validate it.
On Jun 12, 2015 4:37 PM, "Nate Finch" notifications@github.com wrote:

This would be amazing for juju. Our tests take on the order of 5-10
minutes depending on the speed of your hardware.


Reply to this email directly or view it on GitHub
#11193 (comment).

@davecheney

This comment has been minimized.

Contributor

davecheney commented Jun 13, 2015

What hardware are you running, it's more like 1/2 an hour for me, even the Ci machines with 8 cores can't do it in under 18

On 13 Jun 2015, at 09:37, Nate Finch notifications@github.com wrote:

This would be amazing for juju. Our tests take on the order of 5-10 minutes depending on the speed of your hardware.


Reply to this email directly or view it on GitHub.

@natefinch

This comment has been minimized.

Contributor

natefinch commented Jun 13, 2015

@davecheney
real 7m49.011s
user 33m29.257s
sys 4m54.547s

2013 XPS 15 (9530)
Quad Core i7
16 GB RAM
Upgraded drive to a Samsung 850 Pro 512GB SSD

It's probably the drive that makes it so much faster for me. I also run with gomaxprocs=8.

rsc added a commit to rsc/gt that referenced this issue Jun 13, 2015

gt: add import comment
This program is a sketch of a solution for golang/go#11193.
@rsc

This comment has been minimized.

Contributor

rsc commented Nov 5, 2015

I wrote rsc.io/gt for this as a first draft. It seems okay, but if we're going to go down this route we need to capture all the relevant context - external files, environment variables, and so on - in the cache key. That will require significantly more design.

@rsc rsc modified the milestones: Unplanned, Go1.6 Nov 5, 2015

@mackstann

This comment has been minimized.

mackstann commented Mar 30, 2016

It would be impossible to automatically track down all possible dependencies on external state (e.g. database contents, network interactions), so it seems like a line must be drawn in the sand, and users must understand where that line is and how it determines when they can rely on the caching mechanism.

Right now, it looks like it only hashes the source files, and to me that seems like a simple and easy to understand demarcation. It may be crude, but that also means it's simple and easy to remember.

Our tests take about 3.5 minutes and getting that down to a small fraction for incremental changes is really nice. I'm going to start using gt regularly and see how it goes. So far, so good.

@gkop

This comment has been minimized.

gkop commented Jun 29, 2016

Without this enhancement, Go is adverse to TDD. Just adding dependencies on gorm and sqlite adds a 20s overhead to every go test run, on modern hardware.

@davecheney

This comment has been minimized.

Contributor

davecheney commented Jun 29, 2016

Gabe, this is because you are recompiling sqlite every time. Use go install
or go test -i to build all test dependencies.

On Thu, 30 Jun 2016, 09:45 Gabe Kopley notifications@github.com wrote:

Without this enhancement, Go is adverse to TDD. Just adding dependencies
on gorm and sqlite adds a 20s overhead to every go test run, on modern
hardware.


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#11193 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAAcA5ETRBw1-LHzy2sIkvQsSztg5DcKks5qQwOggaJpZM4FBiez
.

@gkop

This comment has been minimized.

gkop commented Jun 29, 2016

Whew, thanks Dave! We're all good :)

@mewmew

This comment has been minimized.

Contributor

mewmew commented Jan 20, 2017

Right now, it looks like it only hashes the source files, and to me that seems like a simple and easy to understand demarcation. It may be crude, but that also means it's simple and easy to remember.

I would second this. Taint analysis of input variables and tracking external state changes is not simple.

Rather than making the implementation of test case output caching very complex, I'd much rather have a simple underlying principle which is easy to reason about for when test cases are re-run.

And, hashing of source file content is just that, simple and still rather effective.

If the file path of a testdata file change, the test would be re-run. If the URL of a test case changes, the test would be re-run. Easy to reason about and still effective.

A solution which attempts to track what files are actually used in a test case is bound to miss some, and thus making it difficult to reason about.

@nathany

This comment has been minimized.

Contributor

nathany commented Jan 20, 2017

If this is just based on source files (and possibly testdata), would it make sense if it was opt-in?

Something like go test -fast ./... to utilize the cache whereas running normally could update the cache but not rely on it? That way the default is to fully test everything without having to remember to flush the cache. It seems like the safest way to introduce the feature as well.

@tv42

This comment has been minimized.

tv42 commented Jan 22, 2017

This makes it much less likely for developers to realize they have flaky tests :(

@nathany

This comment has been minimized.

Contributor

nathany commented Jan 23, 2017

@tv42 That is a valuable consideration. In other language ecosystems the tests are often ran in random order to help weed out (unintended) interdependencies and other flaky behaviour.

Caching results is one way to make tests run faster, certainly not the only way.

@gopherbot

This comment has been minimized.

gopherbot commented Nov 2, 2017

Change https://golang.org/cl/75631 mentions this issue: cmd/go: cache successful test results

@gopherbot gopherbot closed this in bd95f88 Nov 3, 2017

@tmm1

This comment has been minimized.

Contributor

tmm1 commented Nov 7, 2017

Is there a way to turn this feature off, to force that tests be re-run?

I'm trying to run some x/net tests on Windows which behave differently depending on running user's privileges. If I run the tests once as a regular user and then again as Administrator, it uses the cache and doesn't re-run tests that were previously skipped to see if they pass.

@bradfitz

This comment has been minimized.

Member

bradfitz commented Nov 7, 2017

@tmm1, yes, see the docs and Russ's announcement. Both include instructions for turning caching off.

See https://tip.golang.org/cmd/go/ and search for "cach". In particular:

The idiomatic way to disable test caching explicitly is to use -count=1.

@Quasilyte

This comment has been minimized.

Contributor

Quasilyte commented Nov 15, 2017

Scenario:

  1. Run tests that pass.
  2. Change external data file (from testdata, for example).
  3. Run tests again.
    => Tests passed (taken from cache), but new data is not tested.
    If someone does not know about caching, it will lead to bad outcome.

Detailed example:

  1. Run MIPS tests. They pass.
GOROOT=`pwd` bin/go test -a -v -run TestMIPS cmd/asm/internal/asm
=== RUN   TestMIPSEndToEnd
--- PASS: TestMIPSEndToEnd (0.00s)
=== RUN   TestMIPSOperandParser
--- PASS: TestMIPSOperandParser (0.00s)
=== RUN   TestMIPS64OperandParser
--- PASS: TestMIPS64OperandParser (0.00s)
PASS
ok  	cmd/asm/internal/asm	0.006s
  1. Modify mips64.txt.
$ git diff
diff --git a/src/cmd/asm/internal/asm/testdata/mips64.s b/src/cmd/asm/internal/asm/testdata/mips64.s
index 50a2694..74d0d6f 100644
--- a/src/cmd/asm/internal/asm/testdata/mips64.s
+++ b/src/cmd/asm/internal/asm/testdata/mips64.s
@@ -102,7 +102,7 @@ TEXT foo(SB),DUPOK|NOSPLIT,$0
        MOVV    R1, (R2)
 
        SC      R1, (R2) // e0410000
-       SCV     R1, (R2) // f0410000
+       SCV     R1, (R2) // f0410000deadbeef
 
 //     LMOVB rreg ',' addr
  1. Re-run tests.
GOROOT=`pwd` bin/go test -a -v -run TestMIPS cmd/asm/internal/asm
=== RUN   TestMIPSEndToEnd
--- PASS: TestMIPSEndToEnd (0.00s)
=== RUN   TestMIPSOperandParser
--- PASS: TestMIPSOperandParser (0.00s)
=== RUN   TestMIPS64OperandParser
--- PASS: TestMIPS64OperandParser (0.00s)
PASS
ok  	cmd/asm/internal/asm	(cached)

I hope this will be fixed,
or cache will be disabled by default.

Addendum: with -count=1 it works as expected.

$ GOROOT=`pwd` bin/go test -count=1 -a -v -run TestMIPS cmd/asm/internal/asm
=== RUN   TestMIPSEndToEnd
--- FAIL: TestMIPSEndToEnd (0.00s)
	endtoend_test.go:228: 00224 (testdata/mips64.s:105)	SCV	R1, (R2): have encoding f0410000, want f0410000deadbeef
=== RUN   TestMIPSOperandParser
--- PASS: TestMIPSOperandParser (0.00s)
=== RUN   TestMIPS64OperandParser
--- PASS: TestMIPS64OperandParser (0.00s)
FAIL
FAIL	cmd/asm/internal/asm	0.005s
@cespare

This comment has been minimized.

Contributor

cespare commented Nov 15, 2017

@Quasilyte discussion about that is ongoing in #22593.

@golang golang locked and limited conversation to collaborators Nov 15, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.