Move Pants stub bootstrap to conftest.py and clean test_rules.py imports

[Copilot]

py/pants-toolshed/toolshed_publish_reqs/test_rules.py mixed import-time stub bootstrapping with test code, causing F811 (pants redefinition), late imports (E402 suppressions), and brittle module setup.
This change moves Pants stub installation into sibling conftest.py so pytest loads stubs before test module import, allowing test_rules.py to remain a normal test module.

• Stub bootstrap moved to pytest lifecycle

  • Added py/pants-toolshed/toolshed_publish_reqs/conftest.py.
  • Moved _install_pants_stubs() and try: import pants / except ModuleNotFoundError there.
  • Kept stub behavior unchanged, including _FakeTargetAdaptor.
  • Renamed local stub variable from pants → pants_module to avoid name collision with import pants.

• test_rules.py simplified to standard imports

  • Removed sys, types, _install_pants_stubs, and module-level try/except.
  • Removed late imports and all # noqa: E402.
  • Kept tests and assertions intact, with imports now at top-of-file only:
    • import pytest
    • from toolshed_publish_reqs.names import _publish_req_target_name
    • from toolshed_publish_reqs.rules import _adaptors_for_install_requires

• BUILD target shape unchanged

  • No BUILD edits; conftest.py is consumed as source alongside tests under existing target layout.

Example of the new import flow:

# conftest.py
try:
    import pants  # noqa: F401
except ModuleNotFoundError:
    _install_pants_stubs()

# test_rules.py
import pytest
from toolshed_publish_reqs.names import _publish_req_target_name
from toolshed_publish_reqs.rules import _adaptors_for_install_requires

Original prompt

Context

This PR is being raised against the branch pants-reqs in phlax/toolshed (the head branch of envoyproxy#4188). Current head SHA is 7d55701de9caf33c8bd13f7835a5ae235cd2c8a4. Previous review-fixup work was squashed onto that branch.

Do not target main — target pants-reqs.

Problem

py/pants-toolshed/toolshed_publish_reqs/test_rules.py currently has serious code smells:

1. Lint failure: F811 redefinition of unused 'pants' from line 56 — the local pants = types.ModuleType("pants") inside _install_pants_stubs (line 8) collides with the module-level import pants (line 56) that gates the stub installation.
2. Imports of _publish_req_target_name and _adaptors_for_install_requires happen halfway through the file (after the try/except ModuleNotFoundError block), each requiring # noqa: E402.
3. The whole try: import pants / except ModuleNotFoundError: _install_pants_stubs() block is at module import time in the test file itself.

This is fragile and ugly.

Goal

Move the pants.* stub installation out of test_rules.py into a sibling conftest.py. Pytest auto-imports conftest.py files before any test module in the same directory is collected, so by the time test_rules.py is imported, pants.* modules are already available in sys.modules. This means test_rules.py can have clean top-of-file imports with zero noqa, zero sys.modules manipulation, and zero F811.

Required changes

1. Create py/pants-toolshed/toolshed_publish_reqs/conftest.py

Move the _install_pants_stubs function and its try/except invocation here verbatim from test_rules.py. Module-level docstring should explain why this exists. Example shape:

"""Pytest conftest: provide stub ``pants.*`` modules when tests run outside
a real Pants sandbox.

The synthetic-targets plugin under test imports from ``pants.engine.*`` at
module load time. To allow these tests to run with plain pytest (no Pants
RuleRunner), we install minimal stub modules into ``sys.modules`` before
any test module is collected. If a real ``pants`` distribution is already
importable (e.g. when Pants itself runs the tests), we leave the real
modules in place and do nothing.
"""

import sys
import types


def _install_pants_stubs() -> None:
    # ... existing body, unchanged ...


try:
    import pants  # noqa: F401
except ModuleNotFoundError:
    _install_pants_stubs()

The # noqa: F401 on import pants is acceptable in conftest.py because the import is a pure existence-check (it has no other purpose). It must NOT trigger F811: keep the local variable name inside _install_pants_stubs as pants_module = types.ModuleType("pants") (renaming the local to avoid shadowing the imported name) — apply the same rename inside sys.modules["pants"] = pants_module. The other locals (engine, fs, internals, synthetic_targets, target_adaptor, intrinsics, rules, unions) do not collide with anything imported at module top-level and can stay as-is.

The _FakeTargetAdaptor recording class must remain inside _install_pants_stubs exactly as today — it is the same class the new rule-helper tests assert against.

2. Rewrite py/pants-toolshed/toolshed_publish_reqs/test_rules.py

After the conftest move, this file should look like a normal, boring pytest module:

import pytest

from toolshed_publish_reqs.names import _publish_req_target_name
from toolshed_publish_reqs.rules import _adaptors_for_install_requires


@pytest.mark.parametrize(
    ("req_str", "expected"),
    (
        ("aiohttp>=3.8.1", "_publish__aiohttp"),
        ("PyYAML", "_publish__pyyaml"),
        ("pytest-asyncio", "_publish__pytest_asyncio"),
        ("Foo.Bar_Baz", "_publish__foo_bar_baz"),
        ("package[extra]>=1.0", "_publish__package"),
        ("package; python_version >= '3.10'", "_publish__package"),
        ("foo----bar", "_publish__foo_bar"),
        ("  aiohttp>=3.8.1  ", "_publish__aiohttp"),
    ),
)
def test_publish_req_target_name(req_str: str, expected: str):
    assert _publish_req_target_name(req_str) == expected


def test_adaptors_for_install_requires_basic():
    result = _adaptors_for_install_requires(
        "py/foo",
        ["aiohttp>=3.8.1", "PyYAML"])
    assert len(result) == 2
    assert result[0].type_alias == "python_requirement"
    assert result[0].kwargs["name"] == "_publish__aiohttp"
    assert result[0].kwargs["requirements"] == ["aiohttp>=3.8.1"]
    assert result[0].kwargs["resolve"] == "publish"
    assert result[0].kwargs["__description_of_origin__"] == "py/foo/setup.cfg"
    assert result[1].kwargs["name"] == "_publish__pyyaml"


def test_adaptors_for_install_requires_empty():
    result = _adaptors_for_install_requires("py/foo", [])
    assert result == ()


def test_adaptors_for_install_requires_invalid():
    with pytest.raises(ValueError) as exc_info:
        _adaptors_for_install_requires(
            "py/foo",
            ["@@not-a-vali...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>