Skip to content

test: add integration test suite for InfrahubInventory (NORNIR-5)#79

Merged
minitriga merged 14 commits into
stablefrom
ml-20260523-integration-tests-nornir-5
Jun 4, 2026
Merged

test: add integration test suite for InfrahubInventory (NORNIR-5)#79
minitriga merged 14 commits into
stablefrom
ml-20260523-integration-tests-nornir-5

Conversation

@lancamat1

@lancamat1 lancamat1 commented May 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds an integration test suite under tests/integration/ that validates InfrahubInventory against a live Infrahub instance spun up via infrahub-testcontainers
  • 14 tests across 6 classes covering: basic loading, schema mappings, group mappings, CoreStandardGroup membership, filters, and branch routing
  • Fixes a bug in InfrahubInventory where schema_mappings and group_mappings that traverse relationships (e.g. primary_address.address, site.name) silently returned None — mapped relationship names are now auto-added to the device fetch include list

Test Plan

  • Run uv run pytest tests/unit/ -v — 17 unit tests pass
  • Run uv run pytest tests/integration/ -v -p no:pytest-infrahub-performance-test --override-ini="addopts=-v" with Docker running — 14 integration tests pass (requires Docker Desktop)
  • Integration tests are excluded from the default pytest run via -m 'not integration' in addopts so existing CI is unaffected

Notes

  • Integration tests require Docker — CI wiring (huge-runners) is a follow-up
  • Artifact task tests (get_artifact, generate_artifacts, regenerate_host_artifact) are deferred to a follow-up
  • The psutil.cpu_freq() crash from the infrahub-testcontainers perf plugin on macOS/Python 3.13 is worked around by disabling the plugin in addopts

Closes #78

🤖 Generated with Claude Code


Summary by cubic

Adds an integration test suite for InfrahubInventory using infrahub-testcontainers, plus runner config and docs to make it easy to use. Schema/group mapping tests now pass explicit relationship includes; the broader auto-include fix remains deferred to PR #74 (NORNIR-5).

  • New Features

    • Integration tests under tests/integration/ with a base conftest that seeds schema, sample devices/sites/IPs, a CoreStandardGroup, and a feature branch; 14 tests across 6 classes cover loading, schema/group mappings, group membership, filters, and branch routing. Mapping tests pass explicit host_node.include to hydrate relations.
    • Tests spin up a fresh Infrahub per class via infrahub-testcontainers; admin token is read from infrahub_testcontainers.helpers.PROJECT_ENV_VARIABLES. AGENTS.md updated with integration test guidance.
    • Tests are marked integration and excluded by default; pyproject.toml adds "-m 'not integration'" and disables pytest-infrahub-performance-test to avoid a macOS psutil crash.
  • Dependencies

    • Add dev dependency infrahub-testcontainers>=1.9.3.

Written for commit cce0cd1. Summary will update on new commits. Review in cubic

@lancamat1 lancamat1 requested a review from a team as a code owner May 23, 2026 18:01

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 7 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="docs/superpowers/specs/2026-05-23-integration-tests-design.md">

<violation number="1" location="docs/superpowers/specs/2026-05-23-integration-tests-design.md:67">
P3: Use the actual mapped value here. `platform.nornir_platform` resolves to `arista_eos`, not `eos`.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic


### `TestSchemaMappings`
- `schema_mappings: [{name: hostname, mapping: primary_address.address}]` → `router-1` host has `hostname == "10.0.0.1"`
- `schema_mappings: [{name: platform, mapping: platform.nornir_platform}]` → `router-1` host has `platform == "eos"`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Use the actual mapped value here. platform.nornir_platform resolves to arista_eos, not eos.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/superpowers/specs/2026-05-23-integration-tests-design.md, line 67:

<comment>Use the actual mapped value here. `platform.nornir_platform` resolves to `arista_eos`, not `eos`.</comment>

<file context>
@@ -0,0 +1,105 @@
+
+### `TestSchemaMappings`
+- `schema_mappings: [{name: hostname, mapping: primary_address.address}]` → `router-1` host has `hostname == "10.0.0.1"`
+- `schema_mappings: [{name: platform, mapping: platform.nornir_platform}]` → `router-1` host has `platform == "eos"`
+
+### `TestGroupMappings`
</file context>

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented May 23, 2026

Copy link
Copy Markdown

Deploying nornir-infrahub with  Cloudflare Pages  Cloudflare Pages

Latest commit: cce0cd1
Status: ✅  Deploy successful!
Preview URL: https://c6a262fa.nornir-infrahub.pages.dev
Branch Preview URL: https://ml-20260523-integration-test.nornir-infrahub.pages.dev

View logs

@lancamat1 lancamat1 force-pushed the ml-20260523-integration-tests-nornir-5 branch from 20839bc to 9ab4ede Compare May 23, 2026 18:27
lancamat1 and others added 8 commits May 23, 2026 20:33
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…crash

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
schema_mappings and group_mappings that traverse relationships (e.g.
primary_address.address, site.name) require those relationship names to
be in the include list so Infrahub returns full peer data. Previously
only member_of_groups was auto-added; other mapped relationships came
back as stubs with None attribute values.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Disable MD060 (table column alignment) in .markdownlint.yml — impractical with wide description columns
- Add PLR2004 to tests/** ruff ignores — magic numbers in assertions are idiomatic
- Run ruff format to fix set comprehension style in test_inventory.py

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lancamat1 lancamat1 force-pushed the ml-20260523-integration-tests-nornir-5 branch from 9ab4ede to f7a640f Compare May 23, 2026 18:33
lancamat1 and others added 4 commits May 25, 2026 16:52
- Add two unit tests covering the auto-include behavior added in c756442:
  * relationships referenced by schema/group mappings are added to include
  * pre-existing relationships in include are not duplicated
- Import TEST_TOKEN from infrahub_testcontainers.PROJECT_ENV_VARIABLES instead
  of hardcoding the same UUID
- Document the integration test suite, the auto-include behavior, and the
  macOS psutil workaround in AGENTS.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The crash is an Apple Silicon (arm64) psutil bug, not Python 3.13 specific.
Also reference the actual call site (host.get_system_stats via
InfrahubPerformanceTest.__init__) for context.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Document the -p no:pytest-infrahub-performance-test workaround where it
actually lives (pyproject.toml) instead of in AGENTS.md, so future
maintainers see the rationale at the point of confusion.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The auto-include fix (c756442) made the manual host_node.include step
unnecessary for relationships referenced in schema_mappings or
group_mappings. Update the user docs accordingly:

- configuring-schema-mappings.mdx: rewrite the :::important callout
  (which claimed include was required) as a :::note describing the
  auto-include behavior, with a pointer to the performance-optimization
  section for users who still want to limit query scope
- Remove the now-redundant include lines from the beginner examples in
  getting-started.mdx, your-first-automation-workflow.mdx, the Step 5
  example in configuring-schema-mappings.mdx, and the reference doc

The performance-optimization angle (selective includes in the concepts
topic) remains valid and is left untouched.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lancamat1

Copy link
Copy Markdown
Contributor Author

Scope note: this PR started as integration tests for InfrahubInventory, but writing them surfaced a real bug — schema_mappings/group_mappings traversal silently returned None because mapped relationship names weren't being added to the device fetch include list. So the PR ended up touching three things in one branch:

  1. Integration test suite under tests/integration/ using infrahub-testcontainers (the original goal — not run by CI, opt-in via pytest -m integration)
  2. 3-line bugfix in nornir_infrahub/plugins/inventory/infrahub.py that auto-includes mapped relationships, plus 2 unit tests covering it
  3. User-doc updates to reflect the bugfix — PR update nornir docs #55 had just landed docs telling users they must manually populate host_node.include, which became false the moment the bugfix went in

Could be split, but the test work, the bug it exposed, and the docs that follow from it are tightly linked. Happy to split into separate PRs if reviewers prefer.

PR #74 (wvandeun, open since 2026-04-23) fixes the same bug with a more
comprehensive approach: deletes the redundant pre-fetch loop and adds
construction-time validation for multi-hop and unknown-relationship
mappings. Defer to that PR.

Reverts:
- the 3-line auto-include in InfrahubInventory.__init__
- the two unit tests asserting the auto-include behavior
- the AGENTS.md note describing the auto-include
- the 4 user-doc files (configuring-schema-mappings.mdx,
  references/plugins/infrahub_inventory.mdx, getting-started.mdx,
  your-first-automation-workflow.mdx) restored to origin/stable

Integration tests for schema_mappings/group_mappings remain. They will
fail locally without PR #74 applied; that's expected and documents the
problem #74 fixes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lancamat1

Copy link
Copy Markdown
Contributor Author

Scope update: Dropped the bugfix and doc changes from this PR — they're superseded by #74 (same fix, more comprehensive: deletes the redundant pre-fetch loop and adds construction-time validation for multi-hop / unknown-relationship mappings).

This PR is now purely:

  • Integration test suite under tests/integration/ (infrahub-testcontainers, opt-in via pytest -m integration)
  • AGENTS.md + lint config updates that came with it

TestSchemaMappings and TestGroupMappings will fail locally until #74 lands — that's the bug they exist to catch. CI is unaffected (integration tests are excluded from default pytest run).

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 7 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="docs/docs/guides/configuring-schema-mappings.mdx">

<violation number="1">
P3: Don't make `host_node.include` mandatory here. The integration tests resolve these mappings without any `include` entries, so this guide now contradicts the actual behavior. Update the Step 5 `# required` comments too.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

These tests previously assumed the auto-include behavior we reverted
(deferring to PR #74). Without auto-include, the SDK overwrites the
hydrated peer cached by the pre-fetch loop with a stub on the host
fetch, so resolve_node_mapping reads None. Passing include explicitly
on the host_node tells the SDK to hydrate those relations directly,
which works on both current code and PR #74's branch (where it gets
merged with auto-derived).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tests/integration/test_inventory.py">

<violation number="1" location="tests/integration/test_inventory.py:36">
P2: These mapping tests now manually include relationship fields, which masks regressions in the auto-include behavior the integration suite is supposed to protect.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

class TestSchemaMappings(NornirInfrahubIntegration):
def test_ip_address_mapping(self, infrahub_address: str) -> None:
inventory = InfrahubInventory(
host_node={"kind": "InfraDevice", "include": ["primary_address"]},

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: These mapping tests now manually include relationship fields, which masks regressions in the auto-include behavior the integration suite is supposed to protect.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/integration/test_inventory.py, line 36:

<comment>These mapping tests now manually include relationship fields, which masks regressions in the auto-include behavior the integration suite is supposed to protect.</comment>

<file context>
@@ -33,7 +33,7 @@ def test_host_names_match(self, infrahub_address: str) -> None:
     def test_ip_address_mapping(self, infrahub_address: str) -> None:
         inventory = InfrahubInventory(
-            host_node={"kind": "InfraDevice"},
+            host_node={"kind": "InfraDevice", "include": ["primary_address"]},
             address=infrahub_address,
             token=TEST_TOKEN,
</file context>
Suggested change
host_node={"kind": "InfraDevice", "include": ["primary_address"]},
host_node={"kind": "InfraDevice"},

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not actionable in this PR. The auto-include behavior the reviewer wants the tests to protect doesn't exist on this branch — it was reverted in commit "defer auto-include changes to PR #74". Manual include doesn't mask a regression in code that isn't here.

Note that cubic gave the exact opposite feedback on configuring-schema-mappings.mdx minutes earlier ("tests don't use include, so docs are wrong"). Both reviews assume auto-include lives in this PR; it doesn't.

When #74 merges and reintroduces auto-include, removing the explicit include from TestSchemaMappings/TestGroupMappings becomes meaningful regression coverage and naturally falls under #74 scope.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback.

@lancamat1 lancamat1 mentioned this pull request May 27, 2026
6 tasks
@minitriga minitriga merged commit 199c8a6 into stable Jun 4, 2026
12 checks passed
@minitriga minitriga deleted the ml-20260523-integration-tests-nornir-5 branch June 4, 2026 09:44
lancamat1 added a commit that referenced this pull request Jun 4, 2026
Adds tests/integration/test_file_object.py running against a live Infrahub
spun up by #79's testcontainers harness. Loads a TestingTestFile schema
that inherits from CoreFileObject plus a label attribute, and exercises:

- upload_lifecycle: create → idempotent skip → update on content change
- download_returns_content: bytes round-trip via base64
- download_skip_when_local_matches: second download is a no-op
- data_attr_persists_on_skip: label is saved even when the file content
  is unchanged (verifies the metadata-on-skip behaviour)
- explicit_object_id_not_found_fails: typo'd UUID fails cleanly
- unknown_key_in_data_fails_on_update: bogus data key is rejected

Run with 'pytest -m integration' (requires Docker). All 6 pass locally
against the testcontainers Infrahub in ~70s.

Closes the integration-testing follow-up referenced in the testing-status
comment on PR #71 (post-#79).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tests: implement integration test suite using infrahub-testcontainers

2 participants