Merge pull request #1372 from newrelic/fix-linter-pyteststyle

Enable linter flake8-pytest-style

Author: hmstepanek

pyproject.toml

@@ -59,7 +59,7 @@ select = [
     "G",  # flake8-logging-format
     "INP",  # flake8-no-pep420
     "PYI",  # flake8-pyi
-    # "PT",  # flake8-pytest-style
+    "PT",  # flake8-pytest-style
     "Q",  # flake8-quotes
     "RSE",  # flake8-raise
     # "RET",  # flake8-return
@@ -101,13 +101,13 @@ select = [
 
 ignore = [
     # Temporarily disabled rules
-    "RUF100",  # unused-noqa (TODO: remove this once all linters are enabled)
     "PERF203",  # try-except-in-loop (most of these are unavoidable)
     "S110",  # try-except-pass (Bandit wants us to log the exception, which is usually pointless. Spot check these later)
     "PLW0603",  # global-statement (this is currently used extensively)
     "PLW2901",  # redefined-loop-name (frequently used, not generally an issue)
     "PLR",  # Pylint Recommendations (too many to fix all at once)
     "E722",  # bare-except (too many to fix all at once)
+    "PT012",  # pytest-raises-with-multiple-statements (too many to fix all at once)
     # Permanently disabled rules
     "UP006",  # non-pep585-annotation (not compatible with Python 3.7 or 3.8)
     "D203",  # incorrect-blank-line-before-class
@@ -144,6 +144,9 @@ ignore = [
     "PLR2004",  # magic-value-comparison (comparing to constant values)
     "ASYNC251",  # blocking-sleep-in-async-function (acceptable in tests)
     "B904",  # raise-without-from-inside-except (not necessary in tests)
+    "PT006",  # pytest-parametrize-names-wrong-type (style choice)
+    "PT007",  # pytest-parametrize-values-wrong-type (style choice)
+    "PT011",  # pytest-raises-too-broad (we frequently test for generic errors)
 ]
 "newrelic/admin/*" = [
     # Disabled rules in admin scripts

tests/agent_features/test_async_generator_trace.py

@@ -273,7 +273,7 @@ async def _test():
         await gen.asend(None)
 
         assert await gen.asend("foobar") == "foobar"
-        assert _received and _received[0] == "foobar"
+        assert _received[0] == "foobar"
 
         # finish consumption of the coroutine if necessary
         async for _ in gen:

tests/agent_features/test_configuration.py

@@ -47,7 +47,7 @@
 SKIP_IF_NOT_PY311 = pytest.mark.skipif(sys.version_info < (3, 11), reason="TOML not in the standard library.")
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def collector_available_fixture():
     # Disable fixture that requires real application to exist for this file
     pass

tests/agent_features/test_logs_in_context.py

@@ -241,7 +241,7 @@ def test_newrelic_logger_error_inside_transaction_with_stack_trace(log_buffer_wi
     assert filename.endswith("/test_logs_in_context.py")
     assert isinstance(line_number, int)
     assert isinstance(stack_trace, str)
-    assert stack_trace and stack_trace == expected_stack_trace
+    assert stack_trace == expected_stack_trace
 
     expected = {
         "entity.name": "Python Agent Test (agent_features)",
@@ -329,7 +329,7 @@ def test_newrelic_logger_error_outside_transaction_with_stack_trace(log_buffer_w
     assert filename.endswith("/test_logs_in_context.py")
     assert isinstance(line_number, int)
     assert isinstance(stack_trace, str)
-    assert stack_trace and stack_trace == expected_stack_trace
+    assert stack_trace == expected_stack_trace
 
     expected = {
         "entity.name": "Python Agent Test (agent_features)",

tests/agent_features/test_metric_normalization.py

@@ -40,7 +40,7 @@ def core_app(collector_agent_registration):
     return app._agent.application(app.name)
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def rules_engine_fixture(core_app):
     rules_engine = core_app._rules_engine
     previous_rules = rules_engine["metric"]

tests/agent_features/test_serverless_mode.py

@@ -28,7 +28,7 @@
 from newrelic.core.config import global_settings
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def serverless_application(request):
     settings = global_settings()
     orig = settings.serverless_mode.enabled

tests/agent_features/test_web_transaction.py

@@ -141,7 +141,7 @@ def test_base_web_transaction(use_bytes):
         transaction.process_response(200, response_headers)
 
 
-@pytest.fixture()
+@pytest.fixture
 def validate_no_garbage():
     yield
 

tests/agent_streaming/conftest.py

@@ -68,7 +68,7 @@ def wait(self, *args, **kwargs):
         return super().wait(*args, **kwargs)
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def buffer_empty_event(monkeypatch):
     event = threading.Event()
 
@@ -85,23 +85,23 @@ def batching(request):
     return request.param
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def spans_received():
     from _test_handler import SPANS_RECEIVED
 
     SPANS_RECEIVED.clear()
     return SPANS_RECEIVED
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def span_batches_received():
     from _test_handler import SPAN_BATCHES_RECEIVED
 
     SPAN_BATCHES_RECEIVED.clear()
     return SPAN_BATCHES_RECEIVED
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def spans_processed_event():
     from _test_handler import SPANS_PROCESSED_EVENT
 

tests/agent_streaming/test_infinite_tracing.py

@@ -31,7 +31,7 @@
 CONDITION_CLS = type(threading.Condition())
 
 
-@pytest.fixture()
+@pytest.fixture
 def app():
     app = Application("Python Agent Test (Infinite Tracing)")
     yield app

tests/agent_streaming/test_stream_buffer.py

@@ -29,7 +29,7 @@ def stop_iteration_condition(*args, **kwargs):
     return StopIterationOnWait(*args, **kwargs)
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def stop_iteration_on_wait(monkeypatch):
     monkeypatch.setattr(StreamBuffer, "condition", stop_iteration_condition)
 

tests/agent_unittests/conftest.py

@@ -44,7 +44,7 @@ class FakeProtos:
 sys.modules["newrelic.core.infinite_tracing_pb2"] = FakeProtos
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def global_settings(request, monkeypatch):
     ini_contents = request.getfixturevalue("ini")
 

tests/agent_unittests/test_encoding_utils.py

@@ -42,7 +42,6 @@ def test_camel_case(input_, expected, upper):
 @pytest.mark.parametrize(
     "input_,expected",
     [
-        ("", ""),
         ("", ""),
         ("my_string", "my_string"),
         ("myString", "my_string"),

tests/agent_unittests/test_package_version_utils.py

@@ -41,16 +41,16 @@
 SKIP_IF_NOT_PY310_PLUS = pytest.mark.skipif(not IS_PY310_PLUS, reason="These features were added in 3.10+")
 
 
-@pytest.fixture(scope="function", autouse=True)
+@pytest.fixture(autouse=True)
 def patched_pytest_module(monkeypatch):
     for attr in VERSION_ATTRS:
         if hasattr(pytest, attr):
             monkeypatch.delattr(pytest, attr)
 
-    yield pytest
+    return pytest
 
 
-@pytest.fixture(scope="function", autouse=True)
+@pytest.fixture(autouse=True)
 def cleared_package_version_cache():
     """Ensure cache is empty before every test to exercise code paths."""
     _get_package_version.cache_clear()
@@ -163,7 +163,8 @@ def version(self):
     monkeypatch.setattr(pytest, "version", FakeModule.version, raising=False)
 
     version = get_package_version("pytest")
-    assert version not in NULL_VERSIONS and isinstance(version, str), version
+    assert version not in NULL_VERSIONS, version
+    assert isinstance(version, str), version
 
 
 # This test checks to see if the version is a property of the class
@@ -181,4 +182,5 @@ def version(self):
     monkeypatch.setattr(pytest, "version_tuple", (1, 2, 3), raising=False)
 
     version = get_package_version("pytest")
-    assert version not in NULL_VERSIONS and isinstance(version, str), version
+    assert version not in NULL_VERSIONS, version
+    assert isinstance(version, str), version

tests/agent_unittests/test_sampler_metrics.py

@@ -46,7 +46,7 @@ def cpu_data_source():
 @pytest.fixture
 def memory_data_source():
     sampler = memory_usage_data_source(settings=())["factory"](environ=())
-    yield sampler
+    return sampler
 
 
 PID = os.getpid()

tests/agent_unittests/test_trace_cache.py

@@ -25,7 +25,7 @@ class DummyTrace:
     pass
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def trace_cache():
     return TraceCache()
 
@@ -54,7 +54,7 @@ def test_trace_cache_methods(trace_cache):
     assert len(list(trace_cache.values())) == 1
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def iterate_trace_cache(trace_cache):
     def _iterate_trace_cache(shutdown):
         while True:
@@ -70,7 +70,7 @@ def _iterate_trace_cache(shutdown):
     return _iterate_trace_cache
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def change_weakref_dict_size(trace_cache):
     def _change_weakref_dict_size(shutdown, obj_refs):
         """

tests/agent_unittests/test_wrappers.py

@@ -17,7 +17,7 @@
 from newrelic.common.object_wrapper import function_wrapper
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def wrapper():
     @function_wrapper
     def _wrapper(wrapped, instance, args, kwargs):
@@ -26,7 +26,7 @@ def _wrapper(wrapped, instance, args, kwargs):
     return _wrapper
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def wrapped_function(wrapper):
     @wrapper
     def wrapped():

tests/application_celery/conftest.py

@@ -42,4 +42,4 @@ def celery_worker_parameters():
 
 @pytest.fixture(scope="session", autouse=True)
 def celery_worker_available(celery_session_worker):
-    yield celery_session_worker
+    return celery_session_worker

tests/cross_agent/fixtures/ecs_container_id/ecs_mock_server.py

@@ -117,13 +117,13 @@ def bad_response_get(self):
     self.wfile.write(response)
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def mock_server():
     with MockExternalHTTPServer(handler=simple_get) as mock_server:
         yield mock_server
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def bad_response_mock_server():
     with MockExternalHTTPServer(handler=bad_response_get) as bad_response_mock_server:
         yield bad_response_mock_server

tests/cross_agent/fixtures/rum_loader_insertion_location/no_end_header.html

@@ -1,4 +1,4 @@
-<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
 <!--
  Copyright 2010 New Relic, Inc.
 
@@ -15,8 +15,10 @@
  limitations under the License.
 -->
 
-<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
-<head>EXPECTED_RUM_LOADER_LOCATION<body>
-    Cribbed from the Java agent
-  </body>
-</html>
+<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
+  <head>EXPECTED_RUM_LOADER_LOCATION
+    <body>
+      Cribbed from the Java agent
+    </body>
+  <!-- Missing end header tag -->
+</html>

tests/datastore_cassandradriver/conftest.py

@@ -40,7 +40,7 @@
 )
 
 
-@pytest.fixture(scope="function", params=["Libev", "AsyncCore", "Twisted"])
+@pytest.fixture(params=["Libev", "AsyncCore", "Twisted"])
 def connection_class(request):
     # Configure tests to run against a specific async reactor.
     reactor_name = request.param
@@ -62,7 +62,7 @@ def connection_class(request):
     return Connection
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def cluster_options(connection_class):
     from cassandra.cluster import ExecutionProfile
     from cassandra.policies import RoundRobinPolicy
@@ -77,12 +77,12 @@ def cluster_options(connection_class):
         "connection_class": connection_class,
         "protocol_version": 4,
     }
-    yield cluster_options
+    return cluster_options
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def cluster(cluster_options):
     from cassandra.cluster import Cluster
 
     cluster = Cluster(**cluster_options)
-    yield cluster
+    return cluster

tests/datastore_cassandradriver/test_cassandra.py

@@ -29,7 +29,7 @@
 REPLICATION_STRATEGY = "{ 'class' : 'SimpleStrategy', 'replication_factor' : 1 }"
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def exercise(cluster):
     def _exercise(is_async=False):
         with cluster.connect() as session:

tests/datastore_cassandradriver/test_cqlengine.py

@@ -41,7 +41,7 @@ class ABCModel(Model):
     b = columns.Double()
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def exercise(cluster_options):
     # Silence warning from cqlengine when creating tables
     os.environ["CQLENG_ALLOW_SCHEMA_MANAGEMENT"] = "true"

tests/datastore_elasticsearch/conftest.py

@@ -45,7 +45,7 @@
 RUN_IF_V7_OR_BELOW = pytest.mark.skipif(not IS_V7_OR_BELOW, reason="Unsupported for elasticsearch<=7")
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def client():
     from elasticsearch import Elasticsearch
 
@@ -54,7 +54,7 @@ def client():
     _client.close()
 
 
-@pytest.fixture(scope="function")
+@pytest.fixture
 def async_client(loop):
     from elasticsearch import AsyncElasticsearch