sleep and retry on failure to delete temp file in tests

[amccarth]

Summary

This solves a flaky test on Windows.

We have similar logic elsewhere for log files. I considered trying to combine them, but since they are methods of different objects, there wasn't really a clean approach.

[zturner]

How about add a function to lldbsuite\support\fs.py?

def remove_file(file, num_retries, sleep_duration):

Then just call this everywhere you need to. Could even nuke this whole RemoveTempFile method.

[amccarth]

Consolidated the os.remove and os.unlink calls with a function to do the retries. Tested on Windows.

[zturner]

packages/Python/lldbsuite/test/lldbtest.py

2004

I think the default for num_retries should be 0. Usually you just want to try once without retry.

2005

You could do for i in range(num_retries+1): and get rid of a bunch of the counter updating and junk

[amccarth]

packages/Python/lldbsuite/test/lldbtest.py

2004

I disagree. Every call site was already doing exactly one retry. I don't see value in making each of them specify that explicitly.

2005

Fine, but that changes the behavior a bit. My approach avoided that extra sleep after the last retry.

[zturner]

Ahh, you're right, I didn't notice that.

packages/Python/lldbsuite/test/lldbtest.py

def DebugPExpect(self, child):
         if not traceAlways:
             return
 
         print(child)
 
     @classmethod
     def RemoveTempFile(cls, file):
         if os.path.exists(file):
+            try:
+                os.remove(file)
+            except:
+                # On Windows, we can sometimes need a retry because antimalware may
+                # briefly hold the file open.
+                time.sleep(0.5)
                 os.remove(file)