Convert and enable integration tests #730
Conversation
WalkthroughThe PR rewrites and re-enables the integration test suite. Commented scaffolding in tests/integration/conftest.py was replaced with concrete, typed pytest fixtures providing multiple schema dicts. Many integration test modules were converted from placeholders to active tests (including async and sync client variants), updated with explicit type annotations and Docker-backed client usage via infrahub-testcontainers. pyproject.toml Ruff ignores were changed to exclude the entire tests/integration directory and map specific test files to PLR0904. A changelog entry (187.fixed) was added noting the rewrite and re-enablement. Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying infrahub-sdk-python with
|
| Latest commit: |
0c798dc
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://82263b0c.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://gma-20251231-187.infrahub-sdk-python.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## infrahub-develop #730 +/- ##
====================================================
+ Coverage 76.29% 80.34% +4.04%
====================================================
Files 114 114
Lines 9831 9854 +23
Branches 1509 1515 +6
====================================================
+ Hits 7501 7917 +416
+ Misses 1834 1411 -423
- Partials 496 526 +30
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/integration/test_infrahubctl.py (1)
65-79: Environment variable cleanup may leave residual values.When
original_usernameororiginal_passwordisNone(not set originally), the cleanup code doesn't remove the environment variables that were set during the test. This could leak credentials to subsequent tests.🔎 Proposed fix
yield if original_username: os.environ["INFRAHUB_USERNAME"] = original_username + elif "INFRAHUB_USERNAME" in os.environ: + del os.environ["INFRAHUB_USERNAME"] if original_password: os.environ["INFRAHUB_PASSWORD"] = original_password + elif "INFRAHUB_PASSWORD" in os.environ: + del os.environ["INFRAHUB_PASSWORD"]
🧹 Nitpick comments (1)
tests/integration/test_export_import.py (1)
101-105: Duplicatedreset_export_directorymethod.The
reset_export_directorymethod is identical in bothTestSchemaExportImportBase(lines 101-104) andTestSchemaExportImportManyRelationships(lines 265-268). Consider extracting this to a base class or a standalone helper function.🔎 Suggested refactor
Create a shared mixin or move to a common base:
class ExportImportTestMixin: def reset_export_directory(self, temporary_directory: Path) -> None: for file in temporary_directory.iterdir(): if file.is_file(): file.unlink()Then inherit from both the Docker client and the mixin.
Also applies to: 265-269
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
changelog/187.fixed.mdpyproject.tomltests/integration/conftest.pytests/integration/test_convert_object_type.pytests/integration/test_export_import.pytests/integration/test_infrahub_client.pytests/integration/test_infrahub_client_sync.pytests/integration/test_infrahubctl.pytests/integration/test_node.pytests/integration/test_object_store.pytests/integration/test_repository.pytests/integration/test_schema.py
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use type hints on all function signatures
Never mix async/sync inappropriately
Never bypass type checking without justification
Files:
tests/integration/test_convert_object_type.pytests/integration/test_repository.pytests/integration/test_infrahub_client_sync.pytests/integration/test_infrahubctl.pytests/integration/test_object_store.pytests/integration/conftest.pytests/integration/test_schema.pytests/integration/test_node.pytests/integration/test_infrahub_client.pytests/integration/test_export_import.py
tests/**/*.py
📄 CodeRabbit inference engine (tests/AGENTS.md)
tests/**/*.py: Usehttpx_mockfixture for HTTP mocking in tests instead of making real HTTP requests
Do not add@pytest.mark.asynciodecorator to async test functions (async auto-mode is globally enabled)
Files:
tests/integration/test_convert_object_type.pytests/integration/test_repository.pytests/integration/test_infrahub_client_sync.pytests/integration/test_infrahubctl.pytests/integration/test_object_store.pytests/integration/conftest.pytests/integration/test_schema.pytests/integration/test_node.pytests/integration/test_infrahub_client.pytests/integration/test_export_import.py
tests/integration/**/*.py
📄 CodeRabbit inference engine (tests/AGENTS.md)
tests/integration/**/*.py: Integration tests should use testcontainers to interact with real Infrahub instances
Clean up resources in integration tests
Files:
tests/integration/test_convert_object_type.pytests/integration/test_repository.pytests/integration/test_infrahub_client_sync.pytests/integration/test_infrahubctl.pytests/integration/test_object_store.pytests/integration/conftest.pytests/integration/test_schema.pytests/integration/test_node.pytests/integration/test_infrahub_client.pytests/integration/test_export_import.py
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{md,mdx}: Usetextlanguage for directory structure code blocks in markdown documentation
Add blank lines before and after lists in markdown documentation
Always specify language in fenced code blocks in markdown documentation
Files:
changelog/187.fixed.md
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: tests/AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:37.990Z
Learning: Applies to tests/integration/**/*.py : Integration tests should use testcontainers to interact with real Infrahub instances
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: infrahub_sdk/pytest_plugin/AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:29.593Z
Learning: Applies to infrahub_sdk/pytest_plugin/**/*.yaml : Use YAML test format with required fields: `infrahub_tests`, `resource`, `resource_name`, `tests` array containing `name`, `spec.kind`, `input`, and `output`
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:08.136Z
Learning: Applies to infrahub_sdk/**/*.py : Follow async/sync dual pattern for new features in the Python SDK
📚 Learning: 2025-12-10T17:13:08.136Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:08.136Z
Learning: Applies to infrahub_sdk/client.py : Always provide both async and sync versions of client implementations in InfrahubClient
Applied to files:
tests/integration/test_convert_object_type.py
📚 Learning: 2025-12-10T17:13:37.990Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: tests/AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:37.990Z
Learning: Applies to tests/integration/**/*.py : Integration tests should use testcontainers to interact with real Infrahub instances
Applied to files:
tests/integration/test_convert_object_type.pytests/integration/test_infrahub_client_sync.pytests/integration/test_object_store.pytests/integration/test_export_import.py
📚 Learning: 2025-12-10T17:13:29.593Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: infrahub_sdk/pytest_plugin/AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:29.593Z
Learning: Applies to infrahub_sdk/pytest_plugin/**/*.yaml : Use YAML test format with required fields: `infrahub_tests`, `resource`, `resource_name`, `tests` array containing `name`, `spec.kind`, `input`, and `output`
Applied to files:
tests/integration/test_convert_object_type.pytests/integration/test_infrahub_client_sync.pytests/integration/test_infrahubctl.pytests/integration/conftest.pytests/integration/test_infrahub_client.py
📚 Learning: 2025-12-10T17:13:21.977Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: infrahub_sdk/ctl/AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:21.977Z
Learning: Applies to infrahub_sdk/ctl/**/*.py : Do not instantiate InfrahubClient directly; always use initialize_client() or initialize_client_sync() helper functions
Applied to files:
tests/integration/test_convert_object_type.py
📚 Learning: 2025-12-10T17:13:29.593Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: infrahub_sdk/pytest_plugin/AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:29.593Z
Learning: Applies to infrahub_sdk/pytest_plugin/**/infrahub_sdk/pytest_plugin/items/*.py : Inherit from `InfrahubItem` base class when creating new test item classes in `infrahub_sdk/pytest_plugin/items/`
Applied to files:
tests/integration/test_convert_object_type.py
🧬 Code graph analysis (9)
tests/integration/test_convert_object_type.py (2)
infrahub_sdk/client.py (4)
convert_object_type(1732-1762)convert_object_type(3153-3183)InfrahubClient(318-1762)InfrahubClientSync(1765-3183)infrahub_sdk/testing/docker.py (1)
TestInfrahubDockerClient(36-47)
tests/integration/test_infrahub_client_sync.py (4)
infrahub_sdk/constants.py (1)
InfrahubClientMode(4-7)infrahub_sdk/exceptions.py (1)
URLNotFoundError(148-151)infrahub_sdk/task/models.py (4)
Task(32-62)TaskFilter(65-76)TaskLog(21-24)TaskState(9-18)infrahub_sdk/types.py (1)
Order(71-72)
tests/integration/test_infrahubctl.py (5)
tests/fixtures/repos/ctl_integration/generators/tag_generator.py (1)
Generator(4-15)tests/fixtures/repos/ctl_integration/generators/tag_generator_convert.py (1)
Generator(4-16)infrahub_sdk/node/node.py (1)
InfrahubNode(493-1298)infrahub_sdk/transforms.py (1)
client(49-50)infrahub_sdk/generator.py (2)
client(59-62)client(65-66)
tests/integration/test_object_store.py (1)
infrahub_sdk/testing/docker.py (1)
TestInfrahubDockerClient(36-47)
tests/integration/conftest.py (2)
infrahub_sdk/utils.py (1)
str_to_bool(165-194)infrahub_sdk/schema/__init__.py (2)
get(251-279)get(565-605)
tests/integration/test_schema.py (3)
infrahub_sdk/exceptions.py (1)
BranchNotFoundError(51-55)infrahub_sdk/schema/main.py (2)
NodeSchemaAPI(312-314)kind(279-280)infrahub_sdk/schema/__init__.py (4)
all(281-308)all(536-563)get(251-279)get(565-605)
tests/integration/test_node.py (3)
infrahub_sdk/exceptions.py (1)
UninitializedError(158-159)infrahub_sdk/protocols.py (1)
IpamNamespace(587-588)tests/integration/conftest.py (1)
ipam_schema(159-201)
tests/integration/test_infrahub_client.py (3)
infrahub_sdk/constants.py (1)
InfrahubClientMode(4-7)infrahub_sdk/types.py (1)
Order(71-72)tests/unit/sdk/conftest.py (1)
client(32-33)
tests/integration/test_export_import.py (6)
infrahub_sdk/exceptions.py (1)
SchemaNotFoundError(58-62)infrahub_sdk/testing/schemas/car_person.py (6)
SchemaCarPerson(43-253)person_joe(169-172)person_jane(175-178)tag_blue(201-204)tag_red(207-210)tag_green(213-216)infrahub_sdk/transfer/exceptions.py (1)
TransferFileNotFoundError(7-7)infrahub_sdk/transfer/exporter/json.py (2)
LineDelimitedJSONExporter(24-169)export(101-169)infrahub_sdk/transfer/importer/json.py (2)
LineDelimitedJSONImporter(27-200)import_data(54-99)infrahub_sdk/transfer/schema_sorter.py (1)
InfrahubSchemaTopologicalSorter(13-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: integration-tests-latest-infrahub
- GitHub Check: Cloudflare Pages
🔇 Additional comments (22)
changelog/187.fixed.md (1)
1-1: LGTM!The changelog entry accurately summarizes the PR's work to convert and re-enable the integration test suite.
pyproject.toml (1)
350-352: LGTM!The restructuring consolidates per-file ignores by excluding the entire
tests/integration/directory, with specific exceptions for test files that have many test methods (PLR0904). This approach improves maintainability while addressing the "too many public methods" warnings appropriately.tests/integration/test_repository.py (2)
9-10: LGTM!Proper use of
TYPE_CHECKINGto conditionally importPathfor type hints without runtime overhead.
16-16: LGTM!Type annotation correctly added for the
remote_repos_dirparameter. This improves static type checking and aligns with the coding guideline to use type hints on all function signatures.tests/integration/test_schema.py (4)
1-2: LGTM!Type imports correctly added to support the new test methods. The
Anytype is appropriately used for schema fixture typing, andNodeSchemaAPIis used for runtime type validation in tests.Also applies to: 7-7
18-24: LGTM!The test correctly validates schema retrieval by checking for expected namespaces and built-in types. The use of
NodeSchemaAPIfor type validation is appropriate.
26-31: LGTM!The test correctly validates both schema retrieval and caching behavior for a specific kind. The cache assertion ensures the schema manager is functioning as expected.
33-45: LGTM!The test correctly validates loading multiple schemas with convergence. The pattern of loading with
wait_until_converged=Truefollowed byrefresh=Trueensures the schemas are both processed server-side and visible in the client cache.tests/integration/test_convert_object_type.py (2)
4-4: LGTM!Proper use of
TYPE_CHECKINGto conditionally import client types for type hints. This avoids runtime import overhead while enabling static type checking.Also applies to: 12-13
69-71: LGTM!Type annotations correctly added for all parameters. The test appropriately receives both async and sync client fixtures to test the
convert_object_typemethod in both modes while using async operations for setup and verification.tests/integration/test_object_store.py (1)
1-25: LGTM!The test follows the testcontainers pattern correctly by inheriting from
TestInfrahubDockerClient. Type hints are properly applied, and the test logic for upload/retrieval verification is sound. Based on learnings, this correctly uses testcontainers to interact with a real Infrahub instance.tests/integration/test_infrahub_client_sync.py (1)
26-353: Well-structured sync client test suite.The
TestInfrahubClientSyncclass provides comprehensive coverage of the synchronous client API including branch management, node operations, filtering, profiles, tasks, and tracking mode. Thexfailmarkers appropriately document known issues and version requirements.tests/integration/conftest.py (1)
14-201: LGTM!The schema fixtures are well-typed and provide reusable schema definitions for integration tests. The module scope is appropriate since these schemas are static and can be shared across tests within a module.
tests/integration/test_infrahubctl.py (1)
81-165: LGTM!The CLI integration tests properly verify both transform and generator commands with and without node conversion. The test assertions correctly validate the expected output structures.
tests/integration/test_infrahub_client.py (2)
356-380: Potential async/sync mismatch on relationship access.Line 379 uses
site_node.parent.get()withoutawait. Ifparentis a relationship that requires fetching, this might need to beawait site_node.parent.get()or the relationship should be fetched first. However, sincepopulate_store=Truewas used on line 373, the parent node may already be in the store, making the sync access valid.Please verify that
parent.get()works synchronously whenpopulate_store=Trueis used, or if it should beawait parent.get().
23-354: Comprehensive async client test suite.The
TestInfrahubNodeclass provides thorough coverage of the async client API including branch operations, node CRUD, filtering, profiles, tasks, and tracking mode with proper rollback verification. The xfail markers appropriately document known issues.tests/integration/test_export_import.py (2)
25-188: LGTM!The
TestSchemaExportImportBaseclass provides thorough coverage of the export/import workflow, including edge cases like exporting before schema loading, importing with existing data (idempotency), and error handling for invalid directories.
190-327: LGTM!The
TestSchemaExportImportManyRelationshipsclass properly tests the export/import workflow for many-to-many relationships, ensuring relationship counts are preserved after import.tests/integration/test_node.py (4)
415-417: Potential type mismatch in IP address assertion.Line 417 compares
fetched_device.primary_address.peer.address.value(likely a string like"192.168.0.1/32") withipaddress.ip_interface("192.168.0.1/32")(anIPv4Interfaceobject). This comparison may fail unlessaddress.valuereturns anip_interfaceobject.Please verify that
address.valuereturns anipaddress.ip_interfaceobject, or adjust the assertion:- assert fetched_device.primary_address.peer.address.value == ipaddress.ip_interface("192.168.0.1/32") + assert str(fetched_device.primary_address.peer.address.value) == "192.168.0.1/32"
142-193: LGTM!The tests for
_generate_input_data, property creation, and the new relationship management tests (test_relationship_manager_errors_without_fetch,test_relationships_not_overwritten) provide valuable coverage for edge cases and ensure proper error handling.
296-324: Good idempotency test coverage.The
test_node_update_idempotencytest properly validates that re-saving a node with unchanged values doesn't generate unnecessary update data, which is important for performance and avoiding spurious mutations.
326-417: LGTM for IPAM pool tests!The
TestNodeWithPoolsclass provides good coverage for IP address pool functionality including device creation from pools, idempotent allocation, and updating existing assignments. The fixtures properly set up the IPAM infrastructure.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/integration/test_infrahub_client_sync.py (1)
271-332: Consider exposing a public method for retrieving the generated group name.Lines 292, 303, 314, and 329 access
client_sync.group_context._generate_group_name(), a private method. While this pattern is acceptable here since no public API exists and the method is explicitly unit tested, it would be cleaner to expose a publicget_group_name()method ongroup_contextto avoid relying on private implementation details.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/integration/test_infrahub_client_sync.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use type hints on all function signatures
Never mix async/sync inappropriately
Never bypass type checking without justification
Files:
tests/integration/test_infrahub_client_sync.py
tests/**/*.py
📄 CodeRabbit inference engine (tests/AGENTS.md)
tests/**/*.py: Usehttpx_mockfixture for HTTP mocking in tests instead of making real HTTP requests
Do not add@pytest.mark.asynciodecorator to async test functions (async auto-mode is globally enabled)
Files:
tests/integration/test_infrahub_client_sync.py
tests/integration/**/*.py
📄 CodeRabbit inference engine (tests/AGENTS.md)
tests/integration/**/*.py: Integration tests should use testcontainers to interact with real Infrahub instances
Clean up resources in integration tests
Files:
tests/integration/test_infrahub_client_sync.py
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: tests/AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:37.990Z
Learning: Applies to tests/integration/**/*.py : Integration tests should use testcontainers to interact with real Infrahub instances
📚 Learning: 2025-12-10T17:13:37.990Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: tests/AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:37.990Z
Learning: Applies to tests/integration/**/*.py : Integration tests should use testcontainers to interact with real Infrahub instances
Applied to files:
tests/integration/test_infrahub_client_sync.py
📚 Learning: 2025-12-10T17:13:29.593Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: infrahub_sdk/pytest_plugin/AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:29.593Z
Learning: Applies to infrahub_sdk/pytest_plugin/**/*.yaml : Use YAML test format with required fields: `infrahub_tests`, `resource`, `resource_name`, `tests` array containing `name`, `spec.kind`, `input`, and `output`
Applied to files:
tests/integration/test_infrahub_client_sync.py
📚 Learning: 2025-12-10T17:13:08.136Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:08.136Z
Learning: Applies to infrahub_sdk/**/*.py : Follow async/sync dual pattern for new features in the Python SDK
Applied to files:
tests/integration/test_infrahub_client_sync.py
📚 Learning: 2025-12-10T17:13:08.136Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-10T17:13:08.136Z
Learning: Applies to **/*.py : Never mix async/sync inappropriately
Applied to files:
tests/integration/test_infrahub_client_sync.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: unit-tests (3.11)
- GitHub Check: unit-tests (3.14)
- GitHub Check: unit-tests (3.13)
- GitHub Check: unit-tests (3.12)
- GitHub Check: unit-tests (3.10)
- GitHub Check: documentation
- GitHub Check: Cloudflare Pages
🔇 Additional comments (6)
tests/integration/test_infrahub_client_sync.py (6)
1-40: LGTM!The imports, class structure, and base dataset fixture are well-organized. The async fixture correctly uses the async client for test setup.
334-354: LGTM!The recorder and playback test is well-structured. It correctly creates a fresh client for recording and validates playback functionality with proper assertions.
356-380: LGTM!The hierarchical schema tests are correctly implemented with synchronous methods matching the synchronous client usage. The past async/sync mismatch has been properly addressed.
42-270: LGTM!The test methods comprehensively cover client functionality including branch operations, node retrieval, filtering, counting, profiles, and task management. All methods correctly use synchronous APIs with proper type hints and assertions.
333-333: Issue #733 remains open and unresolved; xfail marker should remain.Issue #733 ("task: JSONPlayback can be tricky to test") is still in OPEN state and has not been closed. The xfail marker is correctly applied and should not be removed at this time.
164-164: Remove or update xfail marker—v1.7 requirement is likely outdated.The xfail marker referencing "Require Infrahub v1.7" appears outdated. The SDK version is 1.17.0 and integration tests run against a locally built Infrahub instance (INFRAHUB_IMAGE_VER: "local" in CI), not a pinned v1.7 release. Test whether
test_profile_relationship_is_from_profilepasses without the xfail marker. If it passes, remove the marker. If it fails, replace the reason with a reference to the specific blocking issue.
Fixes #187
This PR rewrites integration tests that were previously disabled. These tests are now based on the
TestInfrahubDockerClient. Some tests are marked asXFAILfor now as they either highlight issues or require a different version of Infrahub server.Summary by CodeRabbit
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.