x/build/internal/relui: buildrelease_test fails with flake on uuid length #53972
Comments
toothrot
added
the
NeedsFix
|
I think my original analysis was incorrect. The failure is actually: |
|
Change https://go.dev/cl/423914 mentions this issue: |
checkFile starts its own subtest, accepting a function for validating the file contents. If there is a mismatch in the file, the check function has the parent's testing.T instead of the correct one for the subtest. This should resolve the flake on "test executed panic(nil) or runtime.Goexit: subtest may have called FailNow on a parent test" For golang/go#53972 Change-Id: I62d9b2e596cd87f925338ba4d997684cb438d784 Reviewed-on: https://go-review.googlesource.com/c/build/+/423914 Reviewed-by: Heschi Kreinick <[email protected]> Auto-Submit: Jenny Rakoczy <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Jenny Rakoczy <[email protected]>
|
We think this is fixed! |
|
It wasn't! 2022-08-31T22:22:43-d7b416f-e4b624e/darwin-arm64-12
|
|
Found another occurrence in triage: |
|
This is almost certainly my mistake so I'll take a look now that I'm not on interrupts. |
|
Oh, of course. On GCS, when you write a file, it appears atomically; you can't observe a half-written file. But the test implementation writes to the local filesystem, where you can. So both the observed failures are cases where we're reading an empty file for either the checksum, or in the latter, the actual file content: Guess I'll do write-and-rename in the local filesystem impl. |
|
Change https://go.dev/cl/429275 mentions this issue: |
|
I still think my hypothesis was right, but I only did half the work in the CL above and made the test much flakier :( second attempt coming. |
|
Change https://go.dev/cl/429536 mentions this issue: |
In CL 429275 I changed gcsfs to write atomically, but I didn't use gcsfs in the test infrastructure, which is probably what was causing the flakes. For whatever reason, the test got much flakier, with timeouts this time :( Finish the job. I'm hoping this fixes the timeouts. I can't reproduce them locally, but I did get some errors where it saw the temporary files, so also hide them from directory listings. For golang/go#53972. Change-Id: I2dff87305f9114b975990cd6649cb4a087fadffd Reviewed-on: https://go-review.googlesource.com/c/build/+/429536 Run-TryBot: Heschi Kreinick <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> Auto-Submit: Heschi Kreinick <[email protected]>
|
more flakiness wack-a-mole: |
|
Commit golang/build@656fd83 is just before CL 429536, so there's no new action take here yet. |
|
This failure hasn't recurred since the Sept 9 fix, so this is likely fixed. Closing until we learn otherwise. |
|
Change https://go.dev/cl/448135 mentions this issue: |
CL 429275 and CL 429536 have hidden partially-written files from the release test. I think a remaining race was that uploadArtifacts only waits for the main artifact file, and uploadArtifact copies that one first. So it was possible for fakeCDNLoad to periodically copy all the main files but not yet one of the .sha256 files, and the test release workflow would complete too early, before fakeCDNLoad finished its work. This race is very unlikely to happen in production since everything else takes longer. Still, we consider the .sha256 metadata file as non-optional, so modify uploadArtifacts to wait for all files to be made available before considering its job as done. Fixes golang/go#55178. (Third time's the charm, right?) Updates golang/go#53972. Change-Id: Id470da1fb94ba91b292c63a53e34705b30ba8d66 Reviewed-on: https://go-review.googlesource.com/c/build/+/448135 TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Heschi Kreinick <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> Run-TryBot: Dmitri Shuralyov <[email protected]>
What did you do?
What did you expect to see?
What did you see instead?
https://build.golang.org/log/1b4e137bcc8fc73010bab292fea76e3908e23a11
The text was updated successfully, but these errors were encountered: