-
Notifications
You must be signed in to change notification settings - Fork 6
Fix pytest plugin for integration tests and empty YAML files #744
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: stable
Are you sure you want to change the base?
Conversation
- Add missing instantiate_transform() call in integration test runner - Handle empty YAML files gracefully in loader to avoid TypeError - Make input field optional in InfrahubInputOutputTest model - Default input to None for InfrahubIntegrationTest since integration tests get input from live GraphQL queries, not files
WalkthroughThe changes modify the pytest plugin for Infrahub across three files. The transform integration item now calls 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: |
5295a13
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4af976ec.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://may-202601-pytest-plugin-upd.infrahub-sdk-python.pages.dev |
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## stable #744 +/- ##
==========================================
- Coverage 80.30% 80.29% -0.01%
==========================================
Files 115 115
Lines 9874 9876 +2
Branches 1518 1518
==========================================
+ Hits 7929 7930 +1
- Misses 1417 1418 +1
Partials 528 528
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
infrahub_sdk/pytest_plugin/items/python_transform.py (1)
90-99: Avoid double-instantiation in integration runs (instantiate now happens twice).With the new
self.instantiate_transform()on Line 92 and the existingrun_transform()behavior, integration tests will create two transform instances per test. Consider caching / “ensure instantiated” so the pre-query instantiation is reused for the actual run.One approach (initialize + guard)
class InfrahubPythonTransformItem(InfrahubItem): @@ ) -> None: super().__init__(*args, resource_name=resource_name, resource_config=resource_config, test=test, **kwargs) - self.transform_instance: InfrahubTransform + self.transform_instance: InfrahubTransform | None = None def instantiate_transform(self) -> None: + if self.transform_instance is not None: + return relative_path = ( str(self.resource_config.file_path.parent) if self.resource_config.file_path.parent != Path() else None # type: ignore[attr-defined] ) @@ self.transform_instance = transform_class(branch="", client=client, infrahub_node=InfrahubNode) def run_transform(self, variables: dict[str, Any]) -> Any: self.instantiate_transform() - return asyncio.run(self.transform_instance.run(data=variables)) + assert self.transform_instance is not None + return asyncio.run(self.transform_instance.run(data=variables))infrahub_sdk/pytest_plugin/models.py (1)
30-41: Aligninputfield semantics: either make it required (Pathtype with default) or truly optional (Path | NonewithNonedefault).The current signature
input: Path | None = Field(Path("input.json"))is semantically contradictory—the optional type hint conflicts with the non-None default. The implementation does handleNonecorrectly (with fallback pattern matching viasearch_input: Path | str = self.input or "input.*"), butInfrahubIntegrationTestuses the patterninput: Path | None = Field(None), suggesting the base class should align. Either change the base class default toNone(matching the optional type and subclass pattern) or remove theNonetype and commit to a required input with default discovery behavior.
🧹 Nitpick comments (1)
infrahub_sdk/pytest_plugin/models.py (1)
63-82: Remove now-unreachable"input.*"fallback (or rework discovery logic if you intended it).Because of
if self.input is not None ...,search_input = self.input or "input.*"will never use"input.*". If this is just legacy residue, simplify; if you intended wildcard discovery wheninputis unset, the control-flow needs adjusting.Proposed minimal cleanup
- if self.input is not None and not self.input.is_file(): - search_input: Path | str = self.input or "input.*" + if self.input is not None and not self.input.is_file(): + search_input: Path | str = self.input results = list(self.directory.rglob(str(search_input)))
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
infrahub_sdk/pytest_plugin/items/python_transform.pyinfrahub_sdk/pytest_plugin/loader.pyinfrahub_sdk/pytest_plugin/models.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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:
infrahub_sdk/pytest_plugin/items/python_transform.pyinfrahub_sdk/pytest_plugin/loader.pyinfrahub_sdk/pytest_plugin/models.py
infrahub_sdk/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Follow async/sync dual pattern for new features in the Python SDK
Files:
infrahub_sdk/pytest_plugin/items/python_transform.pyinfrahub_sdk/pytest_plugin/loader.pyinfrahub_sdk/pytest_plugin/models.py
🧠 Learnings (6)
📓 Common learnings
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: 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/loader.py : Register new test items in `ITEMS_MAPPING` in `infrahub_sdk/pytest_plugin/loader.py`
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/`
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 : Define test `kind` values according to resource type and test level (smoke, unit, integration) as specified in the test kinds table
📚 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:
infrahub_sdk/pytest_plugin/items/python_transform.pyinfrahub_sdk/pytest_plugin/loader.pyinfrahub_sdk/pytest_plugin/models.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/loader.py : Register new test items in `ITEMS_MAPPING` in `infrahub_sdk/pytest_plugin/loader.py`
Applied to files:
infrahub_sdk/pytest_plugin/items/python_transform.pyinfrahub_sdk/pytest_plugin/loader.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:
infrahub_sdk/pytest_plugin/items/python_transform.pyinfrahub_sdk/pytest_plugin/models.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:
infrahub_sdk/pytest_plugin/loader.pyinfrahub_sdk/pytest_plugin/models.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 : Define test `kind` values according to resource type and test level (smoke, unit, integration) as specified in the test kinds table
Applied to files:
infrahub_sdk/pytest_plugin/models.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.14)
- GitHub Check: unit-tests (3.10)
- GitHub Check: unit-tests (3.12)
- GitHub Check: unit-tests (3.11)
- GitHub Check: unit-tests (3.13)
- GitHub Check: integration-tests-latest-infrahub
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
infrahub_sdk/pytest_plugin/models.py (1)
101-107: LGTM: integration tests defaultinput=Nonematches “input comes from live GraphQL”.The override makes
InfrahubIntegrationTestskip input file resolution (sinceupdate_paths()now guardsself.input is not None).infrahub_sdk/pytest_plugin/loader.py (1)
103-110: LGTM: empty YAML now safely produces no collected items.
if not raw ...: returnavoids constructingInfrahubTestFileV1(**raw)whenyaml.safe_load()returnsNone(common for empty files).
Summary by CodeRabbit
Release Notes
Bug Fixes
New Features
✏️ Tip: You can customize this high-level summary in your review settings.