Unify allowlist handling (#11889)

Co-authored-by: Jelle Zijlstra <[email protected]>
Co-authored-by: Ali Hamdan <[email protected]>

Author: 3 people

tests/stubtest_stdlib.py

@@ -6,19 +6,21 @@
 In typeshed CI, we run stubtest with each currently supported Python minor version.
 
 """
+
 from __future__ import annotations
 
 import subprocess
 import sys
 from pathlib import Path
 
+from utils import allowlist_stubtest_arguments, allowlists_path
+
 
 def run_stubtest(typeshed_dir: Path) -> int:
-    allowlist_dir = typeshed_dir / "tests" / "stubtest_allowlists"
     version_allowlist = f"py{sys.version_info.major}{sys.version_info.minor}.txt"
-    platform_allowlist = f"{sys.platform}.txt"
     combined_allowlist = f"{sys.platform}-py{sys.version_info.major}{sys.version_info.minor}.txt"
     local_version_allowlist = version_allowlist + ".local"
+    extra_allowlists = [version_allowlist, combined_allowlist, local_version_allowlist]
 
     # Note when stubtest imports distutils, it will likely actually import setuptools._distutils
     # This is fine because we don't care about distutils and allowlist all errors from it
@@ -31,17 +33,8 @@ def run_stubtest(typeshed_dir: Path) -> int:
         "--check-typeshed",
         "--custom-typeshed-dir",
         str(typeshed_dir),
-        "--allowlist",
-        str(allowlist_dir / "py3_common.txt"),
-        "--allowlist",
-        str(allowlist_dir / version_allowlist),
+        *allowlist_stubtest_arguments("stdlib", extra_allowlists),
     ]
-    if (allowlist_dir / platform_allowlist).exists():
-        cmd += ["--allowlist", str(allowlist_dir / platform_allowlist)]
-    if (allowlist_dir / combined_allowlist).exists():
-        cmd += ["--allowlist", str(allowlist_dir / combined_allowlist)]
-    if (allowlist_dir / local_version_allowlist).exists():
-        cmd += ["--allowlist", str(allowlist_dir / local_version_allowlist)]
     if sys.version_info < (3, 10):
         # As discussed in https://github.com/python/typeshed/issues/3693, we only aim for
         # positional-only arg accuracy for python 3.10 and above.
@@ -58,7 +51,10 @@ def run_stubtest(typeshed_dir: Path) -> int:
             file=sys.stderr,
         )
         print("\n\n", file=sys.stderr)
-        print(f'To fix "unused allowlist" errors, remove the corresponding entries from {allowlist_dir}', file=sys.stderr)
+        print(
+            f'To fix "unused allowlist" errors, remove the corresponding entries from {allowlists_path("stdlib")}',
+            file=sys.stderr,
+        )
         return e.returncode
     else:
         print("stubtest succeeded", file=sys.stderr)

tests/stubtest_third_party.py

@@ -13,7 +13,17 @@
 from typing import NoReturn
 
 from parse_metadata import NoSuchStubError, get_recursive_requirements, read_metadata
-from utils import PYTHON_VERSION, colored, get_mypy_req, print_divider, print_error, print_success_msg, tests_path
+from utils import (
+    PYTHON_VERSION,
+    allowlist_stubtest_arguments,
+    allowlists_path,
+    colored,
+    get_mypy_req,
+    print_divider,
+    print_error,
+    print_success_msg,
+    tests_path,
+)
 
 
 def run_stubtest(
@@ -112,12 +122,7 @@ def run_stubtest(
         # "bisect" to see which variables are actually needed.
         stubtest_env = os.environ | {"MYPYPATH": mypypath, "MYPY_FORCE_COLOR": "1"}
 
-        allowlist_path = tests_path(dist_name) / "stubtest_allowlist.txt"
-        if allowlist_path.exists():
-            stubtest_cmd.extend(["--allowlist", str(allowlist_path)])
-        platform_allowlist = tests_path(dist_name) / f"stubtest_allowlist_{sys.platform}.txt"
-        if platform_allowlist.exists():
-            stubtest_cmd.extend(["--allowlist", str(platform_allowlist)])
+        stubtest_cmd += allowlist_stubtest_arguments(dist_name, [])
 
         # Perform some black magic in order to run stubtest inside uWSGI
         if dist_name == "uWSGI":
@@ -150,11 +155,12 @@ def run_stubtest(
             print_command_output(ret)
 
             print_divider()
-            if allowlist_path.exists():
-                print(f'To fix "unused allowlist" errors, remove the corresponding entries from {allowlist_path}')
+            main_allowlist_path = allowlists_path(dist_name) / "stubtest_allowlist.txt"
+            if main_allowlist_path.exists():
+                print(f'To fix "unused allowlist" errors, remove the corresponding entries from {main_allowlist_path}')
                 print()
             else:
-                print(f"Re-running stubtest with --generate-allowlist.\nAdd the following to {allowlist_path}:")
+                print(f"Re-running stubtest with --generate-allowlist.\nAdd the following to {main_allowlist_path}:")
                 ret = subprocess.run([*stubtest_cmd, "--generate-allowlist"], env=stubtest_env, capture_output=True)
                 print_command_output(ret)
 

tests/utils.py

@@ -161,6 +161,20 @@ def get_all_testcase_directories() -> list[DistributionTests]:
     return [distribution_info("stdlib"), *sorted(testcase_directories)]
 
 
+def allowlists_path(distribution_name: str) -> Path:
+    if distribution_name == "stdlib":
+        return Path("tests", "stubtest_allowlists")
+    else:
+        return tests_path(distribution_name)
+
+
+def common_allowlists(distribution_name: str) -> list[str]:
+    if distribution_name == "stdlib":
+        return ["py3_common.txt", f"{sys.platform}.txt"]
+    else:
+        return ["stubtest_allowlist.txt", f"stubtest_allowlist_{sys.platform}.txt"]
+
+
 # ====================================================================
 # Parsing .gitignore
 # ====================================================================
@@ -177,3 +191,17 @@ def spec_matches_path(spec: pathspec.PathSpec, path: Path) -> bool:
     if path.is_dir():
         normalized_path += "/"
     return spec.match_file(normalized_path)
+
+
+# ====================================================================
+# mypy call
+# ====================================================================
+
+
+def allowlist_stubtest_arguments(distribution_name: str, additional_allowlists: list[str]) -> list[str]:
+    stubtest_arguments: list[str] = []
+    for allowlist in common_allowlists(distribution_name) + additional_allowlists:
+        path = allowlists_path(distribution_name) / allowlist
+        if path.exists():
+            stubtest_arguments.extend(["--allowlist", str(path)])
+    return stubtest_arguments