MAINT Unvendor tests at the lockfile creation step

integration_tests/Makefile

@@ -6,7 +6,7 @@ all:
 test-recipe: check
 	@echo "... Running integration tests for building recipes"
 
-	pyodide build-recipes --recipe-dir=recipes --force-rebuild "*"
+	pyodide build-recipes --recipe-dir=recipes --install --force-rebuild "*"
 
 	@echo "... Passed"
 

pyodide_build/recipe/builder.py

@@ -548,7 +548,7 @@ def _package_wheel(
     ) -> None:
         """Package a wheel
 
-        This unpacks the wheel, unvendors tests if necessary, runs and "build.post"
+        This unpacks the wheel, runs and "build.post"
         script, and then repacks the wheel.
 
         Parameters
@@ -606,18 +606,6 @@ def _package_wheel(
                     host_site_packages / cross_build_file,
                 )
 
-            try:
-                test_dir = self.src_dist_dir / "tests"
-                if self.build_metadata.unvendor_tests:
-                    nmoved = unvendor_tests(
-                        wheel_dir, test_dir, self.build_metadata.retain_test_patterns
-                    )
-                    if nmoved:
-                        with chdir(self.src_dist_dir):
-                            shutil.make_archive(f"{self.name}-tests", "tar", test_dir)
-            finally:
-                shutil.rmtree(test_dir, ignore_errors=True)
-
 
 class RecipeBuilderStaticLibrary(RecipeBuilder):
     """
@@ -745,60 +733,6 @@ def copy_sharedlibs(
     return {}
 
 
-def unvendor_tests(
-    install_prefix: Path, test_install_prefix: Path, retain_test_patterns: list[str]
-) -> int:
-    """Unvendor test files and folders
-
-    This function recursively walks through install_prefix and moves anything
-    that looks like a test folder under test_install_prefix.
-
-
-    Parameters
-    ----------
-    install_prefix
-        the folder where the package was installed
-    test_install_prefix
-        the folder where to move the tests. If it doesn't exist, it will be
-        created.
-
-    Returns
-    -------
-    n_moved
-        number of files or folders moved
-    """
-    n_moved = 0
-    out_files = []
-    shutil.rmtree(test_install_prefix, ignore_errors=True)
-    for root, _dirs, files in os.walk(install_prefix):
-        root_rel = Path(root).relative_to(install_prefix)
-        if root_rel.name == "__pycache__" or root_rel.name.endswith(".egg_info"):
-            continue
-        if root_rel.name in ["test", "tests"]:
-            # This is a test folder
-            (test_install_prefix / root_rel).parent.mkdir(exist_ok=True, parents=True)
-            shutil.move(install_prefix / root_rel, test_install_prefix / root_rel)
-            n_moved += 1
-            continue
-        out_files.append(root)
-        for fpath in files:
-            if (
-                fnmatch.fnmatchcase(fpath, "test_*.py")
-                or fnmatch.fnmatchcase(fpath, "*_test.py")
-                or fpath == "conftest.py"
-            ):
-                if any(fnmatch.fnmatchcase(fpath, pat) for pat in retain_test_patterns):
-                    continue
-                (test_install_prefix / root_rel).mkdir(exist_ok=True, parents=True)
-                shutil.move(
-                    install_prefix / root_rel / fpath,
-                    test_install_prefix / root_rel / fpath,
-                )
-                n_moved += 1
-
-    return n_moved
-
-
 # TODO: move this to common.py or somewhere else
 def needs_rebuild(
     pkg_root: Path,

pyodide_build/recipe/graph_builder.py

@@ -38,7 +38,7 @@
     repack_zip_archive,
 )
 from pyodide_build.logger import console_stdout, logger
-from pyodide_build.recipe import loader
+from pyodide_build.recipe import loader, unvendor
 from pyodide_build.recipe.builder import needs_rebuild
 from pyodide_build.recipe.spec import MetaConfig, _BuildSpecTypes
 
@@ -65,7 +65,7 @@ class BasePackage:
     dependencies: set[str]  # run + host dependencies
     unbuilt_host_dependencies: set[str]
     host_dependents: set[str]
-    unvendored_tests: Path | None = None
+    unvendor_tests: bool = False
     file_name: str | None = None
     install_dir: str = "site"
     _queue_idx: int | None = None
@@ -173,18 +173,14 @@ def dist_artifact_path(self) -> Path | None:
         """
         raise NotImplementedError()
 
-    def tests_path(self) -> Path | None:
-        """
-        Return the path to the unvendored tests of the package.
-        """
-        raise NotImplementedError()
-
 
 @dataclasses.dataclass
 class PythonPackage(BasePackage):
     def __init__(self, pkgdir: Path, config: MetaConfig) -> None:
         super().__init__(pkgdir, config)
 
+        self.unvendor_tests = self.meta.build.unvendor_tests
+
     def dist_artifact_path(self) -> Path | None:
         dist_dir = self.pkgdir / "dist"
         wheel = find_matching_wheel(
@@ -194,13 +190,6 @@ def dist_artifact_path(self) -> Path | None:
             raise RuntimeError(f"Found no wheel while building {self.name}")
         return wheel
 
-    def tests_path(self) -> Path | None:
-        tests = list((self.pkgdir / "dist").glob("*-tests.tar"))
-        assert len(tests) <= 1
-        if tests:
-            return tests[0]
-        return None
-
 
 @dataclasses.dataclass
 class SharedLibrary(BasePackage):
@@ -220,9 +209,6 @@ def dist_artifact_path(self) -> Path | None:
 
         return candidates[0]
 
-    def tests_path(self) -> Path | None:
-        return None
-
 
 @dataclasses.dataclass
 class StaticLibrary(BasePackage):
@@ -232,9 +218,6 @@ def __init__(self, pkgdir: Path, config: MetaConfig) -> None:
     def dist_artifact_path(self) -> Path | None:
         return None
 
-    def tests_path(self) -> Path | None:
-        return None
-
 
 class PackageStatus:
     def __init__(
@@ -833,7 +816,9 @@ def generate_packagedata(
 
         if not pkg.file_name or pkg.package_type == "static_library":
             continue
-        if not Path(output_dir, pkg.file_name).exists():
+
+        wheel_file = output_dir / pkg.file_name
+        if not wheel_file.exists():
             continue
         pkg_entry = PackageLockSpec(
             name=name,
@@ -843,32 +828,29 @@ def generate_packagedata(
             package_type=pkg.package_type,
         )
 
-        update_package_sha256(pkg_entry, output_dir / pkg.file_name)
-
         pkg_entry.depends = [x.lower() for x in pkg.run_dependencies]
 
         if pkg.package_type not in ("static_library", "shared_library"):
             pkg_entry.imports = (
                 pkg.meta.package.top_level if pkg.meta.package.top_level else [name]
             )
 
-        packages[normalized_name.lower()] = pkg_entry
-
-        if pkg.unvendored_tests:
-            packages[normalized_name.lower()].unvendored_tests = True
-
-            # Create the test package if necessary
-            pkg_entry = PackageLockSpec(
-                name=name + "-tests",
-                version=pkg.version,
-                depends=[name.lower()],
-                file_name=pkg.unvendored_tests.name,
-                install_dir=pkg.install_dir,
-            )
-
-            update_package_sha256(pkg_entry, output_dir / pkg.unvendored_tests.name)
+        if pkg.unvendor_tests:
+            unvendored_test_file = unvendor.unvendor_tests_in_wheel(wheel_file)
+            if unvendored_test_file:
+                pkg_entry.unvendored_tests = True
+                test_file_entry = PackageLockSpec(
+                    name=f"{name}-tests",
+                    version=pkg.version,
+                    depends=[name],
+                    file_name=unvendored_test_file.name,
+                    install_dir=pkg.install_dir,
+                )
+                update_package_sha256(test_file_entry, unvendored_test_file)
+                packages[normalized_name + "-tests"] = test_file_entry
 
-            packages[normalized_name.lower() + "-tests"] = pkg_entry
+        update_package_sha256(pkg_entry, wheel_file)
+        packages[normalized_name] = pkg_entry
 
     # sort packages by name
     packages = dict(sorted(packages.items()))
@@ -916,10 +898,6 @@ def copy_packages_to_dist_dir(
                     output_dir / f"{dist_artifact_path.name}.metadata",
                 )
 
-        test_path = pkg.tests_path()
-        if test_path:
-            shutil.copy(test_path, output_dir)
-
 
 def build_packages(
     packages_dir: Path,
@@ -938,14 +916,10 @@ def build_packages(
     build_from_graph(pkg_map, build_args, build_dir, n_jobs, force_rebuild)
     for pkg in pkg_map.values():
         dist_path = pkg.dist_artifact_path()
-        test_path = pkg.tests_path()
 
         if dist_path:
             pkg.file_name = dist_path.name
 
-        if test_path:
-            pkg.unvendored_tests = test_path
-
     return pkg_map
 
 

pyodide_build/recipe/unvendor.py

@@ -0,0 +1,114 @@
+import fnmatch
+import os
+import shutil
+from pathlib import Path
+from tempfile import TemporaryDirectory
+
+from packaging.utils import parse_wheel_filename
+
+from pyodide_build.common import (
+    chdir,
+    modify_wheel,
+)
+
+
+def unvendor_tests_in_wheel(
+    wheel: Path, retain_patterns: list[str] | None = None
+) -> Path | None:
+    """
+    Unvendor tests from a wheel file.
+
+    This function finds the tests in the wheel file, and extracts them to a separate
+    tar file. The tar file is placed in the same directory as the wheel file, with the -tests.tar suffix.
+
+    Parameters
+    ----------
+    wheel
+        The path to the wheel file.
+
+    retain_patterns
+        A list of patterns to retain in the tests. If a pattern is found in the tests, it will be retained.
+
+    Returns
+    -------
+    The path to the tar file containing the tests. If no tests were found, returns None.
+    """
+    retain_patterns = retain_patterns or []
+
+    name = parse_wheel_filename(wheel.name)[0]
+    file_format = "tar"
+    basename = f"{name}-tests"
+    fullname = f"{basename}.{file_format}"
+    destination = wheel.parent / fullname
+
+    with TemporaryDirectory() as _tmpdir:
+        tmpdir = Path(_tmpdir)
+        test_dir = tmpdir / "tests"
+        with modify_wheel(wheel) as wheel_extract_dir:
+            nmoved = unvendor_tests(
+                wheel_extract_dir,
+                test_dir,
+                retain_patterns,
+            )
+            if not nmoved:
+                return None
+
+            with chdir(tmpdir):
+                generated_file = shutil.make_archive(basename, file_format, test_dir)
+                shutil.move(tmpdir / generated_file, destination)
+
+    return destination
+
+
+def unvendor_tests(
+    install_prefix: Path, test_install_prefix: Path, retain_test_patterns: list[str]
+) -> int:
+    """Unvendor test files and folders
+
+    This function recursively walks through install_prefix and moves anything
+    that looks like a test folder under test_install_prefix.
+
+
+    Parameters
+    ----------
+    install_prefix
+        the folder where the package was installed
+    test_install_prefix
+        the folder where to move the tests. If it doesn't exist, it will be
+        created.
+
+    Returns
+    -------
+    n_moved
+        number of files or folders moved
+    """
+    n_moved = 0
+    out_files = []
+    shutil.rmtree(test_install_prefix, ignore_errors=True)
+    for root, _dirs, files in os.walk(install_prefix):
+        root_rel = Path(root).relative_to(install_prefix)
+        if root_rel.name == "__pycache__" or root_rel.name.endswith(".egg_info"):
+            continue
+        if root_rel.name in {"test", "tests"}:
+            # This is a test folder
+            (test_install_prefix / root_rel).parent.mkdir(exist_ok=True, parents=True)
+            shutil.move(install_prefix / root_rel, test_install_prefix / root_rel)
+            n_moved += 1
+            continue
+        out_files.append(root)
+        for fpath in files:
+            if (
+                fnmatch.fnmatchcase(fpath, "test_*.py")
+                or fnmatch.fnmatchcase(fpath, "*_test.py")
+                or fpath == "conftest.py"
+            ):
+                if any(fnmatch.fnmatchcase(fpath, pat) for pat in retain_test_patterns):
+                    continue
+                (test_install_prefix / root_rel).mkdir(exist_ok=True, parents=True)
+                shutil.move(
+                    install_prefix / root_rel / fpath,
+                    test_install_prefix / root_rel / fpath,
+                )
+                n_moved += 1
+
+    return n_moved