Skip to content

Commit

Permalink
Fix TestRemovePath flakiness on windows
Browse files Browse the repository at this point in the history
  • Loading branch information
pchila committed Sep 18, 2023
1 parent 616c36f commit d646f0d
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 20 deletions.
30 changes: 13 additions & 17 deletions internal/pkg/agent/install/uninstall.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,27 +99,23 @@ func Uninstall(cfgFile, topPath, uninstallToken string) error {
func RemovePath(path string) error {
const arbitraryTimeout = 5 * time.Second
start := time.Now()
nextSleep := 1 * time.Millisecond
for {
err := os.RemoveAll(path)
if err == nil {
return nil
}
if isBlockingOnExe(err) {
// try to remove the blocking exe
err = removeBlockingExe(err)
}
if err == nil {
return nil
}
if !isRetryableError(err) {
return err
var lastErr error
for time.Since(start) <= arbitraryTimeout {
lastErr = os.RemoveAll(path)

if lastErr == nil || !isRetryableError(lastErr) {
return lastErr
}

if d := time.Since(start) + nextSleep; d >= arbitraryTimeout {
return err
if isBlockingOnExe(lastErr) {
// try to remove the blocking exe and try again to clean up the path
removeBlockingExe(lastErr)
}

time.Sleep(time.Millisecond)
}

return fmt.Errorf("timed out while removing %q. Last error: %s", path, lastErr)
}

func RemoveBut(path string, bestEffort bool, exceptions ...string) error {
Expand Down
8 changes: 5 additions & 3 deletions internal/pkg/agent/install/uninstall_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@
package install

import (
"io/fs"
"os"
"os/exec"
"path/filepath"
"testing"

"github.com/otiai10/copy"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

Expand All @@ -22,8 +24,6 @@ func TestRemovePath(t *testing.T) {
binaryName = pkgName + ".exe"
)

t.Skip("https://github.com/elastic/elastic-agent/issues/3221")

// Create a temporary directory that we can safely remove. The directory is created as a new
// sub-directory. This avoids having Microsoft Defender quarantine the file if it is exec'd from
// the default temporary directory.
Expand Down Expand Up @@ -51,5 +51,7 @@ func TestRemovePath(t *testing.T) {

// Ensure the directory containing the executable can be removed.
err = RemovePath(destDir)
require.NoError(t, err)
assert.NoError(t, err)
_, err = os.Stat(destDir)
assert.ErrorIsf(t, err, fs.ErrNotExist, "path %q still exists after removal", destDir)
}

0 comments on commit d646f0d

Please sign in to comment.