-
Notifications
You must be signed in to change notification settings - Fork 122
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
Fix TestRemovePath flakiness on windows #3431
Conversation
Pinging @elastic/elastic-agent (Team:Elastic-Agent) |
🌐 Coverage report
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but I'd add a changelog entry. If it turns out the currently released version of this code has an issue users and support will want the changelog to help know which version has the fix.
} | ||
|
||
time.Sleep(50 * time.Millisecond) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might be too much pressure on the OS. But it's a guess. However I think a counter of how many tries happened would be good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
50 milliseconds is a long time to a computer, this is an improvement over what it did before which was not sleeping.
I'd be in favour of logging how long this function took to complete so we can tell if any future flakiness is just related to marginal timing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buildkite test this |
SonarQube Quality Gate |
Slightly modified the RemovePath retry loop on windows adding another iteration after deleting a blocking executable and introducing a small sleep between attempts. (cherry picked from commit 995b85c)
Slightly modified the RemovePath retry loop on windows adding another iteration after deleting a blocking executable and introducing a small sleep between attempts. (cherry picked from commit 995b85c) Co-authored-by: Paolo Chilà <paolo.chila@elastic.co>
What does this PR do?
Slightly modified the
RemovePath
retry loop on windows adding another iteration after deleting a blocking executable.Why is it important?
We want to make sure that we can uninstall elastic-agent even if a binary is still executing within the directory but we want to avoid false positives
Checklist
[ ] I have made corresponding changes to the documentation[ ] I have made corresponding change to the default configuration files[ ] I have added tests that prove my fix is effective or that my feature works[ ] I have added an entry in./changelog/fragments
using the changelog tool[ ] I have added an integration test or an E2E testAuthor's Checklist
How to test this PR locally
Just run the test on windows (multiple times would be ideal) like:
gotestsum -- -count 1000 -v -run '^TestRemovePath$' github.com/elastic/elastic-agent/internal/pkg/agent/install/
Related issues
Use cases
Screenshots
Logs
Questions to ask yourself