IFC-2041: Update test schema for integration tests #736
Conversation
WalkthroughThe PR updates the pre-commit configuration and refactors the testing schema fixtures. The Ruff linter is upgraded from v0.11.9 to v0.14.10 with automatic fix support enabled, and a ruff-format hook is added. In the car\_person testing schema, the SchemaCarPerson class is restructured by extracting fixtures into SchemaCarPersonFixtures, with a new SchemaCarPerson class inheriting from it. The BUILTIN\_TAG constant is renamed to TESTING\_TAG and redefined. All tag-related references are updated accordingly, and a new testing\_tag\_base fixture is introduced for the Tag schema definition. Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## develop #736 +/- ##
========================================
Coverage 76.84% 76.84%
========================================
Files 113 113
Lines 9767 9771 +4
Branches 1497 1497
========================================
+ Hits 7505 7509 +4
Misses 1757 1757
Partials 505 505
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: 0
🧹 Nitpick comments (1)
infrahub_sdk/testing/schemas/car_person.py (1)
135-148: Use the NAMESPACE constant for consistency.The
testing_tag_basefixture hardcodesnamespace="Testing"on line 139, while other fixtures in the file use theNAMESPACEconstant. For consistency and maintainability, use the constant instead.🔎 Proposed fix
def testing_tag_base(self) -> NodeSchema: return NodeSchema( name="Tag", - namespace="Testing", + namespace=NAMESPACE, include_in_menu=True, label="Tag",
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.pre-commit-config.yamlinfrahub_sdk/testing/schemas/car_person.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/testing/schemas/car_person.py
infrahub_sdk/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Follow async/sync dual pattern for new features in the Python SDK
Files:
infrahub_sdk/testing/schemas/car_person.py
🧠 Learnings (1)
📚 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: Run markdownlint before committing markdown changes
Applied to files:
.pre-commit-config.yaml
⏰ 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.13)
- GitHub Check: unit-tests (3.12)
- GitHub Check: unit-tests (3.10)
- GitHub Check: unit-tests (3.11)
- GitHub Check: unit-tests (3.14)
- GitHub Check: integration-tests-latest-infrahub
- GitHub Check: Cloudflare Pages
🔇 Additional comments (8)
.pre-commit-config.yaml (2)
20-22: LGTM! Good additions for code quality.Adding
--fixto automatically fix issues and including theruff-formathook are excellent improvements for maintaining consistent code quality and formatting across the repository.
16-16: This is a new configuration file, not an upgrade.The
.pre-commit-config.yamlfile is being added for the first time to the repository with Ruff v0.14.10. There is no previous version to upgrade from in this codebase. The original concern about compatibility with a version upgrade does not apply since this is an initial configuration, not a migration.Likely an incorrect or invalid review comment.
infrahub_sdk/testing/schemas/car_person.py (6)
21-21: LGTM! Namespace change aligns with PR objectives.The change from
BuiltinTagtoTestingTagcorrectly addresses the restricted namespace issue mentioned in the PR objectives.
43-43: Good refactoring to separate fixtures from test utilities.Extracting fixtures into
SchemaCarPersonFixturesand creating a separateSchemaCarPersonsubclass improves organization and maintainability.
150-160: LGTM! Properly integrates the new Tag schema.The
testing_tag_basefixture is correctly added as a dependency and included in the SchemaRoot nodes list.
97-97: LGTM! Peer reference correctly updated.The relationship peer reference has been properly updated to use
TESTING_TAGinstead of the oldBUILTIN_TAG.
219-234: LGTM! Tag fixtures correctly updated.All tag creation fixtures (
tag_blue,tag_red,tag_green) have been properly updated to useTESTING_TAGinstead of the oldBUILTIN_TAG.
237-273: LGTM! Proper class structure with complete type hints.The new
SchemaCarPersonclass properly inherits fromSchemaCarPersonFixtures, maintaining backwards compatibility while extending functionality. All methods include proper type hints as required by the coding guidelines.
ajtmccarty
left a comment
ajtmccarty
left a comment
There was a problem hiding this comment.
I don't know exactly how the sdk integration tests run, but it seems like this change shouldn't be necessary. I would expect the BuiltinTag schema to be available on the local infrahub server that these tests are interacting with
This commit opsmill/infrahub@36f088d eliminates the need for this PR Closing this PR |
Since
Builtinhas been added restricted namespaces here opsmill/infrahub#8032, integrations tests from infrahub would need this update to passSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.