Skip to content

Commit

Permalink
[ci] Try to fix Windows CI failures (#18145)
Browse files Browse the repository at this point in the history
I often encounter test failures on Windows (failing tests are always
related to `mypy` daemon).

https://github.com/python/mypy/actions/runs/11783610549/job/32821129579
https://github.com/python/mypy/actions/runs/11767558564/job/32776369396
https://github.com/python/mypy/actions/runs/11763771676/job/32768172783

They always manifest as a `RecursionError` during test teardown, the
following two frames are repeated many times:

```
___________ ERROR at teardown of testDaemonRunIgnoreMissingImports ____________
[gw3] win32 -- Python 3.8.10 D:\a\mypy\mypy\.tox\py38\Scripts\python.EXE

path = 'C:\Users\RUNNER~1\AppData\Local\Temp\mypy-test-6t6j6e1l\tmp'
onerror = <function TemporaryDirectory._rmtree.<locals>.onerror at 0x00000250EE0F94C0>

    def _rmtree_unsafe(path, onerror):
        try:
            with os.scandir(path) as scandir_it:
                entries = list(scandir_it)
        except OSError:
            onerror(os.scandir, path, sys.exc_info())
            entries = []
        for entry in entries:
            fullname = entry.path
            if _rmtree_isdir(entry):
                try:
                    if entry.is_symlink():
                        # This can only happen if someone replaces
                        # a directory with a symlink after the call to
                        # os.scandir or entry.is_dir above.
                        raise OSError("Cannot call rmtree on a symbolic link")
                except OSError:
                    onerror(os.path.islink, fullname, sys.exc_info())
                    continue
                _rmtree_unsafe(fullname, onerror)
            else:
                try:
                    os.unlink(fullname)
                except OSError:
                    onerror(os.unlink, fullname, sys.exc_info())
        try:
>           os.rmdir(path)
E           PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\Users\RUNNER~1\AppData\Local\Temp\mypy-test-6t6j6e1l\tmp'

C:\hostedtoolcache\windows\Python\3.8.10\x64\lib\shutil.py:620: PermissionError

During handling of the above exception, another exception occurred:

func = <built-in function rmdir>
path = 'C:\Users\RUNNER~1\AppData\Local\Temp\mypy-test-6t6j6e1l\tmp'
exc_info = (<class 'PermissionError'>, PermissionError(13, 'The process cannot access the file because it is being used by another process'), <traceback object at 0x00000250EEB52A80>)

    def onerror(func, path, exc_info):
        if issubclass(exc_info[0], PermissionError):
            def resetperms(path):
                try:
                    _os.chflags(path, 0)
                except AttributeError:
                    pass
                _os.chmod(path, 0o700)
    
            try:
                if path != name:
                    resetperms(_os.path.dirname(path))
                resetperms(path)
    
                try:
>                   _os.unlink(path)
E                   PermissionError: [WinError 5] Access is denied: 'C:\Users\RUNNER~1\AppData\Local\Temp\mypy-test-6t6j6e1l\tmp'

C:\hostedtoolcache\windows\Python\3.8.10\x64\lib\tempfile.py:802: PermissionError
```

This likely can be solved by `TemporaryDirectory.ignore_cleanup_errors`
but it is only available on 3.10+. I hope this PR does not mask any real
deficiency - those failures are extremely annoying.
  • Loading branch information
sterliakov authored Nov 12, 2024
1 parent f067db4 commit 1ec2ef3
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 13 deletions.
15 changes: 6 additions & 9 deletions mypy/test/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ def __init__(
self.data = data
self.line = line
self.old_cwd: str | None = None
self.tmpdir: tempfile.TemporaryDirectory[str] | None = None
self.tmpdir: str | None = None

def runtest(self) -> None:
if self.skip:
Expand All @@ -323,19 +323,19 @@ def runtest(self) -> None:
save_dir: str | None = self.config.getoption("--save-failures-to", None)
if save_dir:
assert self.tmpdir is not None
target_dir = os.path.join(save_dir, os.path.basename(self.tmpdir.name))
target_dir = os.path.join(save_dir, os.path.basename(self.tmpdir))
print(f"Copying data from test {self.name} to {target_dir}")
if not os.path.isabs(target_dir):
assert self.old_cwd
target_dir = os.path.join(self.old_cwd, target_dir)
shutil.copytree(self.tmpdir.name, target_dir)
shutil.copytree(self.tmpdir, target_dir)
raise

def setup(self) -> None:
parse_test_case(case=self)
self.old_cwd = os.getcwd()
self.tmpdir = tempfile.TemporaryDirectory(prefix="mypy-test-")
os.chdir(self.tmpdir.name)
self.tmpdir = tempfile.mkdtemp(prefix="mypy-test-")
os.chdir(self.tmpdir)
os.mkdir(test_temp_dir)

# Precalculate steps for find_steps()
Expand Down Expand Up @@ -371,10 +371,7 @@ def teardown(self) -> None:
if self.old_cwd is not None:
os.chdir(self.old_cwd)
if self.tmpdir is not None:
try:
self.tmpdir.cleanup()
except OSError:
pass
shutil.rmtree(self.tmpdir, ignore_errors=True)
self.old_cwd = None
self.tmpdir = None

Expand Down
8 changes: 4 additions & 4 deletions mypy/test/testfinegrained.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ def run_case(self, testcase: DataDrivenTestCase) -> None:
if messages:
a.extend(normalize_messages(messages))

assert testcase.tmpdir
a.extend(self.maybe_suggest(step, server, main_src, testcase.tmpdir.name))
assert testcase.tmpdir is not None
a.extend(self.maybe_suggest(step, server, main_src, testcase.tmpdir))
a.extend(self.maybe_inspect(step, server, main_src))

if server.fine_grained_manager:
Expand Down Expand Up @@ -248,8 +248,8 @@ def perform_step(
new_messages = normalize_messages(new_messages)

a = new_messages
assert testcase.tmpdir
a.extend(self.maybe_suggest(step, server, main_src, testcase.tmpdir.name))
assert testcase.tmpdir is not None
a.extend(self.maybe_suggest(step, server, main_src, testcase.tmpdir))
a.extend(self.maybe_inspect(step, server, main_src))

return a, triggered
Expand Down

0 comments on commit 1ec2ef3

Please sign in to comment.