Phase out subscription=False when generating GraphQL schema #7360
Conversation
github-actions
bot
added
the
group/backend
WalkthroughThis pull request systematically refactors GraphQL test infrastructure across the backend test suite. The core change updates the Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
CodSpeed Performance ReportMerging #7360 will not alter performanceComparing Summary
|
ogenstad
force-pushed
the
pog-remove-subscription-false
branch
3 times, most recently
from
9bf76dd to
fd2d322
Compare
ogenstad
force-pushed
the
pog-remove-subscription-false
branch
from
fd2d322 to
53935b8
Compare
ogenstad
force-pushed
the
pog-remove-subscription-false
branch
from
4393b58 to
18aa272
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
backend/tests/unit/graphql/diff/test_diff_update_mutation.py (2)
204-205: Add type-safety assertion forresult.data.Line 205 accesses
result.datawithout first asserting it's not None, which is inconsistent with the pattern established elsewhere in this file (lines 71-72, 107-108). This creates a typing issue sinceresult.datais Optional.Apply this diff to add the missing assertion:
assert result.errors is None + assert result.data assert result.data["DiffUpdate"]["ok"] is True
233-235: Add type-safety assertion forresult.data.Lines 234-235 access
result.datawithout first asserting it's not None, inconsistent with the pattern at lines 71-72 and 107-108. This creates a typing issue.Apply this diff to add the missing assertion:
assert result.errors is None + assert result.data assert result.data["DiffUpdate"]["ok"] is True assert result.data["DiffUpdate"]["task"]["id"] is not Nonebackend/tests/benchmark/test_graphql_query.py (3)
59-59: Add explicit return type annotation.The test function is missing a
-> Nonereturn type annotation, which is inconsistent with the typing fixes applied throughout this PR and the coding guidelines requiring type hints for all function return values.Apply this diff:
-def test_query_one_model(exec_async, aio_benchmark, db: InfrahubDatabase, default_branch: Branch, dataset04): +def test_query_one_model(exec_async, aio_benchmark, db: InfrahubDatabase, default_branch: Branch, dataset04) -> None:
99-99: Add explicit return type annotation.The test function is missing a
-> Nonereturn type annotation.Apply this diff:
-def test_query_rel_many(exec_async, aio_benchmark, db: InfrahubDatabase, default_branch: Branch, dataset04): +def test_query_rel_many(exec_async, aio_benchmark, db: InfrahubDatabase, default_branch: Branch, dataset04) -> None:
147-147: Add explicit return type annotation.The test function is missing a
-> Nonereturn type annotation.Apply this diff:
-def test_query_rel_one(exec_async, aio_benchmark, db: InfrahubDatabase, default_branch: Branch, dataset04): +def test_query_rel_one(exec_async, aio_benchmark, db: InfrahubDatabase, default_branch: Branch, dataset04) -> None:backend/tests/helpers/graphql.py (1)
72-105: Keepgraphql_*helpers compatible with string branch identifiersWe still have callers that hand
branch="main"(or any branch name string) into these helpers. Before this change that worked because we deferred toprepare_graphql_params, which acceptsBranch | str. By callingbranch.update_schema_hash()before normalising the input we now blow up withAttributeError: 'str' object has no attribute 'update_schema_hash', so those tests regress immediately.Please normalise the incoming value to a
Branchinstance before invokingupdate_schema_hash()(same fix is needed in both helpers). For example:- branch = branch or await Branch.get_by_name(name="main", db=db) - branch.update_schema_hash() + if isinstance(branch, str): + branch = await Branch.get_by_name(name=branch, db=db) + elif branch is None: + branch = await Branch.get_by_name(name="main", db=db) + + branch.update_schema_hash()This preserves the existing calling contract while still ensuring the schema hash is refreshed.
backend/tests/unit/graphql/test_mutation_create.py (1)
1684-1723: Missing fixture for GraphQL enum setting toggle.The test
test_create_enum_when_enums_off_fails(lines 1684-1723) setsconfig.SETTINGS.experimental_features.graphql_enums = Falsebut doesn't use thereset_graphql_schema_between_testsfixture, unlike the similar testtest_create_string_when_enums_on_fails(lines 1725-1764) which does use the fixture.When toggling the enum setting, the GraphQL schema structure changes completely, so the cache should be cleared to avoid test interference. The fixture is designed for exactly this scenario.
Apply this diff to add the missing fixture:
async def test_create_enum_when_enums_off_fails( db: InfrahubDatabase, default_branch, person_john_main, car_person_schema, + reset_graphql_schema_between_tests, ):
🧹 Nitpick comments (14)
backend/tests/unit/graphql/test_mutation_create_jinja2_attributes.py (1)
23-24: Consider consolidating redundantupdate_schema_hash()calls.In tests 1 and 2,
branch.update_schema_hash()is called immediately after schema load (lines 23, 69) and again beforeprepare_graphql_params()(lines 48, 98) without any schema modifications in between. The second call could be omitted.Similarly, in test 3, multiple calls at lines 139, 172, and 208 may be excessive since the schema doesn't change during test execution.
That said, the PR objectives acknowledge this tradeoff, noting it's still faster than full schema regenerations.
Example for test 1:
registry.schema.register_schema(schema=schema, branch=branch.name) branch.update_schema_hash() await branch.save(db=db) await load_schema(db, branch_name=branch.name, schema=SchemaRoot(nodes=[CHILD, THING])) + branch.update_schema_hash() fred = await Node.init(schema=TestKind.CHILD, db=db, branch=branch) await fred.new(db=db, name="Fred", height=110) await fred.save(db=db) query = """ ... """ - branch.update_schema_hash() gql_params = await prepare_graphql_params(db=db, branch=branch)Also applies to: 48-49
backend/tests/benchmark/test_graphql_query.py (1)
196-230: Consider removing commented test or using proper pytest markers.The commented-out test has been kept in sync with the codebase changes but remains disabled. Instead of keeping it commented, consider either:
- Removing it entirely if it won't be re-enabled
- Using
@pytest.mark.skip(reason="Inconsistent results")or@pytest.mark.xfaildecorators to properly document the disabled statebackend/tests/unit/graphql/test_graphql_utils.py (1)
55-56: Add return annotation and align fixture typing/naming in the second test.Mirror the first test’s typing and underscore convention; also add the explicit
-> None.-async def test_schema_models_generics( - db: InfrahubDatabase, default_branch: Branch, car_person_schema_generics, query_02: str -): +async def test_schema_models_generics( + db: InfrahubDatabase, default_branch: Branch, _car_person_schema_generics: SchemaRoot, query_02: str +) -> None:backend/tests/unit/graphql/mutations/test_mutation_context.py (2)
53-55: DRY the GraphQL setup (helper wrapping update+prepare).The pattern branch.update_schema_hash() followed by prepare_graphql_params(...) is repeated. Wrap it in a tiny helper to reduce duplication and missed updates.
Example change at these call sites:
- default_branch.update_schema_hash() - gql_params = await prepare_graphql_params(db=db, branch=default_branch, account_session=session_first_account) + gql_params = await build_params(db, default_branch, account_session=session_first_account)Add once (e.g., tests/helpers/graphql.py):
# tests/helpers/graphql.py from infrahub.graphql.initialization import prepare_graphql_params from infrahub.core.branch import Branch from infrahub.database import InfrahubDatabase async def build_params(db: InfrahubDatabase, branch: Branch, **kwargs): branch.update_schema_hash() return await prepare_graphql_params(db=db, branch=branch, **kwargs)Then import and use build_params across tests.
Also applies to: 100-103, 142-147
1-22: Type-only import cleanup.AccountSession is imported at runtime and again under TYPE_CHECKING while future annotations are enabled. Keep only the TYPE_CHECKING import.
-from infrahub.auth import AccountSessionNo behavior change; faster imports.
backend/tests/unit/graphql/test_mutation_update.py (2)
42-44: Centralize GraphQL param creation to cut repetition and flakiness risk.The same update_schema_hash() + prepare_graphql_params(...) sequence appears throughout. Introduce a shared helper and replace the two lines with one call.
Sample change:
- branch.update_schema_hash() - gql_params = await prepare_graphql_params(db=db, branch=branch) + gql_params = await build_params(db, branch)Add once (e.g., tests/helpers/graphql.py):
async def build_params(db: InfrahubDatabase, branch: Branch, **kwargs): branch.update_schema_hash() return await prepare_graphql_params(db=db, branch=branch, **kwargs)Apply similarly where service/account_session is passed:
- branch.update_schema_hash() - gql_params = await prepare_graphql_params(db=db, branch=branch, service=service, account_session=session_first_account) + gql_params = await build_params(db, branch, service=service, account_session=session_first_account)This reduces boilerplate and makes schema-hash bumps uniform across tests.
Also applies to: 72-74, 121-123, 149-151, 185-187
26-41: Prefer GraphQL variables over string interpolation for IDs.Interpolate IDs via variables to avoid quoting/escaping pitfalls and improve readability.
Example:
- query = (""" ... TestPersonUpdate(data: {id: "%s", name: { value: "Jim"}}) { ... } """ % person_john_main.id) + query = """ ... TestPersonUpdate(data: {id: $id, name: { value: "Jim"}}) { ... } """ + # variables={"id": person_john_main.id}Applies to similar mutation/query strings in this file.
backend/tests/functional/ipam/test_ipam_utilization.py (2)
205-206: Extract a build_params helper to remove repeated update+prepare calls.Replace pairs of:
default_branch.update_schema_hash() gql_params = await prepare_graphql_params(db=db, branch=default_branch)with:
gql_params = await build_params(db, default_branch)And similarly for branch2. Define build_params once (e.g., tests/helpers/graphql.py) as:
async def build_params(db: InfrahubDatabase, branch: Branch, **kwargs): branch.update_schema_hash() return await prepare_graphql_params(db=db, branch=branch, **kwargs)Less duplication, clearer intent.
Also applies to: 237-239, 267-269, 351-353, 391-393, 425-427, 465-467, 533-535, 573-575, 613-615, 653-655
345-348: Good move adding explicit -> None.Please consider adding -> None to the remaining async tests in this class for mypy consistency.
Also applies to: 420-422, 529-530, 609-610
backend/tests/unit/graphql/test_graphql_query.py (2)
31-33: Replace repeated update+prepare with a single helper call.Apply the same build_params(db, branch, **kwargs) helper used elsewhere to eliminate boilerplate and keep schema-hash bumps consistent. One-line replacement at each listed site clarifies intent and trims test time.
Also applies to: 68-70, 109-111, 150-152, 204-206, 272-274, 357-359, 430-432, 536-538, 642-644, 737-739, 789-791, 820-822, 990-992, 1066-1068, 1155-1157, 1232-1234, 1570-1571, 1606-1607, 1669-1671, 1718-1720, 1758-1760, 1788-1790, 1830-1832, 1851-1853, 1905-1907, 2000-2001, 2029-2031, 2133-2135, 2338-2340, 2455-2457, 2518-2520, 2580-2582, 2630-2632, 2739-2741, 2801-2803, 2833-2835, 2873-2875, 2933-2935, 3001-3003, 3053-3055, 3109-3111, 3164-3166, 3217-3219, 3267-3269, 3387-3389, 3447-3449
175-177: Standardize schema access for clarity.Prefer get_node_schema/get_node consistently over generic get when you expect a NodeSchema. It self-documents intent and avoids accidental type mismatches.
Example:
- car = registry.schema.get(name="TestCar") + car = registry.schema.get_node_schema(name="TestCar")Optional but improves readability.
Also applies to: 314-316, 673-676, 380-382
backend/tests/unit/graphql/mutations/test_schema.py (1)
22-24: Use a common build_params helper to replace update+prepare pairs.Swap the two lines with:
- default_branch.update_schema_hash() - gql_params = await prepare_graphql_params(db=db, branch=default_branch, account_session=session_admin) + gql_params = await build_params(db, default_branch, account_session=session_admin)Add once (e.g., tests/helpers/graphql.py):
async def build_params(db: InfrahubDatabase, branch: Branch, **kwargs): branch.update_schema_hash() return await prepare_graphql_params(db=db, branch=branch, **kwargs)Cleaner and less error-prone.
Also applies to: 46-48, 70-72, 94-96, 118-120, 146-148, 174-176
backend/tests/unit/graphql/test_mutation_create.py (2)
23-23: Inconsistent type annotations across test functions.Only some test functions have been updated with type hints (e.g., lines 23, 56-58, 230-232), while most others remain without annotations (e.g., lines 80, 135, 165, 198, 272, 325, etc.). The coding guidelines require type hints for all function parameters and return values.
Consider updating all test function signatures in this file for consistency:
async def test_create_with_id( db: InfrahubDatabase, default_branch: Branch, car_person_schema: None ) -> None:Also applies to: 56-58, 80-80, 135-135, 165-165, 198-198, 230-232, 272-272, 325-325, 390-390, 456-458, 541-541, 579-581, 612-614, 654-656, 703-703, 748-750, 825-827, 880-886, 1064-1064, 1093-1093, 1129-1129, 1172-1172, 1209-1209, 1285-1285, 1315-1315, 1338-1340, 1526-1528, 1569-1571, 1634-1642, 1684-1689, 1725-1731
34-35: update_schema_hash() called repeatedly without schema changes.Throughout the file,
update_schema_hash()is called before eachprepare_graphql_params(), including multiple times within the same test (e.g., lines 247 and 259, lines 406 and 440, lines 478 and 519). When the schema hasn't changed between calls, these subsequent calls are unnecessary.For example:
# First call needed default_branch.update_schema_hash() gql_params = await prepare_graphql_params(db=db, branch=default_branch) # ... mutation ... # Second call unnecessary if schema unchanged default_branch.update_schema_hash() # Schema hasn't changed gql_params = await prepare_graphql_params(db=db, branch=default_branch)Based on PR objectives, this pattern is acknowledged as potentially excessive but accepted as a trade-off for avoiding full schema regenerations. If optimization is desired in the future, consider calling
update_schema_hash()only once per test unless the schema actually changes.Also applies to: 66-67, 95-96, 120-121, 150-151, 183-184, 215-216, 247-248, 259-259, 293-294, 344-345, 406-407, 440-441, 478-479, 519-520, 563-564, 597-598, 634-635, 681-682, 729-730, 785-786, 857-858, 925-926, 939-940, 968-969, 982-983
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (44)
backend/tests/benchmark/test_graphql_query.py(4 hunks)backend/tests/functional/ipam/test_ipam_rebase_reconcile.py(5 hunks)backend/tests/functional/ipam/test_ipam_utilization.py(12 hunks)backend/tests/helpers/graphql.py(2 hunks)backend/tests/integration/profiles/test_profile_lifecycle.py(10 hunks)backend/tests/unit/conftest.py(3 hunks)backend/tests/unit/graphql/diff/test_diff_tree_query.py(16 hunks)backend/tests/unit/graphql/diff/test_diff_update_mutation.py(8 hunks)backend/tests/unit/graphql/mutations/test_action.py(4 hunks)backend/tests/unit/graphql/mutations/test_branch.py(8 hunks)backend/tests/unit/graphql/mutations/test_group_event_collection.py(3 hunks)backend/tests/unit/graphql/mutations/test_ipam.py(29 hunks)backend/tests/unit/graphql/mutations/test_mutation_context.py(3 hunks)backend/tests/unit/graphql/mutations/test_proposed_change.py(2 hunks)backend/tests/unit/graphql/mutations/test_resource_manager.py(13 hunks)backend/tests/unit/graphql/mutations/test_schema.py(13 hunks)backend/tests/unit/graphql/mutations/test_update_generic.py(3 hunks)backend/tests/unit/graphql/mutations/test_webhook.py(9 hunks)backend/tests/unit/graphql/profiles/test_mutation_create.py(1 hunks)backend/tests/unit/graphql/profiles/test_query.py(13 hunks)backend/tests/unit/graphql/queries/test_branch.py(4 hunks)backend/tests/unit/graphql/queries/test_display_label.py(13 hunks)backend/tests/unit/graphql/queries/test_event.py(1 hunks)backend/tests/unit/graphql/queries/test_ipam.py(8 hunks)backend/tests/unit/graphql/queries/test_relationship.py(1 hunks)backend/tests/unit/graphql/queries/test_resource_pool.py(15 hunks)backend/tests/unit/graphql/queries/test_search.py(8 hunks)backend/tests/unit/graphql/queries/test_task.py(1 hunks)backend/tests/unit/graphql/test_core_account.py(4 hunks)backend/tests/unit/graphql/test_graphql_query.py(123 hunks)backend/tests/unit/graphql/test_graphql_utils.py(2 hunks)backend/tests/unit/graphql/test_mutation_artifact_definition.py(1 hunks)backend/tests/unit/graphql/test_mutation_create.py(59 hunks)backend/tests/unit/graphql/test_mutation_create_jinja2_attributes.py(5 hunks)backend/tests/unit/graphql/test_mutation_delete.py(5 hunks)backend/tests/unit/graphql/test_mutation_generator.py(1 hunks)backend/tests/unit/graphql/test_mutation_graphqlquery.py(3 hunks)backend/tests/unit/graphql/test_mutation_relationship.py(25 hunks)backend/tests/unit/graphql/test_mutation_update.py(62 hunks)backend/tests/unit/graphql/test_mutation_upsert.py(18 hunks)backend/tests/unit/graphql/test_parser.py(4 hunks)backend/tests/unit/graphql/test_query.py(3 hunks)backend/tests/unit/graphql/test_query_analyzer.py(6 hunks)backend/tests/unit/graphql/test_schema.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
backend/tests/**/*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place backend tests in
backend/tests/
Files:
backend/tests/unit/graphql/queries/test_resource_pool.pybackend/tests/unit/graphql/test_mutation_graphqlquery.pybackend/tests/unit/graphql/mutations/test_group_event_collection.pybackend/tests/unit/graphql/mutations/test_webhook.pybackend/tests/integration/profiles/test_profile_lifecycle.pybackend/tests/unit/graphql/profiles/test_mutation_create.pybackend/tests/unit/graphql/mutations/test_branch.pybackend/tests/unit/graphql/queries/test_event.pybackend/tests/unit/graphql/test_query.pybackend/tests/functional/ipam/test_ipam_rebase_reconcile.pybackend/tests/unit/graphql/mutations/test_action.pybackend/tests/unit/graphql/test_mutation_create_jinja2_attributes.pybackend/tests/unit/conftest.pybackend/tests/unit/graphql/queries/test_task.pybackend/tests/unit/graphql/mutations/test_update_generic.pybackend/tests/unit/graphql/diff/test_diff_tree_query.pybackend/tests/unit/graphql/test_schema.pybackend/tests/unit/graphql/queries/test_ipam.pybackend/tests/unit/graphql/mutations/test_proposed_change.pybackend/tests/unit/graphql/test_mutation_update.pybackend/tests/unit/graphql/test_query_analyzer.pybackend/tests/unit/graphql/test_mutation_delete.pybackend/tests/unit/graphql/test_graphql_utils.pybackend/tests/unit/graphql/test_parser.pybackend/tests/unit/graphql/queries/test_relationship.pybackend/tests/unit/graphql/mutations/test_ipam.pybackend/tests/benchmark/test_graphql_query.pybackend/tests/unit/graphql/test_mutation_artifact_definition.pybackend/tests/helpers/graphql.pybackend/tests/unit/graphql/mutations/test_mutation_context.pybackend/tests/unit/graphql/profiles/test_query.pybackend/tests/unit/graphql/queries/test_display_label.pybackend/tests/unit/graphql/mutations/test_resource_manager.pybackend/tests/unit/graphql/diff/test_diff_update_mutation.pybackend/tests/unit/graphql/test_core_account.pybackend/tests/unit/graphql/queries/test_search.pybackend/tests/unit/graphql/mutations/test_schema.pybackend/tests/unit/graphql/test_mutation_create.pybackend/tests/unit/graphql/test_mutation_upsert.pybackend/tests/functional/ipam/test_ipam_utilization.pybackend/tests/unit/graphql/test_mutation_relationship.pybackend/tests/unit/graphql/test_graphql_query.pybackend/tests/unit/graphql/queries/test_branch.pybackend/tests/unit/graphql/test_mutation_generator.py
backend/**/*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run backend tests with
pytestor viainvoketasks
Files:
backend/tests/unit/graphql/queries/test_resource_pool.pybackend/tests/unit/graphql/test_mutation_graphqlquery.pybackend/tests/unit/graphql/mutations/test_group_event_collection.pybackend/tests/unit/graphql/mutations/test_webhook.pybackend/tests/integration/profiles/test_profile_lifecycle.pybackend/tests/unit/graphql/profiles/test_mutation_create.pybackend/tests/unit/graphql/mutations/test_branch.pybackend/tests/unit/graphql/queries/test_event.pybackend/tests/unit/graphql/test_query.pybackend/tests/functional/ipam/test_ipam_rebase_reconcile.pybackend/tests/unit/graphql/mutations/test_action.pybackend/tests/unit/graphql/test_mutation_create_jinja2_attributes.pybackend/tests/unit/conftest.pybackend/tests/unit/graphql/queries/test_task.pybackend/tests/unit/graphql/mutations/test_update_generic.pybackend/tests/unit/graphql/diff/test_diff_tree_query.pybackend/tests/unit/graphql/test_schema.pybackend/tests/unit/graphql/queries/test_ipam.pybackend/tests/unit/graphql/mutations/test_proposed_change.pybackend/tests/unit/graphql/test_mutation_update.pybackend/tests/unit/graphql/test_query_analyzer.pybackend/tests/unit/graphql/test_mutation_delete.pybackend/tests/unit/graphql/test_graphql_utils.pybackend/tests/unit/graphql/test_parser.pybackend/tests/unit/graphql/queries/test_relationship.pybackend/tests/unit/graphql/mutations/test_ipam.pybackend/tests/benchmark/test_graphql_query.pybackend/tests/unit/graphql/test_mutation_artifact_definition.pybackend/tests/helpers/graphql.pybackend/tests/unit/graphql/mutations/test_mutation_context.pybackend/tests/unit/graphql/profiles/test_query.pybackend/tests/unit/graphql/queries/test_display_label.pybackend/tests/unit/graphql/mutations/test_resource_manager.pybackend/tests/unit/graphql/diff/test_diff_update_mutation.pybackend/tests/unit/graphql/test_core_account.pybackend/tests/unit/graphql/queries/test_search.pybackend/tests/unit/graphql/mutations/test_schema.pybackend/tests/unit/graphql/test_mutation_create.pybackend/tests/unit/graphql/test_mutation_upsert.pybackend/tests/functional/ipam/test_ipam_utilization.pybackend/tests/unit/graphql/test_mutation_relationship.pybackend/tests/unit/graphql/test_graphql_query.pybackend/tests/unit/graphql/queries/test_branch.pybackend/tests/unit/graphql/test_mutation_generator.py
**/*.py
📄 CodeRabbit inference engine (.github/instructions/python-docstring.instructions.md)
**/*.py: Use triple double quotes (""") for all Python docstrings
Write docstrings in Google-style format
Include a brief one-line description at the top of each docstring
Add a detailed description section when additional context is needed
Document function/method parameters under an Args/Parameters section without typing information
Include a Returns section describing the return value
Include a Raises section listing possible exceptions
Provide an Examples section demonstrating usage when helpfulUse ruff and mypy to validate and lint Python files
**/*.py: Use type hints for all Python function parameters and return values
Prefer asynchronous code in Python when feasible
Define asynchronous Python functions withasync def
Useawaitfor asynchronous calls in Python
Use Pydantic models instead of standard dataclasses
Use ruff and mypy for linting and type checking
**/*.py: Use type hints for all Python function parameters and return values
Use async/await whenever possible in Python code
Define asynchronous functions withasync def
Await asynchronous calls withawait
Use Pydantic models instead of standard dataclasses for data modeling
Use triple quotes (""") for all Python docstrings
Write docstrings in Google-style format
Include docstring sections when applicable: one-line summary, optional details, Args (without types), Returns, Raises, Examples
Validate and lint Python with ruff and mypy
Files:
backend/tests/unit/graphql/queries/test_resource_pool.pybackend/tests/unit/graphql/test_mutation_graphqlquery.pybackend/tests/unit/graphql/mutations/test_group_event_collection.pybackend/tests/unit/graphql/mutations/test_webhook.pybackend/tests/integration/profiles/test_profile_lifecycle.pybackend/tests/unit/graphql/profiles/test_mutation_create.pybackend/tests/unit/graphql/mutations/test_branch.pybackend/tests/unit/graphql/queries/test_event.pybackend/tests/unit/graphql/test_query.pybackend/tests/functional/ipam/test_ipam_rebase_reconcile.pybackend/tests/unit/graphql/mutations/test_action.pybackend/tests/unit/graphql/test_mutation_create_jinja2_attributes.pybackend/tests/unit/conftest.pybackend/tests/unit/graphql/queries/test_task.pybackend/tests/unit/graphql/mutations/test_update_generic.pybackend/tests/unit/graphql/diff/test_diff_tree_query.pybackend/tests/unit/graphql/test_schema.pybackend/tests/unit/graphql/queries/test_ipam.pybackend/tests/unit/graphql/mutations/test_proposed_change.pybackend/tests/unit/graphql/test_mutation_update.pybackend/tests/unit/graphql/test_query_analyzer.pybackend/tests/unit/graphql/test_mutation_delete.pybackend/tests/unit/graphql/test_graphql_utils.pybackend/tests/unit/graphql/test_parser.pybackend/tests/unit/graphql/queries/test_relationship.pybackend/tests/unit/graphql/mutations/test_ipam.pybackend/tests/benchmark/test_graphql_query.pybackend/tests/unit/graphql/test_mutation_artifact_definition.pybackend/tests/helpers/graphql.pybackend/tests/unit/graphql/mutations/test_mutation_context.pybackend/tests/unit/graphql/profiles/test_query.pybackend/tests/unit/graphql/queries/test_display_label.pybackend/tests/unit/graphql/mutations/test_resource_manager.pybackend/tests/unit/graphql/diff/test_diff_update_mutation.pybackend/tests/unit/graphql/test_core_account.pybackend/tests/unit/graphql/queries/test_search.pybackend/tests/unit/graphql/mutations/test_schema.pybackend/tests/unit/graphql/test_mutation_create.pybackend/tests/unit/graphql/test_mutation_upsert.pybackend/tests/functional/ipam/test_ipam_utilization.pybackend/tests/unit/graphql/test_mutation_relationship.pybackend/tests/unit/graphql/test_graphql_query.pybackend/tests/unit/graphql/queries/test_branch.pybackend/tests/unit/graphql/test_mutation_generator.py
🧬 Code graph analysis (44)
backend/tests/unit/graphql/queries/test_resource_pool.py (3)
backend/tests/benchmark/test_graphql_query.py (1)
default_branch(36-40)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/test_mutation_graphqlquery.py (1)
backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/mutations/test_group_event_collection.py (4)
backend/tests/benchmark/test_graphql_query.py (1)
default_branch(36-40)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)backend/tests/unit/conftest.py (1)
session_first_account(2714-2716)
backend/tests/unit/graphql/mutations/test_webhook.py (3)
backend/tests/benchmark/test_graphql_query.py (1)
default_branch(36-40)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/integration/profiles/test_profile_lifecycle.py (3)
backend/tests/benchmark/test_graphql_query.py (1)
default_branch(36-40)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/profiles/test_mutation_create.py (3)
backend/tests/benchmark/test_graphql_query.py (1)
default_branch(36-40)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/mutations/test_branch.py (2)
backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/queries/test_event.py (2)
backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/test_query.py (3)
backend/tests/benchmark/test_graphql_query.py (1)
default_branch(36-40)backend/tests/conftest.py (1)
register_core_models_schema(217-218)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)
backend/tests/functional/ipam/test_ipam_rebase_reconcile.py (2)
backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)
backend/tests/unit/graphql/mutations/test_action.py (3)
backend/tests/benchmark/test_graphql_query.py (1)
default_branch(36-40)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/test_mutation_create_jinja2_attributes.py (2)
backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/conftest.py (1)
backend/infrahub/graphql/registry.py (1)
clear_cache(57-68)
backend/tests/unit/graphql/queries/test_task.py (2)
backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/mutations/test_update_generic.py (3)
backend/tests/unit/graphql/queries/test_display_label.py (1)
test_display_label_generic(165-207)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/diff/test_diff_tree_query.py (3)
backend/tests/benchmark/test_graphql_query.py (1)
default_branch(36-40)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/test_schema.py (4)
backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)backend/tests/benchmark/test_graphql_query.py (1)
default_branch(36-40)backend/tests/helpers/test_app.py (1)
default_branch(54-61)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)
backend/tests/unit/graphql/queries/test_ipam.py (3)
backend/tests/benchmark/test_graphql_query.py (1)
default_branch(36-40)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/mutations/test_proposed_change.py (4)
backend/tests/benchmark/test_graphql_query.py (1)
default_branch(36-40)backend/tests/conftest.py (1)
default_branch(161-162)backend/tests/helpers/test_app.py (1)
default_branch(54-61)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)
backend/tests/unit/graphql/test_mutation_update.py (4)
backend/tests/conftest.py (3)
branch(1239-1243)db(122-138)default_branch(161-162)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)backend/tests/helpers/graphql.py (1)
graphql(26-61)
backend/tests/unit/graphql/test_query_analyzer.py (2)
backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/test_mutation_delete.py (3)
backend/tests/benchmark/test_graphql_query.py (1)
default_branch(36-40)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/test_graphql_utils.py (3)
backend/tests/benchmark/test_graphql_query.py (1)
default_branch(36-40)backend/tests/unit/conftest.py (1)
car_person_schema_generics(1401-1410)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)
backend/tests/unit/graphql/test_parser.py (3)
backend/tests/unit/conftest.py (1)
criticality_schema(1841-1852)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/queries/test_relationship.py (2)
backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/mutations/test_ipam.py (4)
backend/tests/conftest.py (3)
default_branch(161-162)db(122-138)branch(1239-1243)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)backend/tests/helpers/graphql.py (1)
graphql(26-61)
backend/tests/benchmark/test_graphql_query.py (3)
backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/tests/benchmark/conftest.py (1)
exec_async(7-18)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/test_mutation_artifact_definition.py (1)
backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/helpers/graphql.py (1)
backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)
backend/tests/unit/graphql/mutations/test_mutation_context.py (4)
backend/tests/benchmark/test_graphql_query.py (1)
default_branch(36-40)backend/tests/conftest.py (3)
default_branch(161-162)db(122-138)branch(1239-1243)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/profiles/test_query.py (3)
backend/tests/benchmark/test_graphql_query.py (1)
default_branch(36-40)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/queries/test_display_label.py (3)
backend/tests/benchmark/test_graphql_query.py (1)
default_branch(36-40)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/mutations/test_resource_manager.py (3)
backend/tests/benchmark/test_graphql_query.py (1)
default_branch(36-40)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/diff/test_diff_update_mutation.py (3)
backend/tests/benchmark/test_graphql_query.py (1)
default_branch(36-40)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/test_core_account.py (4)
backend/tests/conftest.py (3)
db(122-138)default_branch(161-162)branch(1239-1243)backend/tests/unit/conftest.py (2)
first_account(2706-2710)session_admin(2688-2690)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/queries/test_search.py (2)
backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/mutations/test_schema.py (4)
backend/tests/conftest.py (3)
branch(1239-1243)db(122-138)default_branch(161-162)backend/tests/unit/conftest.py (2)
choices_schema(1081-1130)session_admin(2688-2690)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/test_mutation_create.py (4)
backend/tests/conftest.py (3)
db(122-138)default_branch(161-162)branch(1239-1243)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)backend/tests/unit/conftest.py (1)
reset_graphql_schema_between_tests(158-162)
backend/tests/unit/graphql/test_mutation_upsert.py (4)
backend/tests/conftest.py (3)
branch(1239-1243)db(122-138)default_branch(161-162)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)backend/tests/helpers/graphql.py (1)
graphql(26-61)
backend/tests/functional/ipam/test_ipam_utilization.py (2)
backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)
backend/tests/unit/graphql/test_mutation_relationship.py (4)
backend/tests/conftest.py (3)
branch(1239-1243)db(122-138)default_branch(161-162)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)backend/tests/unit/conftest.py (1)
session_first_account(2714-2716)
backend/tests/unit/graphql/test_graphql_query.py (4)
backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)backend/infrahub/core/schema/schema_branch.py (1)
get_node(332-337)backend/tests/helpers/graphql.py (1)
graphql(26-61)
backend/tests/unit/graphql/queries/test_branch.py (4)
backend/tests/benchmark/test_graphql_query.py (1)
default_branch(36-40)backend/tests/helpers/test_app.py (2)
default_branch(54-61)service(112-113)backend/infrahub/core/branch/models.py (1)
update_schema_hash(124-132)backend/infrahub/graphql/initialization.py (1)
prepare_graphql_params(79-125)
backend/tests/unit/graphql/test_mutation_generator.py (1)
backend/tests/benchmark/test_graphql_query.py (1)
default_branch(36-40)
| branch = await create_branch(db=db, branch_name="delete_prefix") | ||
|
|
||
| gql_params = await prepare_graphql_params(db=db, include_subscription=False, branch=registry.default_branch) | ||
| gql_params = await prepare_graphql_params(db=db, branch=registry.default_branch) |
There was a problem hiding this comment.
Missing schema hash refresh before parameter preparation.
This prepare_graphql_params call lacks a preceding default_branch.update_schema_hash() (or registry.default_branch.update_schema_hash()) call, which is inconsistent with the pattern applied throughout this PR. According to the PR objectives, refreshing the schema hash before parameter preparation helps avoid flaky tests and unnecessary schema regenerations.
Apply this change to add the missing call:
+ default_branch = registry.default_branch
+ default_branch.update_schema_hash()
gql_params = await prepare_graphql_params(db=db, branch=registry.default_branch)Or if registry.default_branch is guaranteed to be current, explicitly call:
+ registry.default_branch.update_schema_hash()
gql_params = await prepare_graphql_params(db=db, branch=registry.default_branch)🤖 Prompt for AI Agents
In backend/tests/functional/ipam/test_ipam_rebase_reconcile.py around line 92,
the call to prepare_graphql_params is missing a prior schema hash refresh;
insert a call to update_schema_hash() on the appropriate branch (e.g.,
registry.default_branch.update_schema_hash() or
default_branch.update_schema_hash()) immediately before awaiting
prepare_graphql_params(db=db, branch=registry.default_branch) so the schema hash
is refreshed consistently with the rest of the PR and avoids flaky tests.
| async def test_schema_models( | ||
| db: InfrahubDatabase, default_branch: Branch, car_person_schema_generics: None, query_01: str | ||
| ) -> None: | ||
| document = parse(query_01) | ||
| schema = generate_graphql_schema(db=db, include_subscription=False, branch=default_branch) | ||
| default_branch.update_schema_hash() | ||
| schema = generate_graphql_schema(db=db, branch=default_branch) | ||
| fields = await extract_fields(document.definitions[0].selection_set) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Unify/strengthen fixture typing; mark unused param explicitly.
Good call refreshing the schema hash before generation. Minor typing polish: annotate the fixture with its real type and prefix with “_” to signal it’s only for side‑effects.
Apply within this hunk:
-async def test_schema_models(
- db: InfrahubDatabase, default_branch: Branch, car_person_schema_generics: None, query_01: str
-) -> None:
+async def test_schema_models(
+ db: InfrahubDatabase, default_branch: Branch, _car_person_schema_generics: SchemaRoot, query_01: str
+) -> None:Add the import (adjust module path to where SchemaRoot is defined in your repo):
from infrahub.core.schema import SchemaRoot # verify exact path🏁 Script executed:
#!/bin/bash
# Locate SchemaRoot definition to import from the canonical module
rg -nP -C2 '^\s*class\s+SchemaRoot\b' --type pyLength of output: 297
The web search returns general GraphQL testing information rather than the specific test file. Let me inspect the test file directly from the repository to examine both test functions and their full context.
🏁 Script executed:
#!/bin/bash
# View the test file to understand both test functions and their context
sed -n '20,70p' backend/tests/unit/graphql/test_graphql_utils.pyLength of output: 1830
🏁 Script executed:
#!/bin/bash
# Also check for fixture definitions in this file
rg -nP 'def (car_person_schema_generics|query_01)' backend/tests/unit/graphql/test_graphql_utils.py -A2Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check conftest.py for fixture definitions
fd conftest.py backend/tests --exec rg -nP 'def (car_person_schema_generics|query_01)' {} -A2Length of output: 510
Let me inspect the full fixture definition and verify how it's used in the tests:
🏁 Script executed:
#!/bin/bash
# Get full fixture definition and check what it returns
sed -n '1401,1420p' backend/tests/unit/graphql/test_graphql_utils.pyLength of output: 0
🏁 Script executed:
#!/bin/bash
# Also search conftest.py for the fixture
fd conftest.py backend/tests --exec sed -n '1401,1430p' {}Length of output: 1971
🏁 Script executed:
#!/bin/bash
# Check the full fixture pattern more broadly
fd conftest.py backend/tests -path "*/graphql/*" --exec cat {}Length of output: 168
Fix incomplete type annotations and add missing return type in second test.
The car_person_schema_generics fixture returns SchemaRoot, not None. Both tests use it for schema registration side-effects; prefix with _ to signal this intent, and annotate with the correct type. The second test also needs an explicit -> None return type.
async def test_schema_models(
- db: InfrahubDatabase, default_branch: Branch, car_person_schema_generics: None, query_01: str
+ db: InfrahubDatabase, default_branch: Branch, _car_person_schema_generics: SchemaRoot, query_01: str
) -> None:async def test_schema_models_generics(
- db: InfrahubDatabase, default_branch: Branch, car_person_schema_generics, query_02: str
-):
+ db: InfrahubDatabase, default_branch: Branch, _car_person_schema_generics: SchemaRoot, query_02: str
+) -> None:Add the import: from infrahub.core.schema import SchemaRoot
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In backend/tests/unit/graphql/test_graphql_utils.py around lines 30 to 36, the
fixture parameter car_person_schema_generics is incorrectly typed as None and
not marked as intentionally unused, and the second test is missing an explicit
return type; change the parameter to _car_person_schema_generics: SchemaRoot
(prefix with underscore to indicate unused) and add -> None to the test
signature, and add the import from infrahub.core.schema import SchemaRoot at the
top of the file.
2 participants
This PR aims to phase out the use of subscription=False and mutations=False within the unittests. These parameters was at some point used to speed up the generation of the GraphQL schema, but since we cache the GraphQL schema by hash it's instead a source of flaky tests and can actually slow the CI down as we might end up regenerating the GraphQL schema even though we don't need to (the use of subscription=False or mutations=False causes the schema to be regenerated each time). In production we never use these parameters, it's only within the tests and I think the next step is to remove them completely.
Hopefully this will fix the occasionally flaky tests that we've observed in the unittests.
As part of this fix I introduced a new fixture that forces the regeneration of the schema for tests using GraphQL enums (an experimental feature) as tests might get unreliable without them, the fixture also resets the graphql registry afterwards to avoid impacting other tests.
As part of this PR I've also made some slight fixes to typing where applicable, things like
assert result.datais because result.data is an optional dictionary and we tend to have a lot of typing violations which are not currently reported but listed as errors in the file by pylance, the problem is when we try to access a dictionary within data while not knowing if data isNone.We might be using the
branch.update_schema_hash()call a bit excessively in this PR, but it will still be faster than a complete regeneration of the GraphQL schema in all of these tests.Note there could still potentially be sone areas where test issues comes up as we run some of the tests in an async loop that's updating the global objects (registry), if this happens we might need to reconsider how we approach these tests.
Summary by CodeRabbit