[internal] better align test JVM lockfile support with pytest concepts (

#15804)

The JVM test lockfile support had a significant limitation where only one `jvm_lockfile` fixture can be passed into a test function. This is because of a limitation with pytest marks where a mark can only be applied to a test function and not to fixture functions. Thus, the `jvm_lockfile` fixture was essentially a singleton because it was not possible to declare separate fixtures which were marked individually with the `pytest.mark.jvm_lockfile`.

This PR rewrites the test lockfile support to operate like regular pytest fixture functions. The main caveat is that the test lockfile definition must be placed in a separate fixture function to allow extracting it easily. Also, the file containing a definition must use `from __future__ import annotations` so that the type annotations are available to the extractor.

With this PR, there is no longer any need to register `JvmLockfilePlugin` in a `conftest.py file`, which further simplifies using the test lockfile support.

Moreover, it allows using multiple lockfiles in the `test_compile_with_multiple_scala_versions` test which was not possible without this PR.

[ci skip-rust]

[ci skip-build-wheels]

pants-plugins/internal_plugins/test_lockfile_fixtures/collect_fixtures.py

@@ -0,0 +1,81 @@
+# Copyright 2022 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+import inspect
+import json
+import sys
+from pathlib import Path
+
+import pytest
+from _pytest import fixtures
+from _pytest.compat import get_real_func
+
+
+def get_func_path(func):
+    real_func = get_real_func(func)
+    return inspect.getfile(real_func)
+
+
+def get_fixturedef(fixture_request, name):
+    fixturedef = fixture_request._fixture_defs.get(name)
+    if fixturedef:
+        return fixturedef
+    try:
+        return fixture_request._getnextfixturedef(name)
+    except fixtures.FixtureLookupError:
+        return None
+
+
+def process_fixtures(item):
+    lockfile_definitions = []
+    fixture_request = fixtures.FixtureRequest(item, _ispytest=True)
+    for fixture_name in fixture_request.fixturenames:
+        fixture_def = get_fixturedef(fixture_request, fixture_name)
+        if not fixture_def:
+            continue
+
+        func = fixture_def.func
+        annotations = getattr(func, "__annotations__")
+        if not annotations or annotations.get("return") != "JVMLockfileFixtureDefinition":
+            continue
+
+        # Note: We just invoke the fixture_def function assuming it takes no arguments. The other two
+        # ways of invoking for the fixture value cause errors. I have left them here commented-out as an example
+        # of what failed:
+        #   lockfile_definition = fixture_request.getfixturevalue(fixture_name)
+        #   lockfile_definition = fixture_def.execute(request=request)
+        lockfile_definition = func()
+        if lockfile_definition.__class__.__name__ != "JVMLockfileFixtureDefinition":
+            continue
+
+        cwd = Path.cwd()
+        func_path = Path(get_func_path(func)).relative_to(cwd)
+        lockfile_definitions.append(
+            {
+                "lockfile_rel_path": str(lockfile_definition.lockfile_rel_path),
+                "requirements": [c.to_coord_str() for c in lockfile_definition.requirements],
+                "test_file_path": str(func_path),
+            }
+        )
+    return lockfile_definitions
+
+
+class CollectionPlugin:
+    def __init__(self):
+        self.collected = []
+
+    def pytest_collection_modifyitems(self, items):
+        for item in items:
+            self.collected.append(item)
+
+
+collection_plugin = CollectionPlugin()
+pytest.main(["--setup-only", *sys.argv[1:]], plugins=[collection_plugin])
+
+output = []
+cwd = Path.cwd()
+
+for item in collection_plugin.collected:
+    output.extend(process_fixtures(item))
+
+with open("tests.json", "w") as f:
+    f.write(json.dumps(output))

pants-plugins/internal_plugins/test_lockfile_fixtures/lockfile_fixture.py

@@ -5,27 +5,30 @@
 import textwrap
 from dataclasses import dataclass
 from pathlib import Path
+from typing import Iterable
 
-import pytest
+from _pytest.fixtures import FixtureRequest
 
 from pants.jvm.resolve.common import ArtifactRequirement, ArtifactRequirements, Coordinate
 from pants.jvm.resolve.coursier_fetch import CoursierResolvedLockfile
 from pants.jvm.resolve.lockfile_metadata import LockfileContext
 from pants.util.docutil import bin_name
+from pants.util.meta import frozen_after_init
 
 
-@dataclass(frozen=True)
+@frozen_after_init
+@dataclass(unsafe_hash=True)
 class JVMLockfileFixtureDefinition:
     lockfile_rel_path: Path
-    coordinates: tuple[Coordinate, ...]
+    requirements: tuple[Coordinate, ...]
 
-    @classmethod
-    def from_kwargs(cls, kwargs) -> JVMLockfileFixtureDefinition:
-        lockfile_rel_path = kwargs["path"]
-        if not lockfile_rel_path:
-            raise ValueError("`path` must be specified as a relative path to a lockfile")
+    def __init__(
+        self, lockfile_rel_path: Path | str, requirements: Iterable[Coordinate | str]
+    ) -> None:
+        self.lockfile_rel_path = (
+            lockfile_rel_path if isinstance(lockfile_rel_path, Path) else Path(lockfile_rel_path)
+        )
 
-        requirements = kwargs["requirements"] or []
         coordinates: list[Coordinate] = []
         for requirement in requirements:
             if isinstance(requirement, Coordinate):
@@ -34,15 +37,44 @@ def from_kwargs(cls, kwargs) -> JVMLockfileFixtureDefinition:
                 coordinate = Coordinate.from_coord_str(requirement)
                 coordinates.append(coordinate)
             else:
-                raise ValueError(
+                raise TypeError(
                     f"Unsupported type `{type(requirement)}` for JVM coordinate. Expected `Coordinate` or `str`."
                 )
+        self.requirements = tuple(coordinates)
 
+    @classmethod
+    def from_json_dict(cls, kwargs) -> JVMLockfileFixtureDefinition:
+        lockfile_rel_path = kwargs["lockfile_rel_path"]
+        if not lockfile_rel_path:
+            raise ValueError("`path` must be specified as a relative path to a lockfile")
+
+        requirements = kwargs["requirements"] or []
         return cls(
             lockfile_rel_path=Path(lockfile_rel_path),
-            coordinates=tuple(coordinates),
+            requirements=requirements,
         )
 
+    def load(self, request: FixtureRequest) -> JVMLockfileFixture:
+        lockfile_path = request.node.path.parent / self.lockfile_rel_path
+        lockfile_contents = lockfile_path.read_bytes()
+        lockfile = CoursierResolvedLockfile.from_serialized(lockfile_contents)
+
+        # Check the lockfile's requirements against the requirements in the lockfile.
+        # Fail the test if the lockfile needs to be regenerated.
+        artifact_reqs = ArtifactRequirements(
+            [ArtifactRequirement(coordinate) for coordinate in self.requirements]
+        )
+        if not lockfile.metadata:
+            raise ValueError(f"Expected JVM lockfile {self.lockfile_rel_path} to have metadata.")
+        if not lockfile.metadata.is_valid_for(artifact_reqs, LockfileContext.TOOL):
+            raise ValueError(
+                f"Lockfile fixture {self.lockfile_rel_path} is not valid. "
+                "Please re-generate it using: "
+                f"{bin_name()} internal-generate-test-lockfile-fixtures ::"
+            )
+
+        return JVMLockfileFixture(lockfile, lockfile_contents.decode(), artifact_reqs)
+
 
 @dataclass(frozen=True)
 class JVMLockfileFixture:
@@ -69,40 +101,3 @@ def requirements_as_jvm_artifact_targets(
             """
             )
         return targets
-
-
-class JvmLockfilePlugin:
-    def pytest_configure(self, config):
-        config.addinivalue_line(
-            "markers",
-            "jvm_lockfile(path, requirements): mark test to configure a `jvm_lockfile` fixture",
-        )
-
-    @pytest.fixture
-    def jvm_lockfile(self, request) -> JVMLockfileFixture:
-        mark = request.node.get_closest_marker("jvm_lockfile")
-
-        definition = JVMLockfileFixtureDefinition.from_kwargs(mark.kwargs)
-
-        # Load the lockfile.
-        lockfile_path = request.node.path.parent / definition.lockfile_rel_path
-        lockfile_contents = lockfile_path.read_bytes()
-        lockfile = CoursierResolvedLockfile.from_serialized(lockfile_contents)
-
-        # Check the lockfile's requirements against the requirements in the lockfile.
-        # Fail the test if the lockfile needs to be regenerated.
-        artifact_reqs = ArtifactRequirements(
-            [ArtifactRequirement(coordinate) for coordinate in definition.coordinates]
-        )
-        if not lockfile.metadata:
-            raise ValueError(
-                f"Expected JVM lockfile {definition.lockfile_rel_path} to have metadata."
-            )
-        if not lockfile.metadata.is_valid_for(artifact_reqs, LockfileContext.TOOL):
-            raise ValueError(
-                f"Lockfile fixture {definition.lockfile_rel_path} is not valid. "
-                "Please re-generate it using: "
-                f"{bin_name()} internal-generate-test-lockfile-fixtures ::"
-            )
-
-        return JVMLockfileFixture(lockfile, lockfile_contents.decode(), artifact_reqs)

pants-plugins/internal_plugins/test_lockfile_fixtures/rules.py

@@ -4,6 +4,7 @@
 
 import json
 import os
+import pkgutil
 from dataclasses import dataclass
 from pathlib import PurePath
 
@@ -36,38 +37,6 @@
 from pants.util.docutil import bin_name
 from pants.util.logging import LogLevel
 
-COLLECTION_SCRIPT = r"""\
-from pathlib import Path
-import json
-import sys
-
-import pytest
-
-class CollectionPlugin:
-    def __init__(self):
-        self.collected = []
-
-    def pytest_collection_modifyitems(self, items):
-        for item in items:
-            self.collected.append(item)
-
-
-collection_plugin = CollectionPlugin()
-pytest.main(["--collect-only", *sys.argv[1:]], plugins=[collection_plugin])
-output = []
-cwd = Path.cwd()
-for item in collection_plugin.collected:
-    for mark in item.iter_markers("jvm_lockfile"):
-        path = Path(item.path).relative_to(cwd)
-        output.append({
-            "kwargs": mark.kwargs,
-            "test_file_path": str(path),
-        })
-
-with open("tests.json", "w") as f:
-    f.write(json.dumps(output))
-"""
-
 
 @dataclass(frozen=True)
 class JVMLockfileFixtureConfig:
@@ -131,8 +100,13 @@ async def collect_fixture_configs(
         ),
     )
 
+    script_content_bytes = pkgutil.get_data(__name__, "collect_fixtures.py")
+    if not script_content_bytes:
+        raise AssertionError("Did not find collect_fixtures.py script as resouce.")
     script_content = FileContent(
-        path="collect-fixtures.py", content=COLLECTION_SCRIPT.encode(), is_executable=True
+        path="collect_fixtures.py",
+        content=script_content_bytes,
+        is_executable=True,
     )
     script_digest = await Get(Digest, CreateDigest([script_content]))
 
@@ -198,7 +172,7 @@ async def collect_fixture_configs(
     configs = []
     for item in raw_config_data:
         config = JVMLockfileFixtureConfig(
-            definition=JVMLockfileFixtureDefinition.from_kwargs(item["kwargs"]),
+            definition=JVMLockfileFixtureDefinition.from_json_dict(item),
             test_file_path=item["test_file_path"],
         )
         configs.append(config)
@@ -212,7 +186,7 @@ async def gather_lockfile_fixtures() -> RenderedJVMLockfileFixtures:
     rendered_fixtures = []
     for config in configs:
         artifact_reqs = ArtifactRequirements(
-            [ArtifactRequirement(coordinate) for coordinate in config.definition.coordinates]
+            [ArtifactRequirement(coordinate) for coordinate in config.definition.requirements]
         )
         lockfile = await Get(CoursierResolvedLockfile, ArtifactRequirements, artifact_reqs)
         serialized_lockfile = JVMLockfileMetadata.new(artifact_reqs).add_header_to_lockfile(

src/python/pants/backend/kotlin/BUILD

@@ -2,5 +2,3 @@
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
 python_sources()
-
-python_test_utils(name="test_utils")