What is test_main? Do you mean TestMain?

@ianlancetaylor whoops, yes! Meant main_test.go, i.e. TestMain. Will update the issue now.

EDIT: Issue updated.

Thanks. For complete clarity, can you provide a small self-contained example?

Sure, give me a few minutes to throw together some sample code and I'll update you when it's ready.

My fault: as it turns out I can no longer reproduce this issue today. It occurred yesterday with the same exact version of go, set of tests, etc.

I'll reopen this issue if needed once I figure out what set of steps caused the original failure (clearly the steps I gave are not accurate)

Thanks

@ianlancetaylor Figured it out. Basically, the issue is with SQL migrations. My TestMain is responsible for running these migrations when the test run starts, but since .sql files are not imported by any .go files, I'm assuming that the go test runner has no idea about these migration files and doesn't know to invalidate the cache and run the TestMain code in order to run the migrations. This means that every time I add a new migration, it won't be applied unless I run go test -count=1 ./... manually

To be fair, I don't think it should be Go's responsibility to keep track of extraneous files in the repository and know which ones are critical to the testing process, but I also think it could make sense to always run TestMain (but not necessarily all other tests), even when there's a cache hit, in order to avoid issues like this. Alternatively, the CLI flag that I mentioned in my first post would be useful for me. What do you think?

@bakatz can you please provide an example of code with the issue?

@ysmolsky sure, apologies on the delay on this; been really busy at work. I'll provide an example repo when I next have a free moment. Should be within the next day or two

Ping on that info, @bakatz

@andybons Whoops, I completely forgot about providing a repo for this issue. I've just reproduced the issue with a self-contained piece of code and I'm pushing up the code to github right now. I'll provide a link to the repo once the code is available. Thanks for the reminder

@andybons here is the self-contained repository which you can use to reproduce the bug. See README.md for instructions: https://github.com/bakatz/golangissue21422

EDIT: I've also updated the original issue's "steps to reproduce" since I now know exactly how to reproduce the issue.

Hey @andybons - I'd like to discuss how this issue should be fixed and then just try to fix it myself since I have some free time at the moment. I have a couple questions:

1. Do you think that some sort of default behavior is appropriate here? i.e. always run TestMain even if a cache hit occurs?
2. If not, what about adding a flag to the go test command that forces TestMain to be run?

Thanks!

I do not think we should always run TestMain. If the test is cached, then it is cached. The only question here is whether we should consider the test to be cached or not.

The current guideline we use is that if the test opens any files under GOPATH, then if those files change then the test is no longer cached. This is documented at https://golang.org/cmd/go: "Tests that open files within the package's source root (usually $GOPATH) or that consult environment variables only match future runs in which the files and environment variables are unchanged."

It sounds like your test is opening a .sql file that is not under GOPATH, and that that file is changing. If that is correct, then we do expect the test caching to fail for your case. The workaround for this sort of test is to run it with -test.count=1, which disables caching.

We could also consider changing the current cache plan if we can come up with a simple and efficient mechanism.

@ianlancetaylor Gotcha. That is not correct RE: "opening a .sql file that is not under $GOPATH" -- all of my files are under the directory $GOPATH/src/golangissue21422 in the case of the repository that reproduces this issue.

@bakatz What exactly is changing that causes the cached test result to be invalid?

@ianlancetaylor Adding a new .sql file or updating an existing .sql file within some subdirectory under $GOPATH (i.e. $GOPATH/src/someproject/migrations/somenewmigration.sql). To explain the "real world" situation a bit more: say I have a failing test that is written for an API that isn't working correctly because the database is missing a column. In order to fix this test, I go and add the missing column via a new .sql migration script and re-run the test. The test comes back as cached, so the new .sql migration script that has been added to the filesystem doesn't run, and the test continues to "fail" (even though it's just returning the cached failed result). I then have to re-run the test with -count=1 in order to force the new migration script to run. Let me know if this helps.

@bakatz Thanks. That sounds like a bug. Can you provide the output of GODEBUG=gocachehash=1 go test on this issue or in a gist or something? Note: the output will be long.

Oh, sorry, you provided a repro above. I'll take a look.

I see the problem. The checks for whether changed files affect the hash only affect tests themselves. Files opened in TestMain do not affect whether the test is cached. You could fix your tests by having the tests themselves open the file, e.g., via

var migrationContents []byte
var setMigrationContentsOnce sync.Once

func setMigrationContents(t *testing.T) {
 	migrationContents, err := ioutil.ReadFile("./some_migration.sql")
	if err != nil {
		t.Fatal(err)
	}
}

func TestSomething(t *testing.,T) {
	setMigrationContentsOnce.Do(setMigrationContents)
}

I'll change this issue to consider whether we should check file I/O for caching in TestMain. That won't be until 1.12, though.

@ianlancetaylor thanks for looking into this. The sync.Once workaround is pretty neat; didn't know sync.Once existed till now! Regarding the decision though: is there an argument for not having a file I/O check RE: TestMain? It seems like the behavior here should be the same as it is for any other regular test, but maybe I'm missing something?

As I see it, the argument against is basically that, at least given the current implementation, it would mean that code before TestMain would have to look through the flags, without actually calling flag.Parse. And that might be a little error prone. I think it's pretty clearly desirable if there is a clean and safe way to implement it.

Hi !
Is there any news on this issue ?
Thanks !