-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
datastore: TestLoadCivilTimeInNonUTCZone failed #3402
Comments
commit: b28e0b8 Test output================== WARNING: DATA RACE Write at 0x00000153a5d8 by goroutine 71: cloud.google.com/go/datastore.TestLoadCivilTimeInNonUTCZone() /tmpfs/src/google-cloud-go/datastore/load_test.go:462 +0x434 testing.tRunner() /usr/local/go/src/testing/testing.go:1127 +0x202 |
commit: 26f51c5 Test output================== WARNING: DATA RACE Write at 0x0000015cfc00 by goroutine 41: cloud.google.com/go/datastore.TestLoadCivilTimeInNonUTCZone() /tmpfs/src/google-cloud-go/datastore/load_test.go:462 +0x3b6 testing.tRunner() /usr/local/go/src/testing/testing.go:827 +0x162 |
Looks like this issue is flaky. 😟 I'm going to leave this open and stop commenting. A human should fix and close this. When run at the same commit (26f51c5), this test passed in one build (Build Status, Sponge) and failed in another build (Build Status, Sponge). |
@IlyaFaer looks like your new load test has a race condition, specifically when time.Local is modified here: https://github.com/googleapis/google-cloud-go/blob/master/datastore/load_test.go#L462 Can you come up with a way to test this that does not involve modifying time.Local? |
@tritone, yeah, it can be done, I think. Will try shortly 👌 |
Hi @IlyaFaer , any updates on this? |
@tritone, no, I didn't find a way to do it so far. There is a couple of variants, but both ugly and requires a lot of passing args from one function to another. |
@tbpg and I will figure out a way to handle it. |
This test caused race conditions by manipulating time.Local and there is not a good way to test the behavior while avoiding this. Instead I replace it by passing in timezone explicitly in setVal and writing a test for that. Fixes googleapis#3402
This test causes a race condition and there's no simple way to get it to work. I'll file another issue to figure out a way to test this behavior. Fixes googleapis#3402
This test causes a race condition and there's no simple way to get it to work. I'll file another issue to figure out a way to test this behavior. Fixes #3402
This test failed!
To configure my behavior, see the Build Cop Bot documentation.
If I'm commenting on this issue too often, add the
buildcop: quiet
label andI will stop commenting.
commit: 45131b6
buildURL: Build Status, Sponge
status: failed
Test output
The text was updated successfully, but these errors were encountered: