feat: platform/module test selection with capability x module matrix#530
Draft
abegnoche wants to merge 13 commits into
Draft
feat: platform/module test selection with capability x module matrix#530abegnoche wants to merge 13 commits into
abegnoche wants to merge 13 commits into
Conversation
…dule) Reframe test selection around the capability x module matrix without a directory move, manifest, or catalog-kind redesign. - Add tests.kind (capability|module) to the 10 suites; provider configs inherit it via import. Govern labels in validate_suite_wiring.py: derive the capability/module axes from suites' kind+platform, allowlist modifier labels, and require <=1 capability label per check (scanning provider configs too). - Add --capability (run a whole column: capability config + all module configs, with capability-scoped excludes and continue-on-failure) and --module (run one module suite). Resolve by effective kind, never by filename. Plumb CLI exclude_labels through Orchestrator.run() so they apply even when include/pytest selection bypasses config excludes. PR 485 --label discovery is unchanged. Add my-isv k8s/slurm capability wrappers. - Re-home host syslog + BMC SEL/telemetry observability checks from the observability suite to bare_metal (they need a live host/BMC); update aws and my-isv provider configs and sync docs/test-plan.yaml. - Label the 4 K8s CSI checks with "storage" and allowlist it as a module label. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com>
Replace the redundant tests.platform + tests.kind pairing in suites with a single axis key whose value is also the runtime platform: - ValidationConfig gains capability/module (mutually exclusive) and derives platform from whichever is set; the standalone kind field is removed. The 10 suites now declare `capability: <name>` or `module: <name>` only. - Classification (capability_resolution, validate_suite_wiring) reads the axis key; the validator requires exactly one per suite. - get_platform_from_config (raw-read upload path that skips imports) falls back to the capability/module value, so nico's standalone configs drop the redundant explicit platform and declare only `module: <name>`. Observability provider configs declare `module: observability` explicitly (inherited axis keys are invisible to the raw path). - Update schema JSON, docs, and tests accordingly. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com>
Avoid a competing vocabulary: platform is already the runtime, commands key, upload/report term, and YAML axis key throughout the codebase. Service-line suites declare tests.platform; modules declare tests.module (whose value is the derived runtime platform). - Drop the capability field from ValidationConfig; classify suites by platform-or-module. Revert the four capability suites to platform:. - Rename capability_resolution to platform_resolution; --capability becomes --platform. Platform/module label axes, wiring validation, schema, docs, provider comments, and tests follow the same terminology. - GPU compute capability, DPU capability inventory, and test-plan taxonomy nodes are unchanged. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com>
Move the tests.platform / tests.module axis key to the first line of the tests: block in every suite so classification is visible at a glance and the block shape is consistent across suites. Pure YAML key reordering; no behavior change (validate-suites and demo-test still pass). Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com>
Wrap the catalog document in a versioned envelope: a schemaVersion (independent of the isvtest package version), plus the platform (capability) and module axis label lists derived from the suite axis keys. These let a consumer build the capability x module matrix straight from the catalog instead of a hardcoded taxonomy. isvreporter forwards the new fields on upload; the per-entry label universe is intentionally not summarized at the top level since it is derivable from the entries. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com>
Remove the hand-maintained label allowlists (MODIFIER_LABELS, EXTRA_MODULE_LABELS, SUITE_REQUIRED_LABELS) from the wiring validator and catalog. These duplicated data already declared in the suite YAML: the required suite label is now derived from each suite's tests.platform / tests.module axis key, and the module axis is derived purely from module: keys. Labels are otherwise free-form (they originate in the wiring YAML, so there is no external allowlist to guard). The at-most-one-platform-label rule and the per-check axis-label requirement are preserved. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com>
Consolidate the platform/module selection code paths: - Make isvtest.catalog.build_axis_taxonomy the single canonical scanner of suite axis keys (tests.platform / tests.module); platform_resolution and validate_suite_wiring now delegate to it instead of carrying mirrored copies. - cli/test.py: drop the duplicated JUnit-path helper and thread the shared run() options through one forward_kwargs dict used by both the label-discovery loop and the planned-run loop. - resolve_module_configs classifies the provider once per invocation instead of once per --module flag. - platform_resolution: fold the copy-pasted no-match/multi-match error blocks into _single_config, replace the half-used kind constants with a Literal type, derive the CLI platform aliases from the reporter's canonical alias table, and reject configs declaring both axis keys (matching the schema's mutual-exclusion rule). - label_discovery.provider_config_dir now owns the providers/<name>/config path convention. - validate_suite_wiring: read and parse each file once, share the line-attribution bookkeeping between the suite and provider loops, and simplify the single-error governance helper. - Move the duplicated axis suite/provider fixture builders into isvctl/tests/conftest.py. - Describe commands keys by the axis-key rule instead of enumerating platform names in schema descriptions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com>
The per-entry platforms field was built from hardcoded PLATFORM_CONFIGS / LABEL_TO_PLATFORM tables that predated the platform/module axis split and treated all ten suites as platforms - so module-suite checks (e.g. the control-plane IAM checks) were mis-tagged platforms: [CONTROL_PLANE]. Derive per-entry axis placement from each suite's declared tests.platform / tests.module key instead (reusing the canonical axis scanner), and emit a separate modules field alongside platforms. Drop the barely-used per-entry module (Python import path) field that also clashed with the module axis name. The document schemaVersion stays 1 (backend consumers move in step on their dev branches). Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com>
Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com>
A module suite's derived tests.platform is its module name, so module runs uploaded IAM/NETWORK/... as the test target type, polluting the capability namespace. Runs now report both axes: platform runs upload (capability, -), module runs inside a --platform column inherit the column as their capability via PlannedRun.column_platform, and standalone module runs upload (-, module) with no capability. isvctl deploy resolves the axis pair through import: (new effective_axes helper) instead of reading raw keys, and isvreporter create gains --module plus pair auto-detection that passes unknown axis values through instead of coercing them to kubernetes. Dry-run plans now print the upload axes per run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com>
Generic check classes (StepSuccessCheck, GpuCheck, ...) were wired under one bare name across many suites, so the class-keyed catalog unioned unrelated labels/test_ids onto a single entry: the IAM matrix row counted checks that only run under bare_metal, and per-column counts were artifacts of reuse, not coverage. Every duplicated wiring (121 across 12 files) now uses a distinct variant name per wiring (StepSuccessCheck-iam_teardown, GpuCheck-bm_gpu), so catalog entries, JUnit case names, and matrix cells are 1:1 with wirings. The wiring validator enforces global name uniqueness. The catalog no longer propagates variant attributes to base names, variant entries carry only their own wiring's labels/test_ids, classes wired only under variant names get no bare entry, and the released-only filter accepts variants of released classes (mirroring the runtime gating). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com>
The unique-name refactor (2452add) relies on generic check classes (FieldValueCheck, StepSuccessCheck, ...) never being wired under their bare class name, but nothing enforced it: two bare FieldValueCheck wirings in the NICo network provider config passed validation while silently smearing unrelated labels/test_ids onto one catalog entry. Add two guardrails: - variant_required on the reusable classes in generic.py, enforced at resolve time (bare name -> INVALID_CONFIG) and on the raw --config pytest discovery path. Pre-resolved runs are exempt since resolution already gated them and leaves unique names bare for pytest storage. - validate_suite_wiring.py now scans provider configs for within-file duplicate names and rejects bare variant_required wirings, in addition to the existing cross-suite uniqueness check. Rename the offending bare wirings to unique variants and update the orchestrator/resolution tests that wired bare generic names. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reframes test selection around a platform x module matrix and cleans up the surrounding classification/catalog/reporting machinery. No directory moves or manifest redesign — suites are classified by a single axis key declared in their YAML.
Key changes:
tests:—platform: <name>for a service-line platform (vm,bare_metal,kubernetes,slurm) ormodule: <name>for an operational concern (iam,network,security,observability,control_plane,image_registry, ...). Provider configs inherit the key viaimport:.test rungains--platform(run a platform column: the platform config plus every module config, with platform-scoped excludes) and--module(run one module suite); both require--providerand are mutually exclusive with-f.platformvocabulary (runtime,commandskey, upload term, YAML axis key) instead of introducing a competingcapability/kindpairing.MODIFIER_LABELS,EXTRA_MODULE_LABELS,SUITE_REQUIRED_LABELS); the required suite label and module axis are derived from each suite's axis key. Labels are otherwise free-form.schemaVersionplusplatformsandmodulesaxis lists derived from the suites, so a consumer can build the matrix straight from the catalog. Per-entry axes are split intoplatformsandmodules.StepSuccessCheck-iam_teardown,GpuCheck-bm_gpu) so catalog entries, JUnit case names, and matrix cells are 1:1 with wirings.variant_requiredon the reusable classes plusvalidate_suite_wiring.pyenforce global name uniqueness and reject bare wirings.(capability, module)pair — platform runs report(platform, -), module runs inside a--platformcolumn inherit the column as capability, and standalone module runs report(-, module).isvctl deployresolves the pair throughimport:via a neweffective_axeshelper;isvreportergains--moduleand passes unknown axis values through instead of coercing tokubernetes.bare_metal(they need a live host/BMC).Test plan
make lint&&make testmake demo-testpython scripts/validate_suite_wiring.pymake planrenders cleanlyOpened as a draft for review.