Skip to content

Fix mypy test temporary config file creation#13620

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.

Sign up for GitHub

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

Merged
srittau merged 5 commits into from
Mar 14, 2025
Merged

Fix mypy test temporary config file creation #13620

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3a5da88
Fix mypy test temporary config file creation
Avasam Mar 11, 2025
9e49757
typo forgot to push argument
Avasam Mar 11, 2025
acc33df
Use delete instead of delete_on_close for previous Python versions
Avasam Mar 11, 2025
6942985
Extract contextlib workaround
Avasam Mar 11, 2025
94f9890
pyright: ignore[reportPrivateUsage]
Avasam Mar 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions tests/mypy_test.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,12 +5,14 @@

import argparse
import concurrent.futures
import functools
import os
import subprocess
import sys
import tempfile
import time
from collections import defaultdict
from collections.abc import Generator
from dataclasses import dataclass
from enum import Enum
from itertools import product
Expand DownExpand Up@@ -44,6 +46,24 @@
print_error("Cannot import mypy. Did you install it?")
sys.exit(1)

# We need to work around a limitation of tempfile.NamedTemporaryFile on Windows
# For details, see https://github.com/python/typeshed/pull/13620#discussion_r1990185997
# Python 3.12 added a workaround with `tempfile.NamedTemporaryFile("w+", delete_on_close=False)`
if sys.platform != "win32":

@sobolevnsobolevnMar 12, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If any contributor runs this on win, it will be a NameError to access _named_temporary_file. I guess we have to explicitly say that win32 platform is not supported for running tests.

@AkuliAkuliMar 12, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Windows, _named_temporary_file is defined in the else branch. The idea is to work around a limitation in Windows.

Maybe this would be more clear if the condition was inverted:

ifsys.platform=="win32":
...
else:
...

@sobolevnsobolevnMar 12, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally missed that, thank you!

@AvasamAvasamMar 12, 2025

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally had a "if python 3.12" branch first. But figured it's just introducing a different behaviour for another set of variables, so I removed it in favor of a simple comment.

It made more sense then to have the two different functools.partial first, then the bigger block.

But now I don't mind flipping the condition for readability.

_named_temporary_file = functools.partial(tempfile.NamedTemporaryFile, "w+")
else:
from contextlib import contextmanager

@contextmanager
def _named_temporary_file() -> Generator[tempfile._TemporaryFileWrapper[str]]: # pyright: ignore[reportPrivateUsage]
temp = tempfile.NamedTemporaryFile("w+", delete=False) # noqa: SIM115
try:
yield temp
finally:
temp.close()
os.remove(temp.name)


SUPPORTED_VERSIONS = ["3.13", "3.12", "3.11", "3.10", "3.9"]
SUPPORTED_PLATFORMS = ("linux", "win32", "darwin")
DIRECTORIES_TO_TEST = [STDLIB_PATH, STUBS_PATH]
Expand DownExpand Up@@ -214,7 +234,8 @@ def run_mypy(
env_vars = dict(os.environ)
if mypypath is not None:
env_vars["MYPYPATH"] = mypypath
with tempfile.NamedTemporaryFile("w+") as temp:

with _named_temporary_file() as temp:
temp.write("[mypy]\n")
for dist_conf in configurations:
temp.write(f"[mypy-{dist_conf.module_name}]\n")
Expand DownExpand Up@@ -290,7 +311,7 @@ def add_third_party_files(
if name.startswith("."):
continue
add_files(files, (root / name), args)
add_configuration(configurations, distribution)
add_configuration(configurations, distribution)


class TestResult(NamedTuple):
Expand Down