From 100a18a3229c3a790eba45eeb9a618edd2daede9 Mon Sep 17 00:00:00 2001 From: Alexandre Begnoche Date: Fri, 26 Jun 2026 12:53:16 -0400 Subject: [PATCH 01/10] feat: add provider label discovery Signed-off-by: Alexandre Begnoche --- isvctl/src/isvctl/cli/test.py | 86 ++++++++++++- isvctl/src/isvctl/config/label_discovery.py | 54 ++++++++ isvctl/tests/test_provider_label_discovery.py | 80 ++++++++++++ isvctl/tests/test_test_cli_labels.py | 115 +++++++++++++++++- 4 files changed, 330 insertions(+), 5 deletions(-) create mode 100644 isvctl/src/isvctl/config/label_discovery.py create mode 100644 isvctl/tests/test_provider_label_discovery.py diff --git a/isvctl/src/isvctl/cli/test.py b/isvctl/src/isvctl/cli/test.py index 9fac09a9..4904c7a9 100644 --- a/isvctl/src/isvctl/cli/test.py +++ b/isvctl/src/isvctl/cli/test.py @@ -23,7 +23,7 @@ import sys from datetime import UTC, datetime from pathlib import Path -from typing import Annotated, TextIO +from typing import Annotated, Any, TextIO import typer import yaml @@ -38,6 +38,7 @@ print_progress, print_warning, ) +from isvctl.config.label_discovery import ProviderConfigMatch, discover_provider_label_configs from isvctl.config.merger import merge_yaml_files from isvctl.config.schema import RunConfig from isvctl.orchestrator.loop import Orchestrator, Phase @@ -45,6 +46,7 @@ from isvctl.reporting import check_upload_credentials, create_test_run, get_environment_config, update_test_run logger = logging.getLogger(__name__) +CONFIGS_ROOT = Path(__file__).resolve().parents[3] / "configs" class TeeWriter: @@ -78,11 +80,40 @@ def isatty(self) -> bool: ) +def _provider_discovery_plan(provider: str, labels: list[str], matches: list[ProviderConfigMatch]) -> dict[str, Any]: + """Return a JSON-serializable provider label discovery plan.""" + return { + "provider": provider, + "labels": labels, + "configs": [ + { + "config": str(match.config_path), + "matched_checks": [ + { + "category": check.category, + "name": check.name, + "labels": list(check.labels), + } + for check in match.matched_checks + ], + } + for match in matches + ], + } + + +def _junitxml_for_discovered_config(junitxml: Path, match: ProviderConfigMatch, total: int) -> Path: + """Return a non-overlapping JUnit path for a discovered config run.""" + if total <= 1: + return junitxml + return junitxml.with_name(f"{junitxml.stem}-{match.config_path.stem}{junitxml.suffix}") + + @app.command("run", context_settings={"allow_extra_args": True, "ignore_unknown_options": True}) def run( ctx: typer.Context, config_files: Annotated[ - list[Path], + list[Path] | None, typer.Option( "--config", "-f", @@ -92,7 +123,14 @@ def run( dir_okay=False, readable=True, ), - ], + ] = None, + provider: Annotated[ + str | None, + typer.Option( + "--provider", + help="Provider name for label discovery when no --config/-f files are supplied.", + ), + ] = None, set_values: Annotated[ list[str] | None, typer.Option( @@ -208,6 +246,48 @@ def run( setup_logging(verbose) apply_user_config(no_user_config) + if provider: + if config_files: + print_error("--provider discovery cannot be combined with --config/-f.") + raise typer.Exit(code=1) + if not labels: + print_error("--provider requires at least one --label/-l for discovery.") + raise typer.Exit(code=1) + + matches = discover_provider_label_configs(provider, labels, configs_root=CONFIGS_ROOT) + if not matches: + print_error(f"No {provider!r} provider configs match labels: {', '.join(labels)}") + raise typer.Exit(code=1) + + if dry_run: + typer.echo(json.dumps(_provider_discovery_plan(provider, labels, matches), indent=2)) + return + + print_progress( + f"Discovered {len(matches)} {provider!r} provider config(s) matching labels: {', '.join(labels)}" + ) + for match in matches: + print_progress(f"\n--- Running {match.config_path} ---") + run( + ctx, + config_files=[match.config_path], + provider=None, + set_values=set_values, + phase=phase, + labels=labels, + dry_run=False, + working_dir=working_dir, + verbose=verbose, + no_user_config=no_user_config, + junitxml=_junitxml_for_discovered_config(junitxml, match, len(matches)), + color=color, + no_upload=no_upload, + lab_id=lab_id, + tags=tags, + isv_software_version=isv_software_version, + ) + return + # Validate at least one config file is provided if not config_files: print_error("At least one --config/-f config file is required.") diff --git a/isvctl/src/isvctl/config/label_discovery.py b/isvctl/src/isvctl/config/label_discovery.py new file mode 100644 index 00000000..ce10e3c3 --- /dev/null +++ b/isvctl/src/isvctl/config/label_discovery.py @@ -0,0 +1,54 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Provider-scoped label discovery helpers.""" + +from __future__ import annotations + +from dataclasses import dataclass +from pathlib import Path + +from isvtest.core.resolution import parse_validations + +from isvctl.config.merger import merge_yaml_files + + +@dataclass(frozen=True) +class MatchedCheck: + """A validation check that matched requested labels.""" + + category: str + name: str + labels: tuple[str, ...] + + +@dataclass(frozen=True) +class ProviderConfigMatch: + """A provider config selected by label discovery.""" + + config_path: Path + matched_checks: tuple[MatchedCheck, ...] + + +def discover_provider_label_configs( + provider: str, + labels: list[str], + *, + configs_root: Path, +) -> list[ProviderConfigMatch]: + """Return provider configs whose resolved validation wiring matches all labels.""" + requested = {label for label in labels if label} + provider_config_dir = configs_root / "providers" / provider / "config" + matches: list[ProviderConfigMatch] = [] + + for config_path in sorted(provider_config_dir.glob("*.yaml")): + merged = merge_yaml_files([config_path]) + raw_validations = (merged.get("tests") or {}).get("validations") or {} + matched_checks = tuple( + MatchedCheck(category=entry.category, name=entry.name, labels=entry.labels) + for entry in parse_validations(raw_validations) + if requested.issubset(entry.labels) + ) + if matched_checks: + matches.append(ProviderConfigMatch(config_path=config_path, matched_checks=matched_checks)) + return matches diff --git a/isvctl/tests/test_provider_label_discovery.py b/isvctl/tests/test_provider_label_discovery.py new file mode 100644 index 00000000..29e7d7a9 --- /dev/null +++ b/isvctl/tests/test_provider_label_discovery.py @@ -0,0 +1,80 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Tests for provider-scoped label discovery.""" + +from __future__ import annotations + +from pathlib import Path + +from isvctl.config.label_discovery import discover_provider_label_configs + + +def _write_provider_config(root: Path, provider: str, name: str, suite: str) -> Path: + """Write a provider config importing one provider-neutral suite.""" + config_path = root / "providers" / provider / "config" / name + config_path.parent.mkdir(parents=True, exist_ok=True) + config_path.write_text( + f"""\ +import: + - ../../../suites/{suite} +commands: + demo: + phases: [test] + steps: [] +tests: + platform: demo +""", + encoding="utf-8", + ) + return config_path + + +def _write_suite(root: Path, name: str, labels: list[str], check_name: str) -> None: + """Write a suite with one labelled validation check.""" + labels_yaml = ", ".join(f'"{label}"' for label in labels) + suite_path = root / "suites" / name + suite_path.parent.mkdir(parents=True, exist_ok=True) + suite_path.write_text( + f"""\ +tests: + validations: + sample: + checks: + {check_name}: + test_id: "N/A" + labels: [{labels_yaml}] +""", + encoding="utf-8", + ) + + +def test_discovers_provider_configs_matching_label_through_imports(tmp_path: Path) -> None: + """Provider label discovery matches every provider config whose resolved imports contain the label.""" + configs_root = tmp_path / "configs" + _write_suite(configs_root, "network.yaml", ["network"], "NetworkCheck") + _write_suite(configs_root, "observability.yaml", ["network", "observability"], "VpcFlowLogsCheck") + _write_suite(configs_root, "iam.yaml", ["iam"], "IamCheck") + _write_provider_config(configs_root, "aws", "network.yaml", "network.yaml") + _write_provider_config(configs_root, "aws", "observability.yaml", "observability.yaml") + _write_provider_config(configs_root, "aws", "iam.yaml", "iam.yaml") + + matches = discover_provider_label_configs("aws", ["network"], configs_root=configs_root) + + assert [match.config_path.name for match in matches] == ["network.yaml", "observability.yaml"] + assert [check.name for check in matches[0].matched_checks] == ["NetworkCheck"] + assert [check.name for check in matches[1].matched_checks] == ["VpcFlowLogsCheck"] + + +def test_discovery_requires_all_requested_labels(tmp_path: Path) -> None: + """Repeated labels use AND semantics, matching the existing runtime label filter.""" + configs_root = tmp_path / "configs" + _write_suite(configs_root, "network.yaml", ["network"], "NetworkCheck") + _write_suite(configs_root, "observability.yaml", ["network", "observability"], "VpcFlowLogsCheck") + _write_provider_config(configs_root, "aws", "network.yaml", "network.yaml") + _write_provider_config(configs_root, "aws", "observability.yaml", "observability.yaml") + + matches = discover_provider_label_configs("aws", ["network", "observability"], configs_root=configs_root) + + assert [match.config_path.name for match in matches] == ["observability.yaml"] + assert [check.name for check in matches[0].matched_checks] == ["VpcFlowLogsCheck"] diff --git a/isvctl/tests/test_test_cli_labels.py b/isvctl/tests/test_test_cli_labels.py index 6b6ba6d8..6609f51d 100644 --- a/isvctl/tests/test_test_cli_labels.py +++ b/isvctl/tests/test_test_cli_labels.py @@ -15,6 +15,7 @@ """Tests for isvctl test CLI label filtering.""" +import json from pathlib import Path from typing import Any, ClassVar @@ -49,16 +50,64 @@ def _write_config(tmp_path: Path) -> Path: return config +def _write_provider_config(root: Path, provider: str, name: str, suite: str, platform: str) -> Path: + """Write a minimal provider config importing one suite.""" + config_path = root / "providers" / provider / "config" / name + config_path.parent.mkdir(parents=True, exist_ok=True) + config_path.write_text( + f"""\ +import: + - ../../../suites/{suite} +commands: + {platform}: + phases: [test] + steps: [] +tests: + platform: {platform} +""", + encoding="utf-8", + ) + return config_path + + +def _write_suite(root: Path, name: str, labels: list[str], check_name: str) -> None: + """Write a provider-neutral suite with one check.""" + labels_yaml = ", ".join(f'"{label}"' for label in labels) + suite_path = root / "suites" / name + suite_path.parent.mkdir(parents=True, exist_ok=True) + suite_path.write_text( + f"""\ +tests: + validations: + sample: + checks: + {check_name}: + test_id: "N/A" + labels: [{labels_yaml}] +""", + encoding="utf-8", + ) + + class _FakeOrchestrator: """Capture orchestrator options passed by the CLI for assertion in tests.""" captured: ClassVar[dict[str, Any]] = {} + calls: ClassVar[list[dict[str, Any]]] = [] - def __init__(self, *_args: Any, **_kwargs: Any) -> None: - pass + def __init__(self, config: Any, **kwargs: Any) -> None: + self.config = config + self.kwargs = kwargs def run(self, **kwargs: Any) -> OrchestratorResult: type(self).captured.update(kwargs) + type(self).calls.append( + { + "platform": self.config.tests.platform, + "working_dir": self.kwargs.get("working_dir"), + "run_kwargs": kwargs, + } + ) return OrchestratorResult( success=True, phases=[PhaseResult(phase=Phase.TEST, success=True, message="ok")], @@ -69,6 +118,7 @@ def test_test_run_forwards_label_filters(monkeypatch: pytest.MonkeyPatch, tmp_pa """`isvctl test run -l/--label` passes requested labels to the orchestrator.""" config = _write_config(tmp_path) _FakeOrchestrator.captured = {} + _FakeOrchestrator.calls = [] monkeypatch.setattr(test_cli, "Orchestrator", _FakeOrchestrator) result = runner.invoke(test_cli.app, ["run", "-f", str(config), "--no-upload", "-l", "gpu", "--label", "slow"]) @@ -88,9 +138,70 @@ def test_short_l_flag_binds_to_label_not_lab_id(monkeypatch: pytest.MonkeyPatch, """ config = _write_config(tmp_path) _FakeOrchestrator.captured = {} + _FakeOrchestrator.calls = [] monkeypatch.setattr(test_cli, "Orchestrator", _FakeOrchestrator) result = runner.invoke(test_cli.app, ["run", "-f", str(config), "--no-upload", "-l", "12345"]) assert result.exit_code == 0, result.output assert _FakeOrchestrator.captured["include_labels"] == ["12345"] + + +def test_provider_label_discovery_dispatches_each_matching_config( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """`--provider --label` runs each matching provider config as its own lifecycle.""" + configs_root = tmp_path / "configs" + _write_suite(configs_root, "network.yaml", ["network"], "NetworkCheck") + _write_suite(configs_root, "observability.yaml", ["network", "observability"], "VpcFlowLogsCheck") + _write_suite(configs_root, "iam.yaml", ["iam"], "IamCheck") + network_config = _write_provider_config(configs_root, "aws", "network.yaml", "network.yaml", "network") + observability_config = _write_provider_config( + configs_root, + "aws", + "observability.yaml", + "observability.yaml", + "observability", + ) + _write_provider_config(configs_root, "aws", "iam.yaml", "iam.yaml", "iam") + _FakeOrchestrator.captured = {} + _FakeOrchestrator.calls = [] + monkeypatch.setattr(test_cli, "CONFIGS_ROOT", configs_root) + monkeypatch.setattr(test_cli, "Orchestrator", _FakeOrchestrator) + + result = runner.invoke(test_cli.app, ["run", "--provider", "aws", "--label", "network", "--no-upload"]) + + assert result.exit_code == 0, result.output + assert [call["platform"] for call in _FakeOrchestrator.calls] == ["network", "observability"] + assert [call["working_dir"] for call in _FakeOrchestrator.calls] == [ + network_config.parent, + observability_config.parent, + ] + assert [call["run_kwargs"]["include_labels"] for call in _FakeOrchestrator.calls] == [["network"], ["network"]] + + +def test_provider_label_discovery_dry_run_prints_plan_without_running( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """Discovery dry-run prints selected configs and checks without invoking the orchestrator.""" + configs_root = tmp_path / "configs" + _write_suite(configs_root, "network.yaml", ["network"], "NetworkCheck") + _write_suite(configs_root, "observability.yaml", ["network", "observability"], "VpcFlowLogsCheck") + _write_provider_config(configs_root, "aws", "network.yaml", "network.yaml", "network") + _write_provider_config(configs_root, "aws", "observability.yaml", "observability.yaml", "observability") + _FakeOrchestrator.calls = [] + monkeypatch.setattr(test_cli, "CONFIGS_ROOT", configs_root) + monkeypatch.setattr(test_cli, "Orchestrator", _FakeOrchestrator) + + result = runner.invoke( + test_cli.app, + ["run", "--provider", "aws", "--label", "network", "--dry-run", "--no-upload"], + ) + + assert result.exit_code == 0, result.output + assert _FakeOrchestrator.calls == [] + plan = json.loads(result.output) + assert plan["provider"] == "aws" + assert plan["labels"] == ["network"] + assert [Path(item["config"]).name for item in plan["configs"]] == ["network.yaml", "observability.yaml"] + assert [item["matched_checks"][0]["name"] for item in plan["configs"]] == ["NetworkCheck", "VpcFlowLogsCheck"] From d529e43b1dd024f3761f04c3582ca1cb1db8268c Mon Sep 17 00:00:00 2001 From: Alexandre Begnoche Date: Fri, 26 Jun 2026 13:43:50 -0400 Subject: [PATCH 02/10] fix: clarify error when --label has no --provider or --config `isvctl test run --label ` with neither --provider nor -f previously emitted "At least one --config/-f config file is required", omitting the --provider discovery path. Name both options so the label-discovery entry point is discoverable from the error. Signed-off-by: Alexandre Begnoche --- isvctl/src/isvctl/cli/test.py | 5 ++++- isvctl/tests/test_test_cli_labels.py | 13 +++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/isvctl/src/isvctl/cli/test.py b/isvctl/src/isvctl/cli/test.py index 4904c7a9..c8a36cc5 100644 --- a/isvctl/src/isvctl/cli/test.py +++ b/isvctl/src/isvctl/cli/test.py @@ -290,7 +290,10 @@ def run( # Validate at least one config file is provided if not config_files: - print_error("At least one --config/-f config file is required.") + if labels: + print_error("--label requires either --provider (for label discovery) or --config/-f.") + else: + print_error("At least one --config/-f config file is required.") raise typer.Exit(code=1) # Collect extra pytest args from context (after --) diff --git a/isvctl/tests/test_test_cli_labels.py b/isvctl/tests/test_test_cli_labels.py index 6609f51d..348824f7 100644 --- a/isvctl/tests/test_test_cli_labels.py +++ b/isvctl/tests/test_test_cli_labels.py @@ -147,6 +147,19 @@ def test_short_l_flag_binds_to_label_not_lab_id(monkeypatch: pytest.MonkeyPatch, assert _FakeOrchestrator.captured["include_labels"] == ["12345"] +def test_label_without_provider_or_config_reports_both_options(monkeypatch: pytest.MonkeyPatch) -> None: + """`--label` with neither `--provider` nor `-f` names both ways to supply checks.""" + _FakeOrchestrator.calls = [] + monkeypatch.setattr(test_cli, "Orchestrator", _FakeOrchestrator) + + result = runner.invoke(test_cli.app, ["run", "--label", "iam", "--no-upload"]) + + assert result.exit_code == 1, result.output + assert "--provider" in result.output + assert "--config/-f" in result.output + assert _FakeOrchestrator.calls == [] + + def test_provider_label_discovery_dispatches_each_matching_config( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: From 198154f7e603d9f96750387fcd8ed28d65fc7518 Mon Sep 17 00:00:00 2001 From: Alexandre Begnoche Date: Fri, 26 Jun 2026 13:59:01 -0400 Subject: [PATCH 03/10] fix: gate label discovery by release filter and clarify failures Provider label discovery now applies the same released_tests.json filter as execution, so a config is not selected solely on an unreleased check that would be skipped at runtime (honoring ISVTEST_INCLUDE_UNRELEASED). Discovery failures are now distinguishable: an unknown --provider lists the available providers, and a valid provider with no label match lists the labels that provider exposes (surfacing typos like control-plane vs control_plane). Signed-off-by: Alexandre Begnoche --- isvctl/src/isvctl/cli/test.py | 23 +++++++++-- isvctl/src/isvctl/config/label_discovery.py | 37 +++++++++++++++++- isvctl/tests/test_provider_label_discovery.py | 16 ++++++++ isvctl/tests/test_test_cli_labels.py | 38 +++++++++++++++++++ 4 files changed, 109 insertions(+), 5 deletions(-) diff --git a/isvctl/src/isvctl/cli/test.py b/isvctl/src/isvctl/cli/test.py index c8a36cc5..c6b7cca6 100644 --- a/isvctl/src/isvctl/cli/test.py +++ b/isvctl/src/isvctl/cli/test.py @@ -28,6 +28,7 @@ import typer import yaml from isvtest.catalog import build_catalog, get_catalog_version +from isvtest.release_manifest import load_released_test_filter from isvctl.cli import setup_logging from isvctl.cli.common import ( @@ -38,7 +39,12 @@ print_progress, print_warning, ) -from isvctl.config.label_discovery import ProviderConfigMatch, discover_provider_label_configs +from isvctl.config.label_discovery import ( + ProviderConfigMatch, + available_labels, + discover_provider_label_configs, + list_providers, +) from isvctl.config.merger import merge_yaml_files from isvctl.config.schema import RunConfig from isvctl.orchestrator.loop import Orchestrator, Phase @@ -254,9 +260,20 @@ def run( print_error("--provider requires at least one --label/-l for discovery.") raise typer.Exit(code=1) - matches = discover_provider_label_configs(provider, labels, configs_root=CONFIGS_ROOT) + known_providers = list_providers(CONFIGS_ROOT) + if provider not in known_providers: + print_error(f"Unknown provider {provider!r}. Available providers: {', '.join(known_providers)}") + raise typer.Exit(code=1) + + matches = discover_provider_label_configs( + provider, labels, configs_root=CONFIGS_ROOT, released_tests=load_released_test_filter() + ) if not matches: - print_error(f"No {provider!r} provider configs match labels: {', '.join(labels)}") + known_labels = available_labels(provider, configs_root=CONFIGS_ROOT) + print_error( + f"No {provider!r} provider configs match labels: {', '.join(labels)}. " + f"Available labels for {provider!r}: {', '.join(sorted(known_labels))}" + ) raise typer.Exit(code=1) if dry_run: diff --git a/isvctl/src/isvctl/config/label_discovery.py b/isvctl/src/isvctl/config/label_discovery.py index ce10e3c3..cfcbe37b 100644 --- a/isvctl/src/isvctl/config/label_discovery.py +++ b/isvctl/src/isvctl/config/label_discovery.py @@ -8,7 +8,7 @@ from dataclasses import dataclass from pathlib import Path -from isvtest.core.resolution import parse_validations +from isvtest.core.resolution import parse_validations, resolve_class_key from isvctl.config.merger import merge_yaml_files @@ -30,13 +30,45 @@ class ProviderConfigMatch: matched_checks: tuple[MatchedCheck, ...] +def list_providers(configs_root: Path) -> list[str]: + """Return provider names that expose a discoverable ``config/*.yaml`` directory.""" + providers_dir = configs_root / "providers" + if not providers_dir.is_dir(): + return [] + return sorted( + provider_dir.name + for provider_dir in providers_dir.iterdir() + if provider_dir.is_dir() and any((provider_dir / "config").glob("*.yaml")) + ) + + +def available_labels(provider: str, *, configs_root: Path) -> set[str]: + """Return every label declared across a provider's resolved config wiring.""" + provider_config_dir = configs_root / "providers" / provider / "config" + labels: set[str] = set() + for config_path in provider_config_dir.glob("*.yaml"): + merged = merge_yaml_files([config_path]) + raw_validations = (merged.get("tests") or {}).get("validations") or {} + for entry in parse_validations(raw_validations): + labels.update(entry.labels) + return labels + + def discover_provider_label_configs( provider: str, labels: list[str], *, configs_root: Path, + released_tests: set[str] | None = None, ) -> list[ProviderConfigMatch]: - """Return provider configs whose resolved validation wiring matches all labels.""" + """Return provider configs whose resolved validation wiring matches all labels. + + A check counts toward a match only if it is also runnable under the release + filter, mirroring orchestrator execution: when ``released_tests`` is a set, + unreleased checks are ignored so a config is not selected solely on a check + that would be skipped at runtime. ``None`` disables the filter (include all), + matching ``ISVTEST_INCLUDE_UNRELEASED``. + """ requested = {label for label in labels if label} provider_config_dir = configs_root / "providers" / provider / "config" matches: list[ProviderConfigMatch] = [] @@ -48,6 +80,7 @@ def discover_provider_label_configs( MatchedCheck(category=entry.category, name=entry.name, labels=entry.labels) for entry in parse_validations(raw_validations) if requested.issubset(entry.labels) + and (released_tests is None or resolve_class_key(entry.name, released_tests) is not None) ) if matched_checks: matches.append(ProviderConfigMatch(config_path=config_path, matched_checks=matched_checks)) diff --git a/isvctl/tests/test_provider_label_discovery.py b/isvctl/tests/test_provider_label_discovery.py index 29e7d7a9..22314a3b 100644 --- a/isvctl/tests/test_provider_label_discovery.py +++ b/isvctl/tests/test_provider_label_discovery.py @@ -66,6 +66,22 @@ def test_discovers_provider_configs_matching_label_through_imports(tmp_path: Pat assert [check.name for check in matches[1].matched_checks] == ["VpcFlowLogsCheck"] +def test_discovery_excludes_configs_matched_only_by_unreleased_checks(tmp_path: Path) -> None: + """A config selected solely on an unreleased check is dropped when the release filter is active.""" + configs_root = tmp_path / "configs" + _write_suite(configs_root, "network.yaml", ["network"], "NetworkCheck") + _write_suite(configs_root, "observability.yaml", ["network"], "UnreleasedCheck") + _write_provider_config(configs_root, "aws", "network.yaml", "network.yaml") + _write_provider_config(configs_root, "aws", "observability.yaml", "observability.yaml") + + matches = discover_provider_label_configs( + "aws", ["network"], configs_root=configs_root, released_tests={"NetworkCheck"} + ) + + assert [match.config_path.name for match in matches] == ["network.yaml"] + assert [check.name for check in matches[0].matched_checks] == ["NetworkCheck"] + + def test_discovery_requires_all_requested_labels(tmp_path: Path) -> None: """Repeated labels use AND semantics, matching the existing runtime label filter.""" configs_root = tmp_path / "configs" diff --git a/isvctl/tests/test_test_cli_labels.py b/isvctl/tests/test_test_cli_labels.py index 348824f7..3bfdce85 100644 --- a/isvctl/tests/test_test_cli_labels.py +++ b/isvctl/tests/test_test_cli_labels.py @@ -160,6 +160,42 @@ def test_label_without_provider_or_config_reports_both_options(monkeypatch: pyte assert _FakeOrchestrator.calls == [] +def test_provider_discovery_unknown_provider_lists_available(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + """An unknown --provider reports it as unknown and lists the discoverable providers.""" + configs_root = tmp_path / "configs" + _write_suite(configs_root, "network.yaml", ["network"], "NetworkCheck") + _write_provider_config(configs_root, "aws", "network.yaml", "network.yaml", "network") + _FakeOrchestrator.calls = [] + monkeypatch.setattr(test_cli, "CONFIGS_ROOT", configs_root) + monkeypatch.setattr(test_cli, "Orchestrator", _FakeOrchestrator) + + result = runner.invoke(test_cli.app, ["run", "--provider", "gcp", "--label", "network", "--no-upload"]) + + assert result.exit_code == 1, result.output + assert "Unknown provider 'gcp'" in result.output + assert "aws" in result.output + assert _FakeOrchestrator.calls == [] + + +def test_provider_discovery_no_label_match_lists_available_labels( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """A valid provider with no label match reports the labels that provider does expose.""" + configs_root = tmp_path / "configs" + _write_suite(configs_root, "network.yaml", ["network"], "NetworkCheck") + _write_provider_config(configs_root, "aws", "network.yaml", "network.yaml", "network") + _FakeOrchestrator.calls = [] + monkeypatch.setattr(test_cli, "CONFIGS_ROOT", configs_root) + monkeypatch.setattr(test_cli, "Orchestrator", _FakeOrchestrator) + + result = runner.invoke(test_cli.app, ["run", "--provider", "aws", "--label", "nope", "--no-upload"]) + + assert result.exit_code == 1, result.output + assert "Available labels for 'aws'" in result.output + assert "network" in result.output + assert _FakeOrchestrator.calls == [] + + def test_provider_label_discovery_dispatches_each_matching_config( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: @@ -179,6 +215,7 @@ def test_provider_label_discovery_dispatches_each_matching_config( _write_provider_config(configs_root, "aws", "iam.yaml", "iam.yaml", "iam") _FakeOrchestrator.captured = {} _FakeOrchestrator.calls = [] + monkeypatch.setenv("ISVTEST_INCLUDE_UNRELEASED", "1") monkeypatch.setattr(test_cli, "CONFIGS_ROOT", configs_root) monkeypatch.setattr(test_cli, "Orchestrator", _FakeOrchestrator) @@ -203,6 +240,7 @@ def test_provider_label_discovery_dry_run_prints_plan_without_running( _write_provider_config(configs_root, "aws", "network.yaml", "network.yaml", "network") _write_provider_config(configs_root, "aws", "observability.yaml", "observability.yaml", "observability") _FakeOrchestrator.calls = [] + monkeypatch.setenv("ISVTEST_INCLUDE_UNRELEASED", "1") monkeypatch.setattr(test_cli, "CONFIGS_ROOT", configs_root) monkeypatch.setattr(test_cli, "Orchestrator", _FakeOrchestrator) From 66d59f4ec77fec2303e95514150cf36a39a745f5 Mon Sep 17 00:00:00 2001 From: Alexandre Begnoche Date: Fri, 26 Jun 2026 14:15:55 -0400 Subject: [PATCH 04/10] feat: add `isvctl catalog labels` to list all labels Aggregates the catalog into the set of labels with a per-label test count, giving a first-class way to discover valid label values for label filtering and provider discovery. Honors the same release gate as `catalog list`. Signed-off-by: Alexandre Begnoche --- isvctl/src/isvctl/cli/catalog.py | 41 ++++++++++++++++++++++++++++++++ isvctl/tests/test_catalog_cli.py | 33 +++++++++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/isvctl/src/isvctl/cli/catalog.py b/isvctl/src/isvctl/cli/catalog.py index f04bed7b..305ba3de 100644 --- a/isvctl/src/isvctl/cli/catalog.py +++ b/isvctl/src/isvctl/cli/catalog.py @@ -20,6 +20,7 @@ import json import logging +from collections import Counter from typing import Annotated import typer @@ -99,6 +100,46 @@ def list_cmd( console.print(table) +@app.command("labels") +def labels_cmd( + json_output: Annotated[ + bool, + typer.Option("--json", help="Emit the labels as JSON instead of a table"), + ] = False, +) -> None: + """List every label in the catalog with the number of tests carrying it. + + Released tests only by default. Set ``ISVTEST_INCLUDE_UNRELEASED=1`` to + include unreleased validations (matches the gate used at run time). + + Examples: + isvctl catalog labels + isvctl catalog labels --json + ISVTEST_INCLUDE_UNRELEASED=1 isvctl catalog labels + """ + counts = Counter(label for entry in build_catalog() for label in (entry.get("labels") or [])) + + if json_output: + labels = [{"label": label, "tests": count} for label, count in sorted(counts.items())] + typer.echo(json.dumps({"labels": labels}, indent=2)) + return + + table = Table( + title=f"Catalog Labels ({len(counts)} labels)", + title_justify="left", + show_header=True, + header_style="bold", + padding=(0, 1), + ) + table.add_column("Label", style="green", no_wrap=True) + table.add_column("Tests", style="cyan", justify="right") + + for label, count in sorted(counts.items()): + table.add_row(label, str(count)) + + console.print(table) + + @app.command("push") def push( verbose: Annotated[ diff --git a/isvctl/tests/test_catalog_cli.py b/isvctl/tests/test_catalog_cli.py index 4cfa0103..78bc3530 100644 --- a/isvctl/tests/test_catalog_cli.py +++ b/isvctl/tests/test_catalog_cli.py @@ -77,6 +77,39 @@ def test_catalog_list_json() -> None: assert payload["entries"] == _FAKE_ENTRIES +def test_catalog_labels_table() -> None: + """`catalog labels` renders each label and its test count.""" + entries = [ + {"name": "A", "labels": ["iam", "security"]}, + {"name": "B", "labels": ["iam"]}, + {"name": "C", "labels": []}, + ] + with patch("isvctl.cli.catalog.build_catalog", return_value=entries): + result = runner.invoke(app, ["labels"]) + + assert result.exit_code == 0, result.output + assert "iam" in result.output + assert "security" in result.output + + +def test_catalog_labels_json_counts_tests_per_label() -> None: + """`catalog labels --json` emits sorted labels with per-label test counts.""" + entries = [ + {"name": "A", "labels": ["iam", "security"]}, + {"name": "B", "labels": ["iam"]}, + {"name": "C", "labels": []}, + ] + with patch("isvctl.cli.catalog.build_catalog", return_value=entries): + result = runner.invoke(app, ["labels", "--json"]) + + assert result.exit_code == 0, result.output + payload = json.loads(result.output) + assert payload["labels"] == [ + {"label": "iam", "tests": 2}, + {"label": "security", "tests": 1}, + ] + + def test_catalog_list_unreleased_json() -> None: """`catalog list --unreleased` emits only entries missing from the release manifest.""" with ( From 1821039af401703019cf27e2045a5133a12864bb Mon Sep 17 00:00:00 2001 From: Alexandre Begnoche Date: Fri, 26 Jun 2026 15:31:53 -0400 Subject: [PATCH 05/10] feat(catalog): annotate catalog entries with test_ids Each catalog entry now carries the union of test-plan ids declared on its suite/provider YAML wiring ("N/A" excluded), with a variant's ids propagated to its base class. The entry identity stays the validation class name, so result correlation and coverage are unchanged; test_ids is purely additive metadata. isvreporter forwards the field on upload, and `isvctl catalog list` shows a Test IDs column alongside a merged "Labels (Platforms)" column. Signed-off-by: Alexandre Begnoche --- isvctl/src/isvctl/cli/catalog.py | 11 +++-- isvreporter/src/isvreporter/client.py | 3 +- isvreporter/tests/test_catalog_upload.py | 4 ++ isvtest/src/isvtest/catalog.py | 44 ++++++++++++++++++++ isvtest/tests/test_catalog.py | 52 ++++++++++++++++++++++++ 5 files changed, 109 insertions(+), 5 deletions(-) diff --git a/isvctl/src/isvctl/cli/catalog.py b/isvctl/src/isvctl/cli/catalog.py index 305ba3de..1e34c7de 100644 --- a/isvctl/src/isvctl/cli/catalog.py +++ b/isvctl/src/isvctl/cli/catalog.py @@ -85,15 +85,18 @@ def list_cmd( padding=(0, 1), ) table.add_column("Test", style="green", no_wrap=True) - table.add_column("Platforms", style="cyan") - table.add_column("Labels", style="dim") + table.add_column("Test IDs", style="magenta", max_width=32) + table.add_column("Labels (Platforms)", style="dim", max_width=40) table.add_column("Description") for entry in sorted(catalog_entries, key=lambda e: e["name"]): + labels = ", ".join(entry.get("labels") or []) + platforms = ", ".join(entry.get("platforms") or []) + labels_platforms = f"{labels} ({platforms})" if platforms else labels table.add_row( entry["name"], - ", ".join(entry.get("platforms") or []) or "-", - ", ".join(entry.get("labels") or []) or "-", + ", ".join(entry.get("test_ids") or []) or "-", + labels_platforms or "-", entry.get("description") or "-", ) diff --git a/isvreporter/src/isvreporter/client.py b/isvreporter/src/isvreporter/client.py index 2572a3c2..7189a0e5 100644 --- a/isvreporter/src/isvreporter/client.py +++ b/isvreporter/src/isvreporter/client.py @@ -293,7 +293,7 @@ def upload_test_catalog( jwt_token: JWT access token isv_test_version: Test suite version string (e.g. "1.2.3") entries: List of catalog entry dicts with keys: - name, description, labels, module + name, description, labels, module, platforms, test_ids Returns: True if catalog was uploaded or already exists, False on error @@ -321,6 +321,7 @@ def upload_test_catalog( "labels": e.get("labels", []), "module": e.get("module", ""), "platforms": e.get("platforms", []), + "test_ids": e.get("test_ids", []), } for e in entries ], diff --git a/isvreporter/tests/test_catalog_upload.py b/isvreporter/tests/test_catalog_upload.py index f5b6f9b8..fec2b8ee 100644 --- a/isvreporter/tests/test_catalog_upload.py +++ b/isvreporter/tests/test_catalog_upload.py @@ -48,6 +48,7 @@ def test_successful_upload(self, mock_urlopen: MagicMock) -> None: "description": "Test A", "labels": ["k8s"], "module": "mod.a", + "test_ids": ["K8S06-01"], }, {"name": "TestB", "description": "Test B", "labels": [], "module": "mod.b"}, ] @@ -72,8 +73,10 @@ def test_successful_upload(self, mock_urlopen: MagicMock) -> None: assert len(payload["entries"]) == 2 assert payload["entries"][0]["name"] == "TestA" assert payload["entries"][0]["labels"] == ["k8s"] + assert payload["entries"][0]["test_ids"] == ["K8S06-01"] assert "markers" not in payload["entries"][0] assert payload["entries"][1]["labels"] == [] + assert payload["entries"][1]["test_ids"] == [] @patch("isvreporter.client.urlopen") def test_skips_upload_when_version_exists(self, mock_urlopen: MagicMock) -> None: @@ -176,6 +179,7 @@ def test_empty_optional_fields_use_defaults(self, mock_urlopen: MagicMock) -> No assert entry["labels"] == [] assert "markers" not in entry assert entry["module"] == "" + assert entry["test_ids"] == [] @patch("isvreporter.client.urlopen") def test_markers_field_is_not_forwarded(self, mock_urlopen: MagicMock) -> None: diff --git a/isvtest/src/isvtest/catalog.py b/isvtest/src/isvtest/catalog.py index 5e00e0c6..990626a7 100644 --- a/isvtest/src/isvtest/catalog.py +++ b/isvtest/src/isvtest/catalog.py @@ -142,6 +142,44 @@ def _extract_check_labels_from_config(config_path: Path) -> dict[str, set[str]]: return result +def _extract_check_test_ids_from_config(config_path: Path) -> dict[str, set[str]]: + """Extract per-check ``test_id`` declared on a config's validation wiring. + + The ``"N/A"`` sentinel marks an intentional gap (no plan item) and is + skipped so it never appears as a test id. + """ + result: dict[str, set[str]] = {} + for name, params in iter_config_checks(config_path): + test_id = params.get("test_id") + if isinstance(test_id, str) and test_id and test_id != "N/A": + result.setdefault(name, set()).add(test_id) + return result + + +def build_test_id_map() -> dict[str, set[str]]: + """Map check name -> test_ids declared on its suite/provider YAML wiring. + + test_ids live on the per-check YAML wiring, so this scans every config and + unions the ``test_id`` declared on each check (excluding the ``"N/A"`` + sentinel). A variant's test_ids propagate up to its base name so the base + entry is not left bare, mirroring ``build_label_map``. + """ + configs_dir = _find_configs_dir() + if not configs_dir: + return {} + + test_id_map: dict[str, set[str]] = {} + for config_path in sorted(configs_dir.rglob("*.yaml")): + for name, test_ids in _extract_check_test_ids_from_config(config_path).items(): + test_id_map.setdefault(name, set()).update(test_ids) + + for name, test_ids in list(test_id_map.items()): + base = name.split("-")[0] + if base != name: + test_id_map.setdefault(base, set()).update(test_ids) + return test_id_map + + def build_label_map() -> dict[str, set[str]]: """Map check name -> labels declared on its suite/provider YAML wiring. @@ -215,11 +253,14 @@ def build_catalog(*, released_only: bool = True) -> list[dict[str, Any]]: - name: Validation class name or variant name - description: Human-readable description from class metadata - labels: List of public label strings (e.g. ["kubernetes", "gpu"]) + - test_ids: List of test-plan ids declared on the wiring, "N/A" + excluded (e.g. ["SEC07-01"]); empty when only intentional gaps - module: Fully qualified module path - platforms: List of platform strings (e.g. ["KUBERNETES"]) """ platform_map = _build_platform_map() label_map = build_label_map() + test_id_map = build_test_id_map() # Build class metadata lookup, skipping classes marked for exclusion class_meta: dict[str, dict[str, Any]] = {} @@ -255,6 +296,7 @@ def build_catalog(*, released_only: bool = True) -> list[dict[str, Any]]: "name": name, "description": meta["description"], "labels": meta["labels"], + "test_ids": sorted(test_id_map.get(name, set())), "module": meta["module"], "platforms": sorted(platform_map.get(name, [])), } @@ -274,11 +316,13 @@ def build_catalog(*, released_only: bool = True) -> list[dict[str, Any]]: if variant_suffix: desc = f"{desc} ({variant_suffix.lstrip('-')})" if desc else variant_suffix.lstrip("-") labels = sorted(set(meta.get("labels", [])) | label_map.get(name, set())) + test_ids = sorted(test_id_map.get(name, set())) catalog.append( { "name": name, "description": desc, "labels": labels, + "test_ids": test_ids, "module": meta.get("module", ""), "platforms": sorted(platforms), } diff --git a/isvtest/tests/test_catalog.py b/isvtest/tests/test_catalog.py index 579afbb3..7f1859e2 100644 --- a/isvtest/tests/test_catalog.py +++ b/isvtest/tests/test_catalog.py @@ -49,6 +49,7 @@ def test_entries_have_required_keys(self) -> None: assert "name" in entry assert "description" in entry assert "labels" in entry + assert "test_ids" in entry assert "module" in entry assert "markers" not in entry @@ -93,6 +94,53 @@ def test_extract_checks_supports_direct_dict_category_form(self, tmp_path) -> No assert _extract_checks_from_config(config) == ["DirectCheck", "EmptyParamsCheck"] + def test_extract_check_test_ids_excludes_na_and_blanks(self, tmp_path) -> None: + """Wiring test_ids are extracted per check, with "N/A"/empty dropped.""" + from isvtest.catalog import _extract_check_test_ids_from_config + + config = tmp_path / "test-ids.yaml" + config.write_text( + """\ +tests: + validations: + sample: + checks: + MappedCheck: + test_id: "SEC07-01" + GapCheck: + test_id: "N/A" + BlankCheck: + test_id: "" + NoIdCheck: {} +""", + encoding="utf-8", + ) + + assert _extract_check_test_ids_from_config(config) == {"MappedCheck": {"SEC07-01"}} + + def test_entries_expose_wired_test_ids(self) -> None: + """Catalog entries carry the plan ids declared on their wiring.""" + catalog = build_catalog(released_only=False) + by_name = {e["name"]: e for e in catalog} + + # Every entry has a list-of-strings test_ids and never the "N/A" sentinel. + for entry in catalog: + assert isinstance(entry["test_ids"], list) + assert all(isinstance(tid, str) for tid in entry["test_ids"]) + assert "N/A" not in entry["test_ids"] + + # Single mapping, and a duality unioned across the bm/vm suites. + assert by_name["MfaEnforcedCheck"]["test_ids"] == ["SEC07-01"] + assert by_name["GpuCheck"]["test_ids"] == ["BMAAS08-01", "VMAAS06-01"] + + def test_variant_test_ids_propagate_to_base(self) -> None: + """A variant's wired test_id surfaces on its base-class catalog entry.""" + catalog = build_catalog(released_only=False) + by_name = {e["name"]: e for e in catalog} + + assert by_name["StepSuccessCheck-delete_tenant"]["test_ids"] == ["CP10-01"] + assert "CP10-01" in by_name["StepSuccessCheck"]["test_ids"] + def test_released_only_filters_catalog(self) -> None: """Default catalog generation excludes tests not in the release manifest.""" with patch("isvtest.catalog.load_released_test_filter", return_value={"StepSuccessCheck"}): @@ -125,6 +173,7 @@ def test_catalog_emits_explicit_labels(self) -> None: "isvtest.catalog.build_label_map", return_value={"ExplicitLabelCatalogCheck": {"accelerator", "long_running"}}, ), + patch("isvtest.catalog.build_test_id_map", return_value={}), patch("isvtest.catalog.load_released_test_filter", return_value=None), ): catalog = build_catalog() @@ -134,6 +183,7 @@ def test_catalog_emits_explicit_labels(self) -> None: "name": "ExplicitLabelCatalogCheck", "description": "Explicit labels", "labels": ["accelerator", "long_running"], + "test_ids": [], "module": __name__, "platforms": [], } @@ -210,6 +260,7 @@ def run(self) -> None: "isvtest.catalog.build_label_map", return_value={"ObservabilityLabelledCheck": {"observability"}}, ), + patch("isvtest.catalog.build_test_id_map", return_value={}), patch("isvtest.catalog.load_released_test_filter", return_value=None), ): catalog = build_catalog() @@ -219,6 +270,7 @@ def run(self) -> None: "name": "ObservabilityLabelledCheck", "description": "Observability check labelled but not in any suite", "labels": ["observability"], + "test_ids": [], "module": "isvtest.validations.fake", "platforms": ["OBSERVABILITY"], } From 52fd30c7262589914c21371568f5181800301ab8 Mon Sep 17 00:00:00 2001 From: Alexandre Begnoche Date: Fri, 26 Jun 2026 16:40:10 -0400 Subject: [PATCH 06/10] fix(catalog): exclude abstract _TenantSanitizationCheck base The shared _TenantSanitizationCheck base class was surfacing as its own catalog entry. Mark it catalog_exclude and re-enable the flag on the four concrete subclasses (which would otherwise inherit the exclusion), so only the real sanitization checks appear in the catalog. Signed-off-by: Alexandre Begnoche --- isvtest/src/isvtest/validations/sanitization.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/isvtest/src/isvtest/validations/sanitization.py b/isvtest/src/isvtest/validations/sanitization.py index e77c2e26..aa645864 100644 --- a/isvtest/src/isvtest/validations/sanitization.py +++ b/isvtest/src/isvtest/validations/sanitization.py @@ -102,6 +102,8 @@ class _TenantSanitizationCheck(BaseValidation): single test ID and can be toggled independently in a suite. """ + # Abstract base: the concrete subclasses below are the catalog entries. + catalog_exclude: ClassVar[bool] = True timeout: ClassVar[int] = 120 gpu_only: ClassVar[bool] = False subtest_prefix: ClassVar[str] = "machine" @@ -184,6 +186,7 @@ class MemorySanitizationCheck(_TenantSanitizationCheck): transitions: list[str] -- recent neutral lifecycle sequence """ + catalog_exclude: ClassVar[bool] = False description: ClassVar[str] = "Check host memory is sanitized between tenants" subject: ClassVar[str] = "Host memory sanitization" subtest_prefix: ClassVar[str] = "memory" @@ -204,6 +207,7 @@ class GpuMemorySanitizationCheck(_TenantSanitizationCheck): the in-scope machines). """ + catalog_exclude: ClassVar[bool] = False description: ClassVar[str] = "Check SRAM/GPU memory is sanitized between tenants" subject: ClassVar[str] = "GPU memory sanitization" subtest_prefix: ClassVar[str] = "gpu_memory" @@ -227,6 +231,7 @@ class FirmwareResetCheck(_TenantSanitizationCheck): subtest. """ + catalog_exclude: ClassVar[bool] = False description: ClassVar[str] = "Check TPM/BIOS are reset during tenant transitions" subject: ClassVar[str] = "Firmware reset" subtest_prefix: ClassVar[str] = "firmware" @@ -277,6 +282,7 @@ class DiskSanitizationCheck(_TenantSanitizationCheck): (see ``MemorySanitizationCheck``). """ + catalog_exclude: ClassVar[bool] = False description: ClassVar[str] = "Check storage is sanitized on delete between tenants" subject: ClassVar[str] = "Storage sanitization" subtest_prefix: ClassVar[str] = "disk" From d09a269f16909bb420c2fb9a6b2d3bde3d56ed2d Mon Sep 17 00:00:00 2001 From: Alexandre Begnoche Date: Mon, 29 Jun 2026 10:23:13 -0400 Subject: [PATCH 07/10] refactor: dedup catalog map builders and label-discovery config walk - Generalize build_label_map/build_test_id_map over a shared _build_check_attribute_map(extract_fn) helper (scan + variant->base propagation written once). - Extract _iter_config_validations() in label_discovery so available_labels and discover_provider_label_configs share one merge+parse walk. - Sort catalog labels counts once in labels_cmd instead of per output branch. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Alexandre Begnoche --- isvctl/src/isvctl/cli/catalog.py | 5 +- isvctl/src/isvctl/config/label_discovery.py | 18 +++--- isvtest/src/isvtest/catalog.py | 64 ++++++++++----------- 3 files changed, 44 insertions(+), 43 deletions(-) diff --git a/isvctl/src/isvctl/cli/catalog.py b/isvctl/src/isvctl/cli/catalog.py index 1e34c7de..082c8942 100644 --- a/isvctl/src/isvctl/cli/catalog.py +++ b/isvctl/src/isvctl/cli/catalog.py @@ -121,9 +121,10 @@ def labels_cmd( ISVTEST_INCLUDE_UNRELEASED=1 isvctl catalog labels """ counts = Counter(label for entry in build_catalog() for label in (entry.get("labels") or [])) + sorted_counts = sorted(counts.items()) if json_output: - labels = [{"label": label, "tests": count} for label, count in sorted(counts.items())] + labels = [{"label": label, "tests": count} for label, count in sorted_counts] typer.echo(json.dumps({"labels": labels}, indent=2)) return @@ -137,7 +138,7 @@ def labels_cmd( table.add_column("Label", style="green", no_wrap=True) table.add_column("Tests", style="cyan", justify="right") - for label, count in sorted(counts.items()): + for label, count in sorted_counts: table.add_row(label, str(count)) console.print(table) diff --git a/isvctl/src/isvctl/config/label_discovery.py b/isvctl/src/isvctl/config/label_discovery.py index cfcbe37b..048afe50 100644 --- a/isvctl/src/isvctl/config/label_discovery.py +++ b/isvctl/src/isvctl/config/label_discovery.py @@ -5,14 +5,22 @@ from __future__ import annotations +from collections.abc import Iterator from dataclasses import dataclass from pathlib import Path -from isvtest.core.resolution import parse_validations, resolve_class_key +from isvtest.core.resolution import ValidationEntry, parse_validations, resolve_class_key from isvctl.config.merger import merge_yaml_files +def _iter_config_validations(config_path: Path) -> Iterator[ValidationEntry]: + """Yield the validation entries of a config with its imports resolved.""" + merged = merge_yaml_files([config_path]) + raw_validations = (merged.get("tests") or {}).get("validations") or {} + yield from parse_validations(raw_validations) + + @dataclass(frozen=True) class MatchedCheck: """A validation check that matched requested labels.""" @@ -47,9 +55,7 @@ def available_labels(provider: str, *, configs_root: Path) -> set[str]: provider_config_dir = configs_root / "providers" / provider / "config" labels: set[str] = set() for config_path in provider_config_dir.glob("*.yaml"): - merged = merge_yaml_files([config_path]) - raw_validations = (merged.get("tests") or {}).get("validations") or {} - for entry in parse_validations(raw_validations): + for entry in _iter_config_validations(config_path): labels.update(entry.labels) return labels @@ -74,11 +80,9 @@ def discover_provider_label_configs( matches: list[ProviderConfigMatch] = [] for config_path in sorted(provider_config_dir.glob("*.yaml")): - merged = merge_yaml_files([config_path]) - raw_validations = (merged.get("tests") or {}).get("validations") or {} matched_checks = tuple( MatchedCheck(category=entry.category, name=entry.name, labels=entry.labels) - for entry in parse_validations(raw_validations) + for entry in _iter_config_validations(config_path) if requested.issubset(entry.labels) and (released_tests is None or resolve_class_key(entry.name, released_tests) is not None) ) diff --git a/isvtest/src/isvtest/catalog.py b/isvtest/src/isvtest/catalog.py index 990626a7..4e1d1ec2 100644 --- a/isvtest/src/isvtest/catalog.py +++ b/isvtest/src/isvtest/catalog.py @@ -29,7 +29,7 @@ """ import logging -from collections.abc import Iterator +from collections.abc import Callable, Iterator from pathlib import Path from typing import Any @@ -156,55 +156,51 @@ def _extract_check_test_ids_from_config(config_path: Path) -> dict[str, set[str] return result -def build_test_id_map() -> dict[str, set[str]]: - """Map check name -> test_ids declared on its suite/provider YAML wiring. +def _build_check_attribute_map( + extract_fn: Callable[[Path], dict[str, set[str]]], +) -> dict[str, set[str]]: + """Map check name -> a per-check attribute unioned across all config wiring. - test_ids live on the per-check YAML wiring, so this scans every config and - unions the ``test_id`` declared on each check (excluding the ``"N/A"`` - sentinel). A variant's test_ids propagate up to its base name so the base - entry is not left bare, mirroring ``build_label_map``. + Scans every config (suites AND providers, not just the canonical suites: + on-host ``bm_*`` checks are wired only in provider configs), unions the + values ``extract_fn`` pulls from each, then propagates a variant's values up + to its base name (``Foo-bar`` -> ``Foo``) so the base entry is not left bare. + Shared by ``build_label_map`` and ``build_test_id_map``. """ configs_dir = _find_configs_dir() if not configs_dir: return {} - test_id_map: dict[str, set[str]] = {} + attribute_map: dict[str, set[str]] = {} for config_path in sorted(configs_dir.rglob("*.yaml")): - for name, test_ids in _extract_check_test_ids_from_config(config_path).items(): - test_id_map.setdefault(name, set()).update(test_ids) + for name, values in extract_fn(config_path).items(): + attribute_map.setdefault(name, set()).update(values) - for name, test_ids in list(test_id_map.items()): + for name, values in list(attribute_map.items()): base = name.split("-")[0] if base != name: - test_id_map.setdefault(base, set()).update(test_ids) - return test_id_map + attribute_map.setdefault(base, set()).update(values) + return attribute_map -def build_label_map() -> dict[str, set[str]]: - """Map check name -> labels declared on its suite/provider YAML wiring. +def build_test_id_map() -> dict[str, set[str]]: + """Map check name -> test_ids declared on its suite/provider YAML wiring. - Labels live on the per-check YAML wiring, so this scans every config and - unions the ``labels:`` declared on each check. A variant's labels propagate - up to its base name so the base entry is not left bare. Shared by the - catalog and ``isvctl docs`` so both report the same labels. + test_ids live on the per-check YAML wiring, so every config is scanned and + the ``test_id`` declared on each check is unioned (excluding the ``"N/A"`` + sentinel), mirroring ``build_label_map``. """ - configs_dir = _find_configs_dir() - if not configs_dir: - return {} + return _build_check_attribute_map(_extract_check_test_ids_from_config) - # Scan every config (suites AND providers), not just the canonical suites: - # on-host checks (bm_*) are wired only in provider configs, so their labels - # live there. Per-check ``labels:`` declared anywhere in YAML are unioned. - label_map: dict[str, set[str]] = {} - for config_path in sorted(configs_dir.rglob("*.yaml")): - for name, labels in _extract_check_labels_from_config(config_path).items(): - label_map.setdefault(name, set()).update(labels) - for name, labels in list(label_map.items()): - base = name.split("-")[0] - if base != name: - label_map.setdefault(base, set()).update(labels) - return label_map +def build_label_map() -> dict[str, set[str]]: + """Map check name -> labels declared on its suite/provider YAML wiring. + + Labels live on the per-check YAML wiring, so every config is scanned and the + ``labels:`` declared on each check is unioned. Shared by the catalog and + ``isvctl docs`` so both report the same labels. + """ + return _build_check_attribute_map(_extract_check_labels_from_config) def _build_platform_map() -> dict[str, set[str]]: From 279e648b93f7f8de06be3fdbfe6bbd8e47e7208e Mon Sep 17 00:00:00 2001 From: Alexandre Begnoche Date: Mon, 29 Jun 2026 10:56:15 -0400 Subject: [PATCH 08/10] feat(catalog): add `catalog labels --files` to show declaring configs Add an opt-in --files flag to `isvctl catalog labels` that lists, per label, the config file(s) declaring it (via a new build_label_file_map config scan). Off by default so the table/JSON stay compact; --files adds a Files column and a per-label `files` array in --json. Signed-off-by: Alexandre Begnoche --- isvctl/src/isvctl/cli/catalog.py | 26 ++++++++++++++++++++++---- isvctl/tests/test_catalog_cli.py | 32 +++++++++++++++++++++++++++++++- isvtest/src/isvtest/catalog.py | 19 +++++++++++++++++++ 3 files changed, 72 insertions(+), 5 deletions(-) diff --git a/isvctl/src/isvctl/cli/catalog.py b/isvctl/src/isvctl/cli/catalog.py index 082c8942..570b92fb 100644 --- a/isvctl/src/isvctl/cli/catalog.py +++ b/isvctl/src/isvctl/cli/catalog.py @@ -24,7 +24,7 @@ from typing import Annotated import typer -from isvtest.catalog import build_catalog, get_catalog_version +from isvtest.catalog import build_catalog, build_label_file_map, get_catalog_version from isvtest.release_manifest import load_released_tests from rich.console import Console from rich.table import Table @@ -109,22 +109,35 @@ def labels_cmd( bool, typer.Option("--json", help="Emit the labels as JSON instead of a table"), ] = False, + show_files: Annotated[ + bool, + typer.Option("--files", help="Also show the config file(s) declaring each label"), + ] = False, ) -> None: """List every label in the catalog with the number of tests carrying it. Released tests only by default. Set ``ISVTEST_INCLUDE_UNRELEASED=1`` to - include unreleased validations (matches the gate used at run time). + include unreleased validations (matches the gate used at run time). Pass + ``--files`` to also list the config file(s) that declare each label. Examples: isvctl catalog labels + isvctl catalog labels --files isvctl catalog labels --json ISVTEST_INCLUDE_UNRELEASED=1 isvctl catalog labels """ counts = Counter(label for entry in build_catalog() for label in (entry.get("labels") or [])) sorted_counts = sorted(counts.items()) + label_files = build_label_file_map() if show_files else {} + + def files_for(label: str) -> list[str]: + return sorted(label_files.get(label, set())) if json_output: - labels = [{"label": label, "tests": count} for label, count in sorted_counts] + labels = [ + {"label": label, "tests": count, **({"files": files_for(label)} if show_files else {})} + for label, count in sorted_counts + ] typer.echo(json.dumps({"labels": labels}, indent=2)) return @@ -137,9 +150,14 @@ def labels_cmd( ) table.add_column("Label", style="green", no_wrap=True) table.add_column("Tests", style="cyan", justify="right") + if show_files: + table.add_column("Files", style="dim") for label, count in sorted_counts: - table.add_row(label, str(count)) + row = [label, str(count)] + if show_files: + row.append("\n".join(files_for(label)) or "-") + table.add_row(*row) console.print(table) diff --git a/isvctl/tests/test_catalog_cli.py b/isvctl/tests/test_catalog_cli.py index 78bc3530..253e9c1b 100644 --- a/isvctl/tests/test_catalog_cli.py +++ b/isvctl/tests/test_catalog_cli.py @@ -90,10 +90,11 @@ def test_catalog_labels_table() -> None: assert result.exit_code == 0, result.output assert "iam" in result.output assert "security" in result.output + assert "Files" not in result.output def test_catalog_labels_json_counts_tests_per_label() -> None: - """`catalog labels --json` emits sorted labels with per-label test counts.""" + """`catalog labels --json` (default) emits sorted labels with test counts, no files.""" entries = [ {"name": "A", "labels": ["iam", "security"]}, {"name": "B", "labels": ["iam"]}, @@ -110,6 +111,35 @@ def test_catalog_labels_json_counts_tests_per_label() -> None: ] +def test_catalog_labels_files_option_adds_files() -> None: + """`catalog labels --files --json` includes the declaring config files per label.""" + entries = [ + {"name": "A", "labels": ["iam", "security"]}, + {"name": "B", "labels": ["iam"]}, + {"name": "C", "labels": []}, + ] + file_map = { + "iam": {"suites/control-plane.yaml", "suites/security.yaml"}, + "security": {"suites/security.yaml"}, + } + with ( + patch("isvctl.cli.catalog.build_catalog", return_value=entries), + patch("isvctl.cli.catalog.build_label_file_map", return_value=file_map), + ): + result = runner.invoke(app, ["labels", "--files", "--json"]) + + assert result.exit_code == 0, result.output + payload = json.loads(result.output) + assert payload["labels"] == [ + { + "label": "iam", + "tests": 2, + "files": ["suites/control-plane.yaml", "suites/security.yaml"], + }, + {"label": "security", "tests": 1, "files": ["suites/security.yaml"]}, + ] + + def test_catalog_list_unreleased_json() -> None: """`catalog list --unreleased` emits only entries missing from the release manifest.""" with ( diff --git a/isvtest/src/isvtest/catalog.py b/isvtest/src/isvtest/catalog.py index 4e1d1ec2..24322b01 100644 --- a/isvtest/src/isvtest/catalog.py +++ b/isvtest/src/isvtest/catalog.py @@ -203,6 +203,25 @@ def build_label_map() -> dict[str, set[str]]: return _build_check_attribute_map(_extract_check_labels_from_config) +def build_label_file_map() -> dict[str, set[str]]: + """Map label -> config files (relative to ``isvctl/configs``) that declare it. + + Unlike the catalog this is a raw config scan (not release-gated): it records + every suite/provider YAML where a label appears on a check's wiring. + """ + configs_dir = _find_configs_dir() + if not configs_dir: + return {} + + label_files: dict[str, set[str]] = {} + for config_path in sorted(configs_dir.rglob("*.yaml")): + rel = config_path.relative_to(configs_dir).as_posix() + for labels in _extract_check_labels_from_config(config_path).values(): + for label in labels: + label_files.setdefault(label, set()).add(rel) + return label_files + + def _build_platform_map() -> dict[str, set[str]]: """Build a mapping from test name to set of platform strings. From 10b0851af006a1450c85254a9b24bd785356a2d5 Mon Sep 17 00:00:00 2001 From: Alexandre Begnoche Date: Mon, 29 Jun 2026 11:00:17 -0400 Subject: [PATCH 09/10] fix: address review on provider discovery and catalog list - Upload the per-config JUnit report from provider label discovery: the results upload now prefers the requested --junitxml path before the default name, so discovered runs no longer upload a null report. - catalog list: render platforms-only entries as "PLATFORM" instead of " (PLATFORM)" when a check has platforms but no labels. - Add PEP 257 docstrings to the _FakeOrchestrator test double methods. Signed-off-by: Alexandre Begnoche --- isvctl/src/isvctl/cli/catalog.py | 5 ++++- isvctl/src/isvctl/cli/test.py | 7 +++++-- isvctl/tests/test_test_cli_labels.py | 2 ++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/isvctl/src/isvctl/cli/catalog.py b/isvctl/src/isvctl/cli/catalog.py index 570b92fb..95b748b9 100644 --- a/isvctl/src/isvctl/cli/catalog.py +++ b/isvctl/src/isvctl/cli/catalog.py @@ -92,7 +92,10 @@ def list_cmd( for entry in sorted(catalog_entries, key=lambda e: e["name"]): labels = ", ".join(entry.get("labels") or []) platforms = ", ".join(entry.get("platforms") or []) - labels_platforms = f"{labels} ({platforms})" if platforms else labels + if labels and platforms: + labels_platforms = f"{labels} ({platforms})" + else: + labels_platforms = labels or platforms table.add_row( entry["name"], ", ".join(entry.get("test_ids") or []) or "-", diff --git a/isvctl/src/isvctl/cli/test.py b/isvctl/src/isvctl/cli/test.py index c6b7cca6..5b721c79 100644 --- a/isvctl/src/isvctl/cli/test.py +++ b/isvctl/src/isvctl/cli/test.py @@ -453,8 +453,11 @@ def run( # Update test run after tests complete if upload_results and test_run_id and lab_id: print_progress("Uploading test results to ISV Lab Service...") - # Look for junit XML in _output, working directory, or current directory - junit_path = output_dir / "junit-validation.xml" + # Prefer the requested --junitxml (provider discovery gives each config + # its own report name), then fall back to _output, working dir, or cwd. + junit_path = junitxml + if not junit_path.exists(): + junit_path = output_dir / "junit-validation.xml" if not junit_path.exists(): junit_path = effective_working_dir / "junit-validation.xml" if not junit_path.exists(): diff --git a/isvctl/tests/test_test_cli_labels.py b/isvctl/tests/test_test_cli_labels.py index 3bfdce85..701a23b7 100644 --- a/isvctl/tests/test_test_cli_labels.py +++ b/isvctl/tests/test_test_cli_labels.py @@ -96,10 +96,12 @@ class _FakeOrchestrator: calls: ClassVar[list[dict[str, Any]]] = [] def __init__(self, config: Any, **kwargs: Any) -> None: + """Store constructor inputs so tests can assert CLI wiring.""" self.config = config self.kwargs = kwargs def run(self, **kwargs: Any) -> OrchestratorResult: + """Record a synthetic orchestration call and return a successful result.""" type(self).captured.update(kwargs) type(self).calls.append( { From 34194565d0caadea0e010fbfadd47adffba26e33 Mon Sep 17 00:00:00 2001 From: Alexandre Begnoche Date: Mon, 29 Jun 2026 11:10:50 -0400 Subject: [PATCH 10/10] fix: add docstring to catalog labels files_for helper Address review: PEP 257 docstring required on the nested files_for() helper. Signed-off-by: Alexandre Begnoche --- isvctl/src/isvctl/cli/catalog.py | 1 + 1 file changed, 1 insertion(+) diff --git a/isvctl/src/isvctl/cli/catalog.py b/isvctl/src/isvctl/cli/catalog.py index 95b748b9..0ce16460 100644 --- a/isvctl/src/isvctl/cli/catalog.py +++ b/isvctl/src/isvctl/cli/catalog.py @@ -134,6 +134,7 @@ def labels_cmd( label_files = build_label_file_map() if show_files else {} def files_for(label: str) -> list[str]: + """Return the sorted config files declaring ``label`` (empty without --files).""" return sorted(label_files.get(label, set())) if json_output: