From e22394d5082014258fd2e7fb0cd590ea05fc5bdc Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Mon, 13 Oct 2025 13:08:06 -0700 Subject: [PATCH 1/3] enable ruff PT --- python/lib/sift_client/.ruff.toml | 2 +- .../resources/test_calculated_channels.py | 5 +++-- .../_tests/resources/test_ingestion.py | 1 - .../sift_client/_tests/resources/test_rules.py | 5 +++-- .../sift_client/_tests/resources/test_runs.py | 1 - .../sift_client/_tests/sift_types/test_base.py | 18 +++++++++--------- 6 files changed, 16 insertions(+), 16 deletions(-) diff --git a/python/lib/sift_client/.ruff.toml b/python/lib/sift_client/.ruff.toml index 6786523a6..e11469831 100644 --- a/python/lib/sift_client/.ruff.toml +++ b/python/lib/sift_client/.ruff.toml @@ -39,7 +39,7 @@ select = [ "LOG", # flake8-logging - logging best practices # Tests - # "PT", # flake8-pytest-style - pytest best practices # TODO: FD-59 + "PT", # flake8-pytest-style - pytest best practices ] diff --git a/python/lib/sift_client/_tests/resources/test_calculated_channels.py b/python/lib/sift_client/_tests/resources/test_calculated_channels.py index 3a39bb4b6..1488bb438 100644 --- a/python/lib/sift_client/_tests/resources/test_calculated_channels.py +++ b/python/lib/sift_client/_tests/resources/test_calculated_channels.py @@ -52,7 +52,6 @@ def test_calculated_channel(calculated_channels_api_sync): return calculated_channels[0] -@pytest.fixture(scope="function") def new_calculated_channel(calculated_channels_api_sync, sift_client): """Create a test calculated channel for update tests.""" from datetime import datetime, timezone @@ -450,7 +449,9 @@ async def test_update_with_invalid_expression( assert updated_calc_channel.expression == "invalid_expression" except Exception as e: # If server validates and rejects, that's also acceptable behavior - assert "expression" in str(e).lower() or "invalid" in str(e).lower() + assert ( # noqa: PT017 + "expression" in str(e).lower() or "invalid" in str(e).lower() + ) finally: await calculated_channels_api_async.archive(new_calculated_channel.id_) diff --git a/python/lib/sift_client/_tests/resources/test_ingestion.py b/python/lib/sift_client/_tests/resources/test_ingestion.py index 1858ff10b..dc4c0b122 100644 --- a/python/lib/sift_client/_tests/resources/test_ingestion.py +++ b/python/lib/sift_client/_tests/resources/test_ingestion.py @@ -29,7 +29,6 @@ def test_client_binding(sift_client): assert sift_client.async_.ingestion -@pytest.fixture(scope="function") def test_run(sift_client: SiftClient): """Create a test run for ingestion tests.""" run = sift_client.runs.create( diff --git a/python/lib/sift_client/_tests/resources/test_rules.py b/python/lib/sift_client/_tests/resources/test_rules.py index 6036bec19..6a1a12513 100644 --- a/python/lib/sift_client/_tests/resources/test_rules.py +++ b/python/lib/sift_client/_tests/resources/test_rules.py @@ -54,7 +54,6 @@ def test_rule(rules_api_sync): return rules[0] -@pytest.fixture(scope="function") def new_rule(rules_api_sync, sift_client): """Create a test rule for update tests.""" from datetime import datetime, timezone @@ -558,7 +557,9 @@ async def test_update_with_invalid_expression(self, rules_api_async, new_rule): assert updated_rule.expression == "invalid_expression" except Exception as e: # If server validates and rejects, that's also acceptable behavior - assert "expression" in str(e).lower() or "invalid" in str(e).lower() + assert ( # noqa: PT017 + "expression" in str(e).lower() or "invalid" in str(e).lower() + ) finally: await rules_api_async.archive(new_rule.id_) diff --git a/python/lib/sift_client/_tests/resources/test_runs.py b/python/lib/sift_client/_tests/resources/test_runs.py index f687ecaed..1e563af67 100644 --- a/python/lib/sift_client/_tests/resources/test_runs.py +++ b/python/lib/sift_client/_tests/resources/test_runs.py @@ -46,7 +46,6 @@ def test_run(runs_api_sync): return runs[0] -@pytest.fixture(scope="function") def new_run(runs_api_sync): """Create a test run for update tests.""" run_name = f"test_run_update_{datetime.now(timezone.utc).isoformat()}" diff --git a/python/lib/sift_client/_tests/sift_types/test_base.py b/python/lib/sift_client/_tests/sift_types/test_base.py index cc266e7b9..a0c3cfc58 100644 --- a/python/lib/sift_client/_tests/sift_types/test_base.py +++ b/python/lib/sift_client/_tests/sift_types/test_base.py @@ -288,18 +288,18 @@ def test_multiple_nested_fields_same_parent(self): def test_validation_error_on_invalid_helper_field(self): """Test that MappingHelper validation catches mismatched fields.""" - with pytest.raises(ValueError, match="MappingHelper created for"): - class InvalidModel(ModelCreate[CreateCalculatedChannelRequest]): - name: str + class InvalidModel(ModelCreate[CreateCalculatedChannelRequest]): + name: str - _to_proto_helpers: ClassVar[dict[str, MappingHelper]] = { - "nonexistent_field": MappingHelper(proto_attr_path="some.path"), - } + _to_proto_helpers: ClassVar[dict[str, MappingHelper]] = { + "nonexistent_field": MappingHelper(proto_attr_path="some.path"), + } - def _get_proto_class(self): - return CreateCalculatedChannelRequest + def _get_proto_class(self): + return CreateCalculatedChannelRequest + with pytest.raises(ValueError, match="MappingHelper created for"): # This should raise during __init__ InvalidModel(name="test") @@ -495,7 +495,7 @@ def _from_proto(cls, proto, sift_client=None): model = TestModel(name="test") # Should not be able to modify frozen model - with pytest.raises(Exception): # Pydantic raises ValidationError # noqa: B017 + with pytest.raises(Exception): # noqa: B017, PT011 model.name = "new_name" From d4a1324a55e8e35d118fbd61a728814a546529f4 Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Wed, 15 Oct 2025 09:42:10 -0700 Subject: [PATCH 2/3] update exclusions --- python/lib/sift_client/_tests/resources/test_results.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/lib/sift_client/_tests/resources/test_results.py b/python/lib/sift_client/_tests/resources/test_results.py index 12eaedfe0..a94e28d28 100644 --- a/python/lib/sift_client/_tests/resources/test_results.py +++ b/python/lib/sift_client/_tests/resources/test_results.py @@ -321,7 +321,7 @@ def test_archive_and_delete_test_report(self, sift_client): assert deleted_report is None # Shouldn't reach here so error if we get something. except aiogrpc.AioRpcError as e: self.test_reports.pop("basic_test_report") - assert e.code() == grpc.StatusCode.NOT_FOUND + assert e.code() == grpc.StatusCode.NOT_FOUND # noqa: PT017 def test_import_test_report(self, sift_client): # Import a test report from a file From 95d45bb2fdc816b591c2b540129207241d15b619 Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Wed, 15 Oct 2025 09:59:32 -0700 Subject: [PATCH 3/3] re-add fixture decorators --- .../sift_client/_tests/resources/test_calculated_channels.py | 1 + python/lib/sift_client/_tests/resources/test_ingestion.py | 1 + python/lib/sift_client/_tests/resources/test_results.py | 2 +- python/lib/sift_client/_tests/resources/test_rules.py | 1 + python/lib/sift_client/_tests/resources/test_runs.py | 1 + 5 files changed, 5 insertions(+), 1 deletion(-) diff --git a/python/lib/sift_client/_tests/resources/test_calculated_channels.py b/python/lib/sift_client/_tests/resources/test_calculated_channels.py index 1488bb438..b9dab9fed 100644 --- a/python/lib/sift_client/_tests/resources/test_calculated_channels.py +++ b/python/lib/sift_client/_tests/resources/test_calculated_channels.py @@ -52,6 +52,7 @@ def test_calculated_channel(calculated_channels_api_sync): return calculated_channels[0] +@pytest.fixture def new_calculated_channel(calculated_channels_api_sync, sift_client): """Create a test calculated channel for update tests.""" from datetime import datetime, timezone diff --git a/python/lib/sift_client/_tests/resources/test_ingestion.py b/python/lib/sift_client/_tests/resources/test_ingestion.py index dc4c0b122..d64b91ea6 100644 --- a/python/lib/sift_client/_tests/resources/test_ingestion.py +++ b/python/lib/sift_client/_tests/resources/test_ingestion.py @@ -29,6 +29,7 @@ def test_client_binding(sift_client): assert sift_client.async_.ingestion +@pytest.fixture def test_run(sift_client: SiftClient): """Create a test run for ingestion tests.""" run = sift_client.runs.create( diff --git a/python/lib/sift_client/_tests/resources/test_results.py b/python/lib/sift_client/_tests/resources/test_results.py index a94e28d28..b09b9b839 100644 --- a/python/lib/sift_client/_tests/resources/test_results.py +++ b/python/lib/sift_client/_tests/resources/test_results.py @@ -321,7 +321,7 @@ def test_archive_and_delete_test_report(self, sift_client): assert deleted_report is None # Shouldn't reach here so error if we get something. except aiogrpc.AioRpcError as e: self.test_reports.pop("basic_test_report") - assert e.code() == grpc.StatusCode.NOT_FOUND # noqa: PT017 + assert e.code() == grpc.StatusCode.NOT_FOUND # noqa: PT017 def test_import_test_report(self, sift_client): # Import a test report from a file diff --git a/python/lib/sift_client/_tests/resources/test_rules.py b/python/lib/sift_client/_tests/resources/test_rules.py index 6a1a12513..539b3d006 100644 --- a/python/lib/sift_client/_tests/resources/test_rules.py +++ b/python/lib/sift_client/_tests/resources/test_rules.py @@ -54,6 +54,7 @@ def test_rule(rules_api_sync): return rules[0] +@pytest.fixture def new_rule(rules_api_sync, sift_client): """Create a test rule for update tests.""" from datetime import datetime, timezone diff --git a/python/lib/sift_client/_tests/resources/test_runs.py b/python/lib/sift_client/_tests/resources/test_runs.py index 1e563af67..d2b2defe5 100644 --- a/python/lib/sift_client/_tests/resources/test_runs.py +++ b/python/lib/sift_client/_tests/resources/test_runs.py @@ -46,6 +46,7 @@ def test_run(runs_api_sync): return runs[0] +@pytest.fixture def new_run(runs_api_sync): """Create a test run for update tests.""" run_name = f"test_run_update_{datetime.now(timezone.utc).isoformat()}"