Fix flaky TestSave in comp/core/flare/helpers (FLREM-148) - #54960
Closed
pducolin wants to merge 2 commits into
Closed
Fix flaky TestSave in comp/core/flare/helpers (FLREM-148)#54960pducolin wants to merge 2 commits into
pducolin wants to merge 2 commits into
Conversation
TestSave in comp/core/flare/helpers failed intermittently in CI with "unable to find file" for the saved archive. The archive file name only had second-level time precision, and the final archive path is in the shared system temp directory, not in the flare builder's own unique temp dir. When two flare builds happen in the same second (for example two different test packages running in parallel), they could compute the same final archive name and race with each other. One build could remove or overwrite the other's file before it gets checked. Fix: include the flare builder's own unique temp dir name in the archive file name, so each archive name is globally unique.
Move the fix for the flaky flare archive-name collision from Save() into the test. In production there is only one agent per host, so Save() colliding with another concurrent flare build is not a real-world risk; the collision only happens between concurrent test processes on a CI runner sharing the same OS temp directory. Point TestSave at its own temp directory instead, and leave Save() unchanged.
github-actions
Bot
added
the
short review
pducolin
added
changelog/no-changelog
Collaborator
Author
|
Closing: main already has #54911 (merged), which fixes this exact TestSave flakiness. This PR is now redundant. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes flaky test comp/core/flare/helpers.TestSave. FLREM-148.
Root cause
The flare archive file name only had second-level time precision.
Save()moves the final archive file into the shared system temp directory, not into the flare builder's own private temp directory.Two flare builds can run in the same second in different test processes on the same CI runner. Both can compute the same final archive file name. One build can then overwrite or remove the other build's file before it gets checked. This caused the test to fail sometimes with "unable to find file".
Fix
The test now points its own temp directory to a private, unique folder before it calls
Save(). This keeps the test's archive file away from any other process using the shared system temp directory.Save()itself is unchanged. In production there is only one Agent per host, so this collision is a test-only problem, not a real risk in the field.Test plan
dda inv test --targets=./comp/core/flare/helpers/...locally. All 48 tests pass.dda inv linter.go --targets=./comp/core/flare/helpers. 0 issues.