Skip to content

Conversation

@gmazoyer
Copy link
Contributor

@gmazoyer gmazoyer commented Feb 6, 2026

These tests won't pass yet, as we need the upcoming Infrahub 1.8 to run them, hence marking them as XFAIL for now.

Summary by CodeRabbit

  • Tests
    • Added comprehensive integration tests and fixtures for FileObject lifecycle: uploads (bytes & path), updates and upserts, downloads (memory & disk), metadata checks (name, size, checksum, storage id), schema-loading fixtures, node-creation helpers, and relationship-based queries — available for both asynchronous and synchronous test flows.

These tests won't pass yet, as we need the upcoming Infrahub 1.8 to run
them, hence marking them as XFAIL for now.
@coderabbitai
Copy link

coderabbitai bot commented Feb 6, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds infrahub_sdk/testing/schemas/file_object.py, which defines NodeSchema fixtures FileContract and Circuit, builds a SchemaRoot, and provides async/sync fixtures to load the schema and create/persist Circuit nodes. Adds tests/integration/test_file_object.py with async (TestFileObjectAsync) and sync (TestFileObjectSync) integration tests covering FileObject creation from bytes and path, metadata verification, updates with new files, upsert behavior, downloads to memory and disk, attribute-only updates, and querying contracts via the circuit relationship.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 63.16% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add integration tests for file objects' directly and concisely summarizes the main change: adding new integration test files for file object functionality.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Tip

We've launched Issue Planner and it is currently in beta. Please try it out and share your feedback on Discord!


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Feb 6, 2026

Deploying infrahub-sdk-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5d9bad8
Status: ✅  Deploy successful!
Preview URL: https://b1a2f68d.infrahub-sdk-python.pages.dev
Branch Preview URL: https://gma-20260206-ihs193-integrat.infrahub-sdk-python.pages.dev

View logs

@codecov
Copy link

codecov bot commented Feb 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@                 Coverage Diff                  @@
##           infrahub-develop     #802      +/-   ##
====================================================
+ Coverage             80.33%   80.50%   +0.17%     
====================================================
  Files                   115      118       +3     
  Lines                  9865    10249     +384     
  Branches               1504     1547      +43     
====================================================
+ Hits                   7925     8251     +326     
- Misses                 1419     1462      +43     
- Partials                521      536      +15     
Flag Coverage Δ
integration-tests 40.35% <100.00%> (-1.10%) ⬇️
python-3.10 51.72% <0.00%> (+0.33%) ⬆️
python-3.11 51.72% <0.00%> (+0.33%) ⬆️
python-3.12 51.72% <0.00%> (+0.33%) ⬆️
python-3.13 51.74% <0.00%> (+0.37%) ⬆️
python-3.14 53.43% <0.00%> (+0.40%) ⬆️
python-filler-3.12 23.96% <0.00%> (-0.12%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
infrahub_sdk/testing/schemas/file_object.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gmazoyer gmazoyer marked this pull request as ready for review February 6, 2026 14:19
@gmazoyer gmazoyer requested a review from a team as a code owner February 6, 2026 14:19
@gmazoyer gmazoyer force-pushed the gma-20260206-ihs193-integration-tests branch 2 times, most recently from 327143c to 7fc5250 Compare February 9, 2026 09:33
) -> InfrahubNodeSync:
obj = client_sync.create(kind=TESTING_CIRCUIT, circuit_id="CIRCUIT-SYNC-001", bandwidth=2000)
obj.save()
return obj
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it's valuable to have this included in the SDK or if it should just be a fixture within the integration tests directory. Not a bit issue for me but do you think we'll use it outside of the SDK tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it kinda highlight that the line I drew between SDK integration tests and backend integration tests is a bit blurry to me. I think I will remove these completely from the SDK as they don't seem very well suited here and we have something similar in the backend integration tests anyway.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@infrahub_sdk/testing/schemas/file_object.py`:
- Around line 70-82: The async fixture load_file_object_schema and the sync
fixture load_file_object_schema_sync call client.schema.load but don't check the
returned SchemaLoadResponse for errors; update both fixtures
(load_file_object_schema and load_file_object_schema_sync) to capture the
response (e.g., resp = await client.schema.load(...) and resp =
client_sync.schema.load(...)), inspect resp.errors, and if present raise an
exception (use raise GraphQLError(errors=[resp.errors]) as in the established
pattern) so schema load failures surface immediately.
🧹 Nitpick comments (1)
tests/integration/test_file_object.py (1)

36-73: Implicit test ordering dependency — tests must run in definition order.

test_create_file_object_with_upload creates contracts linked to circuit_main that test_query_contracts_through_circuit_relationship later counts via len(contract_refs) >= 2. If test_query_contracts_through_circuit_relationship runs before the other tests, there may be fewer than 2 contracts linked to the circuit. This works only because pytest runs tests in definition order by default, but it's fragile.

The >= assertion at line 184 mitigates this somewhat, but it's worth noting the implicit ordering dependency.

@gmazoyer gmazoyer merged commit 0abb16d into infrahub-develop Feb 10, 2026
21 checks passed
@gmazoyer gmazoyer deleted the gma-20260206-ihs193-integration-tests branch February 10, 2026 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants