-
Notifications
You must be signed in to change notification settings - Fork 6
Auto fix return type None for tests #504
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
Conversation
WalkthroughThis change primarily adds explicit None return type annotations to numerous pytest fixtures and test functions across integration and unit test files, including many async tests. Some function signatures are reformatted to multiline style without altering bodies. Two behavioral test updates are present: tests/unit/ctl/test_cli.py adjusts expected detail sections in test_info_detail_command_success, and tests/unit/sdk/test_timestamp.py expands coverage (additional parse cases, Timestamp(None) equivalence, and invalid-input error assertion). No production code changes; runtime behavior of existing tests remains the same. Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Deploying infrahub-sdk-python with
|
| Latest commit: |
bf2efc9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://81e52b87.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://pog-test-annotations.infrahub-sdk-python.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## stable #504 +/- ##
===========================================
+ Coverage 53.79% 75.76% +21.96%
===========================================
Files 100 100
Lines 8789 8825 +36
Branches 1719 1728 +9
===========================================
+ Hits 4728 6686 +1958
+ Misses 3688 1664 -2024
- Partials 373 475 +102
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 (5)
tests/unit/sdk/test_utils.py (1)
107-131: Fix: the test currently raises TypeError before calling str_to_bool.
tuple("a", "b", "c")itself raisesTypeError; the intention is to havestr_to_boolraise it for non-supported types. Update to pass a tuple literal (or build via a single iterable).- with pytest.raises(TypeError): - str_to_bool(tuple("a", "b", "c")) + with pytest.raises(TypeError): + str_to_bool(("a", "b", "c"))tests/unit/sdk/test_schema.py (4)
205-206: Fix type annotation: client_type is a str, not list[str]client_type comes from parametrize("client_type", ["standard", "sync"]) and is a single string.
-async def test_schema_wait_happy_path(clients: BothClients, client_type: list[str], httpx_mock: HTTPXMock) -> None: +async def test_schema_wait_happy_path(clients: BothClients, client_type: str, httpx_mock: HTTPXMock) -> None:
226-231: Fix type annotation: client_type should be strSame issue here—parametrize injects a single string value.
-async def test_schema_set_cache_dict(clients: BothClients, client_type: list[str], schema_query_01_data: dict) -> None: +async def test_schema_set_cache_dict(clients: BothClients, client_type: str, schema_query_01_data: dict) -> None:
238-245: Fix type annotation: client_type should be strUpdate the multiline signature accordingly.
-async def test_schema_set_cache_branch_schema( - clients: BothClients, client_type: list[str], schema_query_01_data: dict -) -> None: +async def test_schema_set_cache_branch_schema( + clients: BothClients, client_type: str, schema_query_01_data: dict +) -> None:
190-202: Bug in test: wrong method called in sync branch for “remove enum option raises”The sync branch calls add_enum_option instead of remove_enum_option, so the test doesn’t validate the intended path.
else: with pytest.raises(SchemaNotFoundError): - clients.sync.schema.add_enum_option("DoesNotExist", "atribute", "option") + clients.sync.schema.remove_enum_option("DoesNotExist", "atribute", "option") with pytest.raises(ValueError): - clients.sync.schema.add_enum_option("BuiltinTag", "attribute", "option") + clients.sync.schema.remove_enum_option("BuiltinTag", "attribute", "option")
🧹 Nitpick comments (29)
tests/unit/sdk/test_yaml.py (1)
14-19: Avoid shadowing the built-in namedirMinor clarity/readability nit. Using
directoryavoids masking Python’s built-indir().- dir = here / "test_data" - full_path = dir / file_name + directory = here / "test_data" + full_path = directory / file_name @@ - assert yaml_file.error_message == f"{file_name}: not found at {dir}" + assert yaml_file.error_message == f"{file_name}: not found at {directory}"tests/unit/sdk/checks/test_checks.py (1)
49-56: Match overridden method signatures by annotating validate() -> NoneMinor consistency tweak: these overrides should declare -> None to match the abstract base class (InfrahubCheck.validate returns None). Helps static checkers avoid override-mismatch noise.
class IFCheckAsync(InfrahubCheck): query = "my_query" - async def validate(self, data: dict): + async def validate(self, data: dict) -> None: self.log_error("Not valid") class IFCheckSync(InfrahubCheck): query = "my_query" - def validate(self, data: dict): + def validate(self, data: dict) -> None: self.log_error("Not valid")tests/integration/test_infrahubctl.py (2)
64-74: Tighten fixture return typing for Generator to avoid implicit AnyOptional: specify the send/return types for the
Generatorto avoid implicit Any in type checkers.- def ctl_client_config(self, client: InfrahubClient) -> Generator: + def ctl_client_config(self, client: InfrahubClient) -> Generator[None, None, None]: load_configuration(value="infrahubctl.toml")
49-60: Also consider precise typing on repository fixtureNot required for ANN201, but for completeness you can specify send/return types too.
- def repository(self) -> Generator[str]: + def repository(self) -> Generator[str, None, None]: temp_dir = tempfile.mkdtemp()tests/unit/sdk/test_branch.py (2)
24-24: Optionally annotate parameter type for clarity
methodis a string fromasync_branch_methods; annotating helps type checkers and readability.-def test_validate_method_signature(method) -> None: +def test_validate_method_signature(method: str) -> None:
34-34: Parameter typing nit: narrow client_typeOptional: annotate
client_typeasstr(orLiteral["standard", "sync"]if you prefer stricter typing).-async def test_get_branches(clients, mock_branches_list_query, client_type) -> None: +async def test_get_branches(clients, mock_branches_list_query, client_type: str) -> None:If you opt for
Literal, add:from typing import Literaland use:
client_type: Literal["standard", "sync"]tests/unit/sdk/test_protocols_generator.py (1)
40-41: Remove duplicate assertion or assert idempotence explicitlyTwo identical assertions don’t add value. If you intend to assert idempotence across repeated calls, make it explicit; otherwise drop the duplicate.
Apply one of the following:
Option A — keep a single assertion:
- assert CodeGenerator._jinja2_filter_syncify(value=test_case.input, sync=test_case.sync) == test_case.output - assert CodeGenerator._jinja2_filter_syncify(value=test_case.input, sync=test_case.sync) == test_case.output + assert CodeGenerator._jinja2_filter_syncify(value=test_case.input, sync=test_case.sync) == test_case.outputOption B — assert idempotence explicitly:
- assert CodeGenerator._jinja2_filter_syncify(value=test_case.input, sync=test_case.sync) == test_case.output - assert CodeGenerator._jinja2_filter_syncify(value=test_case.input, sync=test_case.sync) == test_case.output + first = CodeGenerator._jinja2_filter_syncify(value=test_case.input, sync=test_case.sync) + second = CodeGenerator._jinja2_filter_syncify(value=test_case.input, sync=test_case.sync) + assert first == second == test_case.outputtests/unit/ctl/test_validate_app.py (1)
35-36: Make xfail strict to catch unexpected passesIf this test starts passing before the underlying issue is fixed, you likely want the suite to fail rather than silently XPASS.
Apply:
-@pytest.mark.xfail(reason="FIXME: Currently not catching the proper exception") +@pytest.mark.xfail(reason="FIXME: Currently not catching the proper exception", strict=True) async def test_validate_schema_json_non_valid() -> None:tests/unit/sdk/test_repository.py (1)
32-32: Docstring wording nit: existing repos do have an active branchThe test name/docstring suggests “without an active branch,” but an initialized repo has a default branch. Consider rewording for accuracy.
- """Test that the GitRepoManager uses an existing repository without an active branch.""" + """Test that the GitRepoManager opens and uses an existing repository."""tests/unit/sdk/test_object_store.py (1)
47-47: Annotated async test signatureConsider, optionally, adding a type for
methodif you later enforce ANN001/ANN002; fine to defer since this PR targets ANN201 only.tests/unit/sdk/test_group_context.py (1)
20-29: Align signature/parameter comparisons with existing client and node testsIn other modules (e.g., tests/unit/sdk/test_client.py and tests/unit/sdk/test_node.py), parameter comparisons normalize annotations using replace_async_parameter_annotations/replace_sync_parameter_annotations to avoid false negatives when types differ only by sync/async aliases. Here you compare parameters directly, which is stricter and potentially brittle.
Consider harmonizing with the established pattern to reduce flakiness and improve consistency across suites.
Proposed refactor (adapts the approach used in client/node tests):
@pytest.mark.parametrize("method", async_methods) -async def test_validate_method_signature( - method, replace_sync_return_annotation, replace_async_return_annotation -) -> None: +async def test_validate_method_signature( + method, + replace_sync_return_annotation, + replace_async_return_annotation, + replace_async_parameter_annotations, + replace_sync_parameter_annotations, +) -> None: async_method = getattr(InfrahubGroupContext, method) sync_method = getattr(InfrahubGroupContextSync, method) async_sig = inspect.signature(async_method) sync_sig = inspect.signature(sync_method) - assert async_sig.parameters == sync_sig.parameters + assert replace_sync_parameter_annotations(async_sig.parameters) == replace_sync_parameter_annotations( + sync_sig.parameters + ) + assert replace_async_parameter_annotations(async_sig.parameters) == replace_async_parameter_annotations( + sync_sig.parameters + ) assert async_sig.return_annotation == replace_sync_return_annotation(sync_sig.return_annotation) assert replace_async_return_annotation(async_sig.return_annotation) == sync_sig.return_annotationtests/unit/sdk/test_topological_sort.py (1)
6-8: Library typing could be generalized to match test usage with non-string node IDsTests use integers and strings as node identifiers, whereas the library’s signature currently advertises Mapping[str, Iterable[str]] and list[set[str]]. Consider a generic type parameter for better static correctness.
Outside this file, update the library typing:
--- a/infrahub_sdk/topological_sort.py +++ b/infrahub_sdk/topological_sort.py @@ -from typing import Iterable, Mapping +from typing import Iterable, Mapping, TypeVar, Hashable +T = TypeVar("T", bound=Hashable) -def topological_sort(dependency_dict: Mapping[str, Iterable[str]]) -> list[set[str]]: +def topological_sort(dependency_dict: Mapping[T, Iterable[T]]) -> list[set[T]]: if not dependency_dict: return [] @@ - ordered = [] + ordered: list[set[T]] = []This aligns type hints with actual usage in tests here and elsewhere.
tests/unit/ctl/test_cli.py (1)
51-58: Minor maintainability: parametrize expected headingsTo keep this resilient as sections evolve, consider parametrizing the headings rather than looping inline. This improves failure messages and makes additions/removals a one-line change.
Example:
- for expected in [ - "Connection Status", - "Version Information", - "Client Info", - "Infrahub Info", - "Groups:", - ]: - assert expected in result.stdout, f"'{expected}' not found in detailed info command output" + headings = pytest.mark.parametrize( + "expected", + ["Connection Status", "Version Information", "Client Info", "Infrahub Info", "Groups:"], + ) + for expected in ["Connection Status", "Version Information", "Client Info", "Infrahub Info", "Groups:"]: + assert expected in result.stdout, f"'{expected}' not found in detailed info command output"Or define a shared EXPECTED_HEADINGS constant reused by multiple tests.
tests/unit/pytest_plugin/test_plugin.py (2)
1-5: Optional: annotate pytester parameter for clearer intentTo complement return annotations, you can also type the pytester fixture for better IDE/help feedback.
- Add import at file top:
from _pytest.pytester import Pytester
- Then adjust signatures, e.g.:
-def test_help_message(pytester) -> None: +def test_help_message(pytester: Pytester) -> None: ...Apply similarly to other tests in this file.
13-26: Optional: use dedent for heredoc YAML to avoid indentation driftThe YAML/JSON blobs are indented within Python code. They work today, but using textwrap.dedent reduces the chance of accidental leading spaces breaking formats when editing.
Sketch:
from textwrap import dedent pytester.makefile( ".yml", test_empty=dedent( """\ --- version: "1.0" infrahub_tests: [] """ ), )tests/unit/sdk/test_query_analyzer.py (4)
8-14: Consistent typing and minor cleanup recommended.
- Good addition of -> None. For consistency with the rest of the file, consider annotating fixtures here as
query_01: str, bad_query_01: str.- Inside the
pytest.raisesblock, thegqa =assignment is unused; call the constructor without binding to a variable.-async def test_analyzer_init_query_only(query_01, bad_query_01) -> None: +async def test_analyzer_init_query_only(query_01: str, bad_query_01: str) -> None: @@ - with pytest.raises(GraphQLSyntaxError): - gqa = GraphQLQueryAnalyzer(query=bad_query_01) + with pytest.raises(GraphQLSyntaxError): + GraphQLQueryAnalyzer(query=bad_query_01)
64-76: Optional: parametrize repeated depth checks.You can reduce repetition by parametrizing the four (query, expected_depth) pairs. Functionally correct as-is.
+@pytest.mark.parametrize("q,expected", [ + (query_01, 9), + (query_02, 11), + (query_03, 9), + (query_04, 6), +]) +async def test_calculate_depth_param(q: str, expected: int) -> None: + gqa = GraphQLQueryAnalyzer(query=q) + assert await gqa.calculate_depth() == expected
78-90: Optional: parametrize repeated height checks.Same suggestion as for depth to DRY this test.
150-176: Add parameter type hints for clarity (optional)In tests/unit/sdk/test_query_analyzer.py (around line 150), consider annotating the test parameters to mirror the existing return annotation and improve readability/IDE support:
-async def test_get_nested_variables(var_type, var_required) -> None: +async def test_get_nested_variables(var_type: str, var_required: bool) -> None:This is purely a nit—tests already declare
-> None—but adding: strand: boolcan make the intent clearer and align with other typed parametrized tests in this suite.tests/unit/sdk/test_timestamp.py (2)
30-43: Nit: consider parametrizing parse cases and aligning test id names.Current assertions are fine. Optionally parametrize inputs and expected outcomes for consistency with later tests; also consider adding an explicit expectation for the date-only input.
66-69: Nit: misleading test id.The param id
"milliseconds_with_offset"uses aZ-suffixed timestamp (UTC), not an offset example. Either update the id or provide an offset-based input.- id="milliseconds_with_offset", + id="microseconds_utc",tests/unit/sdk/test_utils.py (3)
28-32: LGTM: explicit None return added.Minor note: asserting uniqueness with two calls is fine; for higher assurance, consider sampling more ids if flakiness ever appears.
198-213: Prefer context manager for TemporaryDirectory to avoid leaks on failure.Use a
withblock so cleanup is guaranteed even if an assertion fails midway.-def test_write_to_file() -> None: - tmp_dir = tempfile.TemporaryDirectory() - directory = Path(tmp_dir.name) +def test_write_to_file() -> None: + with tempfile.TemporaryDirectory() as tmp_dir: + directory = Path(tmp_dir) @@ - tmp_dir.cleanup() + # cleanup is automatic via the context manager
215-230: LGTM; consider hardening against time-based flakiness.These assertions depend on current time; if flakiness ever appears, consider freezing time or comparing ranges. Functional change not required now.
tests/integration/test_spec_object.py (1)
138-151: LGTM: menu load and verification; minor naming nit.Small readability nit:
menu = await ...then{menu.display_label: menu for menu in menu}reuses the name. Considermenus = await ...andfor m in menus.- menu = await client.filters(kind=menu_file.spec.kind, protected__value=False, branch=branch_name) - assert len(menu) == 3 - menu_by_name = {menu.display_label: menu for menu in menu} - await menu_by_name["Animals"].children.fetch() + menus = await client.filters(kind=menu_file.spec.kind, protected__value=False, branch=branch_name) + assert len(menus) == 3 + menu_by_name = {m.display_label: m for m in menus} + await menu_by_name["Animals"].children.fetch()tests/unit/sdk/test_graphql.py (1)
265-281: Optional: simplify multiline expected strings with dedentSeveral tests compare large triple-quoted strings. Using textwrap.dedent can reduce indentation-related noise and make diffs smaller if indentation changes.
Apply this pattern where appropriate:
+from textwrap import dedent ... - expected_query = """ + expected_query = dedent(""" query { device { name { value } } } -""" +""")tests/unit/sdk/test_artifact.py (1)
9-12: Optional: tighten typing for client_type with LiteralIf you want stronger type safety during refactors, consider annotating client_type as a Literal of the two allowed values.
+from typing import Literal client_types = ["standard", "sync"] ... -async def test_node_artifact_generate_raise_featurenotsupported( - client, client_type, location_schema, location_data01 -) -> None: +async def test_node_artifact_generate_raise_featurenotsupported( + client, client_type: Literal["standard", "sync"], location_schema, location_data01 +) -> None:tests/integration/test_infrahub_client.py (2)
35-41: Annotate async-yield fixture with AsyncIterator[None] for full typing accuracyset_pagination_size3 is an async generator fixture; annotating it as AsyncIterator[None] improves type checking consistency and future refactors. Also import AsyncIterator.
-from typing import TYPE_CHECKING +from typing import TYPE_CHECKING +from collections.abc import AsyncIterator ... -@pytest.fixture -async def set_pagination_size3(self, client: InfrahubClient): +@pytest.fixture +async def set_pagination_size3(self, client: InfrahubClient) -> AsyncIterator[None]: original_pagination_size = client.pagination_size client.pagination_size = 3 yield client.pagination_size = original_pagination_size
1-1: Optional: Add Missing Return Annotations in TestsWe ran the suggested repo-wide scan and identified 40+ test functions and fixtures without explicit return annotations (e.g.,
-> Noneor the appropriate type). While annotating test code is not strictly required, adding these helps maintain consistency with ANN201 and improves readability.Summary of findings (non-exhaustive):
- tests/conftest.py:
event_loop(),clean_env_vars()- tests/unit/pytest_plugin/test_plugin.py:
async def transform(self, data)- tests/unit/sdk/test_repository.py:
temp_dir(),mock_init(...)- tests/unit/sdk/test_graphql.py: several
query_data_*()helpers- tests/unit/ctl/test_transform_app.py:
tags_transform_dir()- tests/unit/ctl/conftest.py:
async def authentication_error_payload()- tests/integration/test_infrahub_client.py:
async def set_pagination_size3(self, client: InfrahubClient)- …and dozens more in
tests/unit/sdk/conftest.pyand other modulesRecommendation:
• For each public test function or fixture, add an explicit-> None(or the correct return type).
• If you prefer to keep tests unannotated, you can safely ignore this; these annotations are purely stylistic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (37)
tests/conftest.py(1 hunks)tests/integration/test_infrahub_client.py(9 hunks)tests/integration/test_infrahubctl.py(1 hunks)tests/integration/test_node.py(6 hunks)tests/integration/test_repository.py(1 hunks)tests/integration/test_schema.py(1 hunks)tests/integration/test_spec_object.py(7 hunks)tests/unit/ctl/test_branch_app.py(2 hunks)tests/unit/ctl/test_cli.py(2 hunks)tests/unit/ctl/test_schema_app.py(3 hunks)tests/unit/ctl/test_validate_app.py(2 hunks)tests/unit/pytest_plugin/test_plugin.py(6 hunks)tests/unit/sdk/checks/test_checks.py(2 hunks)tests/unit/sdk/spec/test_object.py(4 hunks)tests/unit/sdk/test_artifact.py(6 hunks)tests/unit/sdk/test_batch.py(3 hunks)tests/unit/sdk/test_branch.py(2 hunks)tests/unit/sdk/test_client.py(26 hunks)tests/unit/sdk/test_config.py(1 hunks)tests/unit/sdk/test_diff_summary.py(1 hunks)tests/unit/sdk/test_graphql.py(13 hunks)tests/unit/sdk/test_group_context.py(5 hunks)tests/unit/sdk/test_node.py(50 hunks)tests/unit/sdk/test_object_store.py(3 hunks)tests/unit/sdk/test_protocols_generator.py(1 hunks)tests/unit/sdk/test_query_analyzer.py(6 hunks)tests/unit/sdk/test_repository.py(4 hunks)tests/unit/sdk/test_schema.py(12 hunks)tests/unit/sdk/test_schema_sorter.py(1 hunks)tests/unit/sdk/test_store.py(4 hunks)tests/unit/sdk/test_store_branch.py(4 hunks)tests/unit/sdk/test_task.py(8 hunks)tests/unit/sdk/test_timestamp.py(6 hunks)tests/unit/sdk/test_topological_sort.py(6 hunks)tests/unit/sdk/test_utils.py(8 hunks)tests/unit/sdk/test_uuidt.py(2 hunks)tests/unit/sdk/test_yaml.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (26)
tests/unit/sdk/test_object_store.py (6)
tests/unit/sdk/test_branch.py (2)
test_method_sanity(17-20)test_validate_method_signature(24-30)tests/unit/sdk/test_client.py (2)
test_method_sanity(32-35)test_validate_method_signature(39-58)tests/unit/sdk/test_group_context.py (2)
test_method_sanity(13-16)test_validate_method_signature(20-29)tests/unit/sdk/test_node.py (2)
test_method_sanity(56-59)test_validate_method_signature(75-102)tests/unit/sdk/test_schema.py (2)
test_method_sanity(28-31)test_validate_method_signature(35-41)tests/unit/sdk/conftest.py (1)
clients(37-42)
tests/integration/test_schema.py (1)
tests/unit/sdk/conftest.py (1)
client(32-33)
tests/unit/sdk/test_protocols_generator.py (2)
infrahub_sdk/protocols_generator/generator.py (1)
_jinja2_filter_syncify(89-109)tests/unit/sdk/conftest.py (2)
client(32-33)mock_schema_query_05(1846-1853)
tests/unit/sdk/test_branch.py (2)
tests/unit/sdk/test_client.py (1)
test_method_sanity(32-35)tests/unit/sdk/conftest.py (2)
clients(37-42)mock_branches_list_query(1442-1474)
tests/unit/sdk/checks/test_checks.py (3)
tests/unit/sdk/conftest.py (1)
client(32-33)infrahub_sdk/checks.py (1)
InfrahubCheck(33-175)tests/unit/sdk/checks/conftest.py (1)
mock_gql_query_my_query(13-22)
tests/integration/test_spec_object.py (2)
tests/integration/test_infrahub_client.py (1)
test_create_branch(140-143)tests/integration/test_node.py (1)
initial_schema(13-19)
tests/unit/sdk/test_diff_summary.py (1)
tests/unit/sdk/conftest.py (2)
clients(37-42)BothClients(25-28)
tests/unit/ctl/test_branch_app.py (2)
tests/unit/sdk/conftest.py (1)
mock_branches_list_query(1442-1474)tests/unit/ctl/conftest.py (2)
authentication_error_payload(62-73)mock_branch_create_error(77-95)
tests/unit/sdk/test_config.py (2)
infrahub_sdk/exceptions.py (1)
ValidationError(115-127)infrahub_sdk/config.py (1)
password_authentication(133-134)
tests/unit/sdk/test_store.py (2)
tests/unit/sdk/test_store_branch.py (4)
test_node_store_set(9-22)test_node_store_set_no_hfid(25-39)test_node_store_get(42-65)test_node_store_get_with_hfid(68-95)tests/unit/sdk/conftest.py (3)
clients(37-42)schema_with_hfid(176-237)location_schema(140-172)
tests/unit/sdk/test_batch.py (1)
tests/unit/sdk/conftest.py (2)
clients(37-42)BothClients(25-28)
tests/unit/sdk/spec/test_object.py (1)
tests/unit/sdk/conftest.py (2)
client(32-33)mock_schema_query_01(1824-1831)
tests/unit/ctl/test_schema_app.py (1)
infrahub_sdk/ctl/utils.py (1)
get_fixtures_dir(178-181)
tests/unit/ctl/test_cli.py (2)
infrahub_sdk/async_typer.py (2)
runner(17-18)command(29-31)tests/unit/sdk/conftest.py (2)
mock_query_infrahub_version(2194-2196)mock_query_infrahub_user(2200-2203)
tests/unit/sdk/test_query_analyzer.py (2)
tests/unit/sdk/conftest.py (8)
query_01(2207-2231)bad_query_01(2403-2424)query_03(2281-2317)query_introspection(2428-2530)query_02(2235-2277)query_04(2321-2336)query_05(2340-2370)query_06(2374-2399)infrahub_sdk/analyzer.py (2)
GraphQLQueryAnalyzer(32-121)nbr_queries(51-52)
tests/unit/sdk/test_store_branch.py (2)
tests/unit/sdk/test_store.py (4)
test_node_store_set(11-32)test_node_store_set_no_hfid(36-66)test_node_store_get(70-110)test_node_store_get_with_hfid(114-148)tests/unit/sdk/conftest.py (2)
schema_with_hfid(176-237)location_schema(140-172)
tests/unit/ctl/test_validate_app.py (2)
tests/helpers/fixtures.py (1)
get_fixtures_dir(4-7)tests/helpers/cli.py (1)
remove_ansi_color(4-6)
tests/unit/sdk/test_repository.py (1)
infrahub_sdk/repository.py (2)
GitRepoManager(9-33)active_branch(31-33)
tests/unit/sdk/test_topological_sort.py (1)
infrahub_sdk/topological_sort.py (1)
topological_sort(20-40)
tests/unit/sdk/test_utils.py (2)
infrahub_sdk/utils.py (8)
generate_short_id(30-32)is_valid_url(215-227)duplicates(105-116)base36encode(35-50)base36decode(53-54)base16encode(61-76)base16decode(57-58)dict_hash(239-245)tests/unit/sdk/conftest.py (2)
query_01(2207-2231)query_02(2235-2277)
tests/unit/sdk/test_artifact.py (1)
tests/unit/sdk/conftest.py (6)
location_schema(140-172)location_data01(375-412)clients(37-42)mock_rest_api_artifact_fetch(1863-1954)device_schema(1184-1230)device_data(1234-1387)
tests/unit/sdk/test_task.py (2)
tests/unit/sdk/conftest.py (1)
clients(37-42)tests/unit/sdk/test_client.py (1)
test_method_get_not_found(404-411)
tests/unit/sdk/test_group_context.py (4)
tests/unit/sdk/test_branch.py (2)
test_method_sanity(17-20)test_validate_method_signature(24-30)tests/unit/sdk/test_client.py (2)
test_method_sanity(32-35)test_validate_method_signature(39-58)tests/unit/sdk/test_node.py (2)
test_method_sanity(56-59)test_validate_method_signature(75-102)tests/unit/sdk/conftest.py (3)
replace_sync_return_annotation(115-122)replace_async_return_annotation(91-97)std_group_schema(241-251)
tests/unit/sdk/test_schema.py (7)
tests/unit/sdk/test_branch.py (2)
test_method_sanity(17-20)test_validate_method_signature(24-30)tests/unit/sdk/test_client.py (2)
test_method_sanity(32-35)test_validate_method_signature(39-58)tests/unit/sdk/test_group_context.py (2)
test_method_sanity(13-16)test_validate_method_signature(20-29)tests/unit/sdk/test_node.py (2)
test_method_sanity(56-59)test_validate_method_signature(75-102)tests/unit/sdk/test_object_store.py (2)
test_method_sanity(40-43)test_validate_method_signature(47-53)tests/unit/sdk/conftest.py (6)
mock_schema_query_01(1824-1831)rfile_schema(813-849)clients(37-42)mock_query_mutation_schema_dropdown_add(2149-2151)mock_query_mutation_schema_enum_add(2161-2163)mock_query_mutation_schema_enum_remove(2167-2169)infrahub_sdk/schema/__init__.py (6)
remove_dropdown_option(426-431)remove_dropdown_option(650-655)add_enum_option(374-379)add_enum_option(600-605)remove_enum_option(381-386)remove_enum_option(607-612)
tests/unit/sdk/test_client.py (1)
tests/unit/sdk/conftest.py (8)
clients(37-42)mock_query_repository_count(1688-1690)mock_query_repository_page1_2(1709-1755)mock_query_repository_page1_1(1612-1649)mock_query_corenode_page1_1(1653-1684)mock_schema_query_01(1824-1831)mock_query_repository_page1_empty(1694-1705)echo_clients(46-58)
tests/unit/sdk/test_node.py (3)
tests/unit/sdk/test_branch.py (1)
test_method_sanity(17-20)tests/unit/sdk/test_client.py (1)
test_method_sanity(32-35)infrahub_sdk/node/parsers.py (1)
parse_human_friendly_id(6-15)
⏰ 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). (6)
- 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: integration-tests-latest-infrahub
- GitHub Check: Cloudflare Pages
🔇 Additional comments (90)
tests/unit/sdk/test_yaml.py (1)
51-55: Explicit None return annotation for test — LGTMMatches Ruff ANN201 intent; no behavioral impact.
tests/conftest.py (1)
23-26: Fixture explicit None return annotation — LGTMAccurately reflects the autouse session fixture’s behavior and satisfies ANN201.
tests/unit/sdk/test_uuidt.py (2)
7-21: Add -> None to test_uuidt — LGTMNo runtime changes; satisfies ANN201.
22-28: Add -> None to test_uuidt_short — LGTMConsistent with the suite-wide typing updates.
tests/unit/sdk/test_diff_summary.py (1)
95-106: Annotate async test with -> None — LGTMKeeps async test semantics intact while addressing ANN201.
tests/integration/test_repository.py (1)
14-26: Add -> None to async test method — LGTMType hint aligns with PR objective; no behavioral change.
tests/integration/test_schema.py (1)
18-18: Explicit -> None on async test method: LGTMThe added return annotation satisfies Ruff ANN201 without changing behavior. All imports are present, so no runtime impact.
tests/unit/sdk/checks/test_checks.py (3)
9-9: Return type annotation added: LGTMMatches the PR’s objective (ANN201) and keeps runtime unchanged.
37-37: Return type annotation added: LGTMConsistent with the rest of the suite and with async usage here.
45-45: Return type annotation added: LGTMNo functional changes; aligns with Ruff requirements.
tests/integration/test_infrahubctl.py (1)
45-45: Annotating async fixture to -> None: LGTMClear signal that the fixture is setup-only; works well with its usage as
base_dataset: Nonein tests.tests/unit/sdk/test_schema_sorter.py (1)
5-5: Return type annotation added: LGTMConforms to ANN201; no behavior changes.
tests/unit/sdk/test_branch.py (1)
17-17: Return type annotation added: LGTMMeets Ruff ANN201; no runtime impact.
tests/unit/sdk/test_protocols_generator.py (2)
39-39: Add explicit None return on async test — looks goodThe annotation aligns with Ruff ANN201 and doesn’t change behavior.
44-44: Add explicit None return on async test — looks goodConsistent with the PR’s goal; no functional impact.
tests/unit/ctl/test_branch_app.py (1)
9-9: Return type annotations added — consistent and correctAll three tests now explicitly return None as intended for Ruff ANN201. No behavior changes.
Also applies to: 15-15, 27-27
tests/unit/sdk/test_config.py (1)
7-7: Type annotations finalized for tests — LGTMAdding explicit -> None return types meets the linting objective without affecting test semantics.
Also applies to: 14-14, 21-21, 26-26, 31-31
tests/unit/ctl/test_validate_app.py (1)
11-11: Explicit None returns — good consistencyThe annotations meet ANN201 across these tests and keep behavior unchanged.
Also applies to: 19-19, 27-27, 36-36
tests/unit/sdk/test_repository.py (1)
19-19: Return type annotations on repository tests — looks goodThese updates are in line with the PR goals; no runtime impact.
Also applies to: 31-31, 42-42, 50-50, 62-62
tests/unit/sdk/test_store_branch.py (4)
9-9: Explicit None return type added — compliant with Ruff ANN201Good addition; no behavioral change and keeps tests consistent with the repo-wide typing pass.
25-25: Consistent annotation on test functionMatches the PR objective and maintains parity with sibling tests.
42-42: Return type annotation looks goodClear intent for pytest; no impact on runtime.
68-68: LGTM on annotationBrings the file fully in line with ANN201 expectations.
tests/unit/sdk/test_store.py (4)
36-36: Return annotation added — consistent with surrounding testsAll good.
70-70: Annotation aligns with PR scopeNo functional impact; improves typing clarity.
114-114: Final test in file annotated — looks goodCompletes the set for this file.
11-11: All test functions have explicit-> Nonereturn annotationsThe repository-wide verification script confirmed 0 missing return annotations for functions named
test_*undertests/. ANN201 is fully resolved across the test suite.tests/unit/sdk/test_object_store.py (3)
40-40: Add -> None for async test — correct and harmlessMatches ANN201 and mirrors similar “method sanity” tests elsewhere.
57-57: Explicit None return type for get testGood; no behavior change.
69-69: Upload test annotated — LGTMConsistent with the rest of the file.
tests/unit/ctl/test_schema_app.py (4)
12-12: Return type None addedCorrect and consistent with the objective.
20-20: Annotation looks goodKeeps tests uniform.
51-51: Annotated multi-schema load testNo behavioral deltas; good.
84-84: Final test annotated — LGTMAll CLI schema tests now align with ANN201.
tests/unit/sdk/test_batch.py (3)
18-18: Added -> None to async test — correctClearer intent; no runtime change.
57-57: Annotated return type for exception-returning variantMatches the rest of the suite.
103-103: Annotated return type for exception-raising variantLooks good and consistent.
tests/unit/sdk/test_group_context.py (2)
13-17: Typing-only changes: explicit -> None annotations look goodThese updates satisfy Ruff ANN201 without altering behavior. No issues spotted in these tests.
Also applies to: 32-42, 44-52, 54-66, 68-82
54-66: Stable, deterministic hashing confirmedI’ve verified that the
dict_hashhelper ininfrahub_sdk/utils.pyusesujson.dumps(..., sort_keys=True)followed by an MD5 digest, ensuring keys are always sorted before hashing. This guarantees that_generate_group_nameproduces a consistent suffix across Python versions and platforms. No further changes are needed.tests/unit/sdk/test_topological_sort.py (1)
6-8: Typing-only changes: explicit -> None annotations look goodNo behavior changes; tests continue to reflect intended scenarios (empty input, cycles, disjoint graphs, and a deeper tree).
Also applies to: 10-17, 19-27, 29-35, 37-47, 49-62, 64-74, 76-95
tests/unit/sdk/spec/test_object.py (1)
46-51: Typing-only changes: async tests now explicitly return None — goodAll four tests simply add -> None. Logic for positive and negative validation paths, plus parameterized relationship parsing, remains unchanged and correct.
Also applies to: 53-61, 63-71, 112-120
tests/unit/ctl/test_cli.py (1)
11-15: CLI tests: -> None annotations and updated detail sections LGTM
- The new headings asserted in test_info_detail_command_success reflect the expanded output; checks are presence-only, which is resilient to ordering.
- General None annotations satisfy Ruff without behavior change.
Also applies to: 17-21, 23-27, 29-33, 35-40, 42-46, 48-59, 61-65
tests/unit/pytest_plugin/test_plugin.py (1)
1-5: Typing-only changes: -> None annotations across plugin tests look goodNo behavioral changes; these satisfy Ruff ANN201 consistently across this module.
Also applies to: 7-11, 13-26, 28-64, 66-105, 107-162, 164-234, 236-291
tests/unit/sdk/test_query_analyzer.py (4)
16-22: LGTM: return annotation added.The explicit
-> Nonealigns with Ruff ANN201. No behavioral changes.
24-35: LGTM: explicit return typing across query type checks.Clear and readable; assertions align with
GraphQLOperationequality semantics.
37-62: LGTM: fields extraction checks remain precise.Assertions validate nested structure well; no changes beyond typing.
92-148: LGTM: variables extraction assertions remain accurate.Expanded type hints and
-> Nonelook good; expectations match the analyzer’s handling of defaults and non-null.tests/unit/sdk/test_timestamp.py (8)
12-20: LGTM: added None-return and extra None-init coverage.Good addition to cover
Timestamp(None)parity with default initialization.
22-28: LGTM: constructor copy semantics covered.Return typing added; assertions remain correct.
88-92: LGTM: string formatting with/without Z validated.Clear expectations; return typing added.
94-98: LGTM.
100-104: LGTM.
106-120: LGTM: comparison operators behavior validated.
122-129: LGTM: serialization round-trip checks.
132-135: LGTM: error type validated for invalid inputs.tests/unit/sdk/test_utils.py (10)
34-44: LGTM.
60-62: LGTM.
64-70: LGTM.
72-91: LGTM.
93-105: LGTM.
133-138: LGTM.
140-145: LGTM.
147-153: LGTM.
155-168: LGTM: async extract_fields test typed and annotated.
170-196: LGTM: fragment extraction validated.tests/unit/sdk/test_task.py (9)
13-21: LGTM: explicit None return and dual-client coverage.
24-32: LGTM: full retrieval path with include flags.
34-47: LGTM: count query generation validated.Clear assertions on rendered GraphQL.
48-62: LGTM: filtered count query rendering.
65-73: LGTM: filter semantics over both clients.
76-82: LGTM: TooManyTasksError path.
85-91: LGTM: NotFound error path.
94-102: LGTM: get by id (happy path).
105-156: LGTM: full task with logs and related nodes validated.Return types added; assertions are comprehensive.
tests/integration/test_spec_object.py (8)
27-35: LGTM: added None-return for loader order test.Clear assertions on ordering.
48-55: LGTM: fixture typed to return None.Explicit typing improves clarity with Ruff ANN201 context.
56-58: LGTM: branch creation test uses explicit None return.
59-69: LGTM: tags load path.
70-83: LGTM: tags update path.
84-94: LGTM: persons load path.
95-105: LGTM: dogs load path.
106-137: LGTM: persons02 with relationships validation.Comprehensive checks of relationships; return types added.
tests/integration/test_node.py (1)
21-24: Return annotations added correctly for async testsAll updated signatures now explicitly return None, aligning with Ruff ANN201 without affecting behavior. Looks good.
Also applies to: 33-38, 48-56, 66-75, 88-94, 174-187
tests/unit/sdk/test_graphql.py (1)
118-118: Consistent -> None annotations across test functionsThe added return annotations are correct and non-invasive. Nice sweep.
Also applies to: 161-161, 183-183, 207-207, 262-262, 286-286, 310-310, 334-334, 354-354, 374-374, 414-414, 460-460, 494-494
tests/unit/sdk/test_artifact.py (1)
10-13: Return annotations for async tests look goodThe added -> None annotations are correct for both standard and sync code paths under parametrize. No behavioral changes introduced.
Also applies to: 25-28, 40-41, 53-60, 69-72, 88-91
tests/integration/test_infrahub_client.py (1)
21-33: Good compliance with ANN201: explicit -> None on fixtures/testsThe base_dataset fixture and updated test methods now explicitly return None. Changes are consistent with the PR objective.
Also applies to: 42-61, 62-67, 83-91, 92-100, 101-110, 111-118, 119-139, 140-148, 149-156, 157-160, 161-172, 173-210
tests/unit/sdk/test_schema.py (1)
28-42: Return annotations: overall changes look goodThe added -> None annotations across these tests align with ANN201 and don’t change behavior.
Also applies to: 45-52, 91-101, 110-116, 120-128, 131-136, 139-146, 149-160, 163-174, 176-188, 253-302, 304-329, 339-362, 372-395
tests/unit/sdk/test_client.py (2)
32-32: LGTM: explicit None return annotations across testsAll these changes cleanly satisfy Ruff ANN201 without altering runtime behavior. Async tests remain Coroutine[..., None] under type-checkers; sync tests are likewise explicit. Good consistency.
Also applies to: 45-45, 61-61, 70-70, 86-86, 96-96, 106-106, 116-116, 127-127, 138-138, 157-157, 177-177, 195-195, 213-213, 225-225, 273-273, 322-322, 368-368, 404-406, 417-417, 435-435, 444-444, 490-492, 521-521, 622-622, 737-737
32-32: Verification Passed: All test functions declare-> None
The optional repo-wide guard script confirms that everytest_function now has an explicit-> Nonereturn annotation. No further action is required.tests/unit/sdk/test_node.py (1)
56-56: LGTM: consistent -> None return types throughoutThe explicit None annotations across these async and sync tests align with ANN201 and improve type clarity without behavioral changes. Consistency looks solid across the file.
Also applies to: 81-81, 106-106, 113-113, 126-126, 149-149, 167-167, 206-206, 255-255, 277-277, 377-377, 421-421, 485-485, 517-517, 653-653, 705-705, 781-781, 821-821, 849-849, 996-996, 1058-1058, 1181-1181, 1234-1234, 1293-1293, 1324-1324, 1338-1338, 1372-1372, 1398-1398, 1437-1437, 1469-1469, 1534-1534, 1571-1571, 1585-1585, 1601-1601, 1658-1658, 1729-1729, 1774-1774, 1822-1822, 1843-1843, 1867-1867, 1929-1929, 1946-1946, 1965-1965, 1992-1992, 2017-2017, 2035-2035, 2054-2054, 2106-2106, 2165-2165, 2260-2260, 2322-2322
Fixes for Ruff ANN201: Missing return type annotation for public function
Summary by CodeRabbit