tests/integration/test_rpm.py (1)

71-80: Add a negative-case integration for an unsatisfiable arch filter

Consider a companion case with binary={"arch": "s390x"} (or any absent arch) expecting a non‑zero exit and a clear error message. This guards the user‑error path end‑to‑end.

tests/unit/package_managers/rpm/test_rpm.py (1)

596-618: Fix unused mock and strengthen the assertion

Use the patched asyncio.run to avoid Ruff ARG001 and assert it’s called exactly once (only the filtered arch is processed).

Apply this diff:

 @mock.patch("hermeto.core.package_managers.rpm.main.asyncio.run")
 def test_download_filters_architectures(
-    mock_asyncio: mock.Mock, rooted_tmp_path: RootedPath
+    mock_asyncio: mock.Mock, rooted_tmp_path: RootedPath
 ) -> None:
@@
     assert all("x86_64" in p for p in paths)
     assert not any("aarch64" in p for p in paths)
 
+    # Ensure exactly one download loop ran (only the filtered arch).
+    mock_asyncio.assert_called_once()

hermeto/core/package_managers/rpm/main.py (1)

319-325: Document the new binary_filter parameter in _download

Add a brief note so future readers don’t have to trace call sites.

Apply this diff:

 def _download(
     lockfile: RedhatRpmsLock,
     output_dir: Path,
     ssl_options: Optional[SSLOptions] = None,
     binary_filter: Optional[RpmBinaryFilters] = None,
 ) -> dict[Path, Any]:
     """
     Download packages and module metadata mentioned in the lockfile.
 
     Go through the parsed lockfile structure and find all RPM, SRPM and module metadata files.
     Create a metadata structure indexed by destination path used
     for later verification (size, checksum) after download.
     Prepare a list of files to be downloaded, and then download files.
+    If binary_filter is provided, only architectures matching the filter are processed.
     """

hermeto/core/package_managers/rpm/binary_filters.py (3)

50-53: Tidy exception construction and handle empty-available case

Move long f-strings to variables (appeases Ruff TRY003) and provide a clearer message when no arches are available.

Apply this diff:

-                raise UnsatisfiableArchitectureFilter(
-                    f"Specified RPM architecture(s) not found in lockfile: {', '.join(sorted(unsatisfiable))}",
-                    solution=f"Use one of the available architectures: {', '.join(sorted(available))}",
-                )
+                reason = "Specified RPM architecture(s) not found in lockfile: " + ", ".join(sorted(unsatisfiable))
+                available_msg = ", ".join(sorted(available)) if available else "none available in lockfile"
+                solution = f"Use one of the available architectures: {available_msg}"
+                raise UnsatisfiableArchitectureFilter(reason, solution=solution)

---

3-3: Remove unused logger

logging import and log are unused.

Apply this diff:

-import logging
@@
-log = logging.getLogger(__name__)

Also applies to: 11-11

---

40-43: Signature polish: accept Sequence to be more general

If you want to accept tuples/lists equally, consider Sequence[LockfileArch] for the parameter type.

Apply this diff:

-from typing import Any, Optional
+from typing import Any, Optional, Sequence
@@
-    def filter(self, arches: list[LockfileArch]) -> list[LockfileArch]:
+    def filter(self, arches: Sequence[LockfileArch]) -> list[LockfileArch]:
         """Filter a list of architectures based on constraints."""
         return [arch for arch in arches if arch in self]

tests/unit/package_managers/rpm/test_binary_filters.py (3)

35-39: Prefer typing.cast over type: ignore

Avoid suppressing type checking; cast the mocked list to the expected type.

Apply this diff:

-from typing import Optional
+from typing import Optional, cast
@@
-    all_arches = [mock.Mock(spec=LockfileArch, arch=arch) for arch in lockfile_arches]
-
-    filtered = arch_filter.validate_and_filter(all_arches)  # type: ignore[arg-type]
+    all_arches = [mock.Mock(spec=LockfileArch, arch=arch) for arch in lockfile_arches]
+    all_arches_typed = cast(list[LockfileArch], all_arches)
+
+    filtered = arch_filter.validate_and_filter(all_arches_typed)
     filtered_arch_strings = [arch.arch for arch in filtered]

---

14-28: Add edge-case coverage: ':all:' combined and whitespace handling

Strengthen guarantees around parsing by covering mixed ':all:' specs and whitespace trimming.

Apply this diff (add two params):

         pytest.param(
             RpmBinaryFilters(arch=":all:"),
             ["x86_64", "aarch64", "s390x", "ppc64le"],
             id="all_keyword",
         ),
+        pytest.param(
+            RpmBinaryFilters(arch="x86_64,:all:"),
+            ["x86_64", "aarch64", "s390x", "ppc64le"],
+            id="all_mixed_with_other",
+        ),
+        pytest.param(
+            RpmBinaryFilters(arch="x86_64, aarch64"),
+            ["x86_64", "aarch64"],
+            id="whitespace_trim",
+        ),

---

58-59: Assert error message content for better contract checking

Lock in user‑facing text fragment to prevent regressions.

Apply this diff:

-    with pytest.raises(UnsatisfiableArchitectureFilter):
-        _ = arch_filter.validate_and_filter(all_arches)  # type: ignore[arg-type]
+    with pytest.raises(UnsatisfiableArchitectureFilter, match="not found in lockfile"):
+        arch_filter.validate_and_filter(all_arches)  # type: ignore[arg-type]

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

hermeto/core/package_managers/rpm/binary_filters.py (1)

26-39: contains is too strict; breaks duck-typed tests and mocks

Don't require isinstance(LockfileArch); accept any object with an arch attribute.

Apply this diff:

-from hermeto.core.package_managers.rpm.redhat import LockfileArch
@@
-        if isinstance(item, str):
-            arch = item
-        elif isinstance(item, LockfileArch):
-            arch = item.arch
-        else:
-            return False
+        if isinstance(item, str):
+            arch = item
+        elif hasattr(item, "arch"):
+            # Accept LockfileArch instances and test doubles/mocks with an `arch` attribute
+            arch = getattr(item, "arch")
+        else:
+            return False

And remove the now-unused import:

-from hermeto.core.package_managers.rpm.redhat import LockfileArch

tests/integration/test_rpm.py (1)

71-80: New multi-arch filtering scenario looks good

Covers the positive path for specifying a binary arch filter in integration. No issues spotted.

Consider adding a negative case (e.g., arch not present) to assert an UnsatisfiableArchitectureFilter is surfaced end-to-end.

tests/unit/package_managers/rpm/test_rpm.py (1)

596-618: Fix unused mocked argument; assert asyncio.run was invoked

Ruff flags mock_asyncio as unused (ARG001). Use it to assert the download path was exercised.

Apply this diff:

 @mock.patch("hermeto.core.package_managers.rpm.main.asyncio.run")
 def test_download_filters_architectures(
-    mock_asyncio: mock.Mock, rooted_tmp_path: RootedPath
+    mock_asyncio: mock.Mock, rooted_tmp_path: RootedPath
 ) -> None:
@@
     assert all("x86_64" in p for p in paths)
     assert not any("aarch64" in p for p in paths)
+
+    # Ensure the async download path was triggered
+    mock_asyncio.assert_called_once()

hermeto/core/package_managers/rpm/binary_filters.py (2)

40-43: Fast-path when no constraints

Return arches unmodified when accepting all to avoid needless membership checks.

Apply this diff:

 def filter(self, arches: list[LockfileArch]) -> list[LockfileArch]:
     """Filter a list of architectures based on constraints."""
-    return [arch for arch in arches if arch in self]
+    if self.arch_constraints is None:
+        return arches
+    return [arch for arch in arches if arch in self]

---

14-16: Tidy raise site per TRY003: move message formatting into the exception

Encapsulates long messages in the exception class; improves reuse and satisfies style checks.

Apply this diff:

 class UnsatisfiableArchitectureFilter(PackageRejected):
-    """RPM architecture filter constraints cannot be satisfied by lockfile architectures."""
+    """RPM architecture filter constraints cannot be satisfied by lockfile architectures."""
+
+    def __init__(self, unsatisfiable: set[str], available: set[str]) -> None:
+        reason = (
+            "Specified RPM architecture(s) not found in lockfile: "
+            f"{', '.join(sorted(unsatisfiable))}"
+        )
+        solution = f"Use one of the available architectures: {', '.join(sorted(available))}"
+        super().__init__(reason, solution=solution)
@@
-            if unsatisfiable:
-                raise UnsatisfiableArchitectureFilter(
-                    f"Specified RPM architecture(s) not found in lockfile: {', '.join(sorted(unsatisfiable))}",
-                    solution=f"Use one of the available architectures: {', '.join(sorted(available))}",
-                )
+            if unsatisfiable:
+                raise UnsatisfiableArchitectureFilter(unsatisfiable, available)

Also applies to: 50-53

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

hermeto/core/package_managers/rpm/binary_filters.py (1)

23-35: contains is too strict; duck‑type on arch to support mocks and test doubles.
Using isinstance(item, LockfileArch) breaks tests that pass mocks with an arch attribute.

Apply this diff:

-        if isinstance(item, str):
-            arch = item
-        elif isinstance(item, LockfileArch):
-            arch = item.arch
-        else:
-            return False
+        if isinstance(item, str):
+            arch = item
+        elif hasattr(item, "arch"):
+            # Accept LockfileArch instances and test doubles/mocks with an `arch` attribute
+            arch = getattr(item, "arch")
+        else:
+            return False

tests/unit/package_managers/rpm/test_main.py (1)

596-599: Silence unused patched argument to satisfy Ruff ARG001.
Rename the unused mock_asyncio parameter to _mock_asyncio (or use it) to avoid ARG001.

Apply this diff:

 @mock.patch("hermeto.core.package_managers.rpm.main.asyncio.run")
 def test_download_filters_architectures(
-    mock_asyncio: mock.Mock, rooted_tmp_path: RootedPath
+    _mock_asyncio: mock.Mock, rooted_tmp_path: RootedPath
 ) -> None:

hermeto/core/package_managers/rpm/main.py (1)

333-335: Architecture filtering applied before download.
Consider adding a debug log to trace which arches were selected.

Apply this diff:

 arch_filter = RPMArchitectureFilter(binary_filter)
 arches_to_process = arch_filter.validate_and_filter(lockfile.arches)
+log.debug("Filtered architectures to process: %s", [a.arch for a in arches_to_process])

hermeto/core/package_managers/rpm/binary_filters.py (1)

47-50: Minor: long error message assembly inside raise.
Style-only: consider moving formatting into the exception class or a helper to satisfy TRY003.

Example:

-class UnsatisfiableArchitectureFilter(PackageRejected):
-    """RPM architecture filter constraints cannot be satisfied by lockfile architectures."""
+class UnsatisfiableArchitectureFilter(PackageRejected):
+    """RPM architecture filter constraints cannot be satisfied by lockfile architectures."""
+    def __init__(self, missing: set[str], available: set[str]) -> None:
+        super().__init__(
+            f"Specified RPM architecture(s) not found in lockfile: {', '.join(sorted(missing))}",
+            solution=f"Use one of the available architectures: {', '.join(sorted(available))}",
+        )

and then:

-                raise UnsatisfiableArchitectureFilter(
-                    f"Specified RPM architecture(s) not found in lockfile: {', '.join(sorted(unsatisfiable))}",
-                    solution=f"Use one of the available architectures: {', '.join(sorted(available))}",
-                )
+                raise UnsatisfiableArchitectureFilter(unsatisfiable, available)

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro