Enable PyTest ruff checks

hardware-specific-tests/picamera2/test_acquisition.py

@@ -3,14 +3,14 @@
 from fastapi.testclient import TestClient
 from PIL import Image
 import numpy as np
-from pytest import fixture
+import pytest
 
 import labthings_fastapi as lt
 
 from openflexure_microscope_server.things.camera.picamera import StreamingPiCamera2
 
 
-@fixture(scope="module")
+@pytest.fixture(scope="module")
 def client() -> lt.ThingClient:
     """Initialise a test client for the StreamingPiCamera2 Thing.
 

hardware-specific-tests/picamera2/test_calibration.py

@@ -14,7 +14,7 @@ from openflexure_microscope_server.things.camera.picamera import (
 )
 
 
-@pytest.fixture()
+@pytest.fixture
 def picamera_thing() -> StreamingPiCamera2:
     """Return a StreamingPiCamera2 Thing.
 
@@ -24,7 +24,7 @@ def picamera_thing() -> StreamingPiCamera2:
     return StreamingPiCamera2()
 
 
-@pytest.fixture()
+@pytest.fixture
 def client(picamera_thing) -> lt.ThingClient:
     """Initialise a test client for the StreamingPiCamera2 Thing.
 

hardware-specific-tests/picamera2/test_tuning.py

@@ -68,9 +68,6 @@ def _test_bad_tuning_after_good_tuning(configure: bool = False):
     with pytest.raises(IndexError):
         # The bad version should cause a problem
         cam = Picamera2(tuning=bad_tuning)
-        print_tuning()
-        print("Success (not expected)!")
-        del cam
     recalibrate_utils.recreate_camera_manager()
     with Picamera2(tuning=default_tuning) as cam:
         # Reload the camera with working tuning, or it will stop responding

pyproject.toml

@@ -116,7 +116,7 @@ select = [
            # If they are not fixed before merge they should be converted to GitLab issues
     "LOG", # Flake8 logging issues (pedantic logger formatting issues can be added with "G")
     "T20", # Warns for print statements, production code should log
-#    "PT", # pytest linting
+    "PT", # pytest linting
     "RET", # Consistent clear return statements
     "RSE", # Raise parentheses
 #    "SIM", # Simplifications detected
@@ -140,6 +140,8 @@ ignore = [
     "PLR2004", # Ignore magic values in comparison for now. It doesn't seem we can set
                # allowed magic values and a number of our magic values are not really
                # magic (such as 255 when doing uint8 maths)
+    "PT011", # Ifnore pytest.raises being used on too gereneral expectations without a
+             # match. We may need to revisit this.
 ]
 
 [tool.ruff.lint.per-file-ignores]

tests/test_camera.py

@@ -54,7 +54,11 @@ def test_handle_broken_frame():
         portal = lt.get_blocking_portal(camera)
 
         # Check that this does cause broken frames.
-        with pytest.raises(OSError, match="broken data stream when reading image file"):
+        # The noqa is because we don't know exactly when the error is thrown so we
+        # can't have a single simple statements in the pytest raises.
+        with pytest.raises(  # noqa PT012
+            OSError, match="broken data stream when reading image file"
+        ):
             for _i in range(15):
                 jpeg = camera.grab_jpeg(portal)
                 np.asarray(Image.open(jpeg.open()))

tests/test_dummy_server.py

@@ -43,7 +43,7 @@ def thing_server():
     assert os.path.exists(os.path.join(temp_folder.name, "camera/"))
     # Note: yield is important. If return is used the temp folder gets deleted
     # before the test runs
-    yield server
+    yield server  # noqa: PT022
 
 
 @pytest.fixture

tests/test_logging.py

@@ -161,13 +161,13 @@ FAKE_UVICORN_LOGGER = logging.getLogger("uvicorn.error")
 
 
 @pytest.mark.parametrize(
-    "log_command, names_in_log, names_not_in_log",
+    ("log_command", "names_in_log", "names_not_in_log"),
     [
-        [FAKE_UVICORN_LOGGER.debug, [], ["<uvicorn>", "<uvicorn.error>"]],
-        [FAKE_UVICORN_LOGGER.info, ["<uvicorn>"], ["<uvicorn.error>"]],
-        [FAKE_UVICORN_LOGGER.warning, ["<uvicorn>"], ["<uvicorn.error>"]],
-        [FAKE_UVICORN_LOGGER.error, ["<uvicorn.error>"], ["<uvicorn>"]],
-        [FAKE_UVICORN_LOGGER.exception, ["<uvicorn.error>"], ["<uvicorn>"]],
+        (FAKE_UVICORN_LOGGER.debug, [], ["<uvicorn>", "<uvicorn.error>"]),
+        (FAKE_UVICORN_LOGGER.info, ["<uvicorn>"], ["<uvicorn.error>"]),
+        (FAKE_UVICORN_LOGGER.warning, ["<uvicorn>"], ["<uvicorn.error>"]),
+        (FAKE_UVICORN_LOGGER.error, ["<uvicorn.error>"], ["<uvicorn>"]),
+        (FAKE_UVICORN_LOGGER.exception, ["<uvicorn.error>"], ["<uvicorn>"]),
     ],
 )
 def test_uvicorn_error_only_says_error_on_error(

tests/test_serve_static_files.py

@@ -26,12 +26,12 @@ def mock_static_dir():
 
 
 @pytest.mark.parametrize(
-    "filename, allow_cache",
+    ("filename", "allow_cache"),
     [
-        ["foo", False],
-        ["woff2.foo", False],
-        ["strange_file_ending_in_woff2", False],
-        ["fontfile.woff2", True],
+        ("foo", False),
+        ("woff2.foo", False),
+        ("strange_file_ending_in_woff2", False),
+        ("fontfile.woff2", True),
     ],
 )
 def test_add_static_file(filename, allow_cache, mocker):

tests/test_server_config.py

@@ -86,8 +86,8 @@ def test_customise_server(mocker):
 
 
 @pytest.mark.parametrize(
-    "config_file, expected_scan_dir",
-    [[FULL_CONFIG, "/var/openflexure/scans/"], [SIM_CONFIG, "./openflexure/scans/"]],
+    ("config_file", "expected_scan_dir"),
+    [(FULL_CONFIG, "/var/openflexure/scans/"), (SIM_CONFIG, "./openflexure/scans/")],
 )
 def test_get_scans_dir_ok(config_file, expected_scan_dir):
     """Test the _get_scans_dir function and also check the standard config files."""

tests/test_smart_scan.py

@@ -120,20 +120,20 @@ def test_public_delete_scan(smart_scan_thing, caplog):
         # Attempt to delete the fake scan. Expect it to fail
     with pytest.raises(HTTPException) as exc_info:
         smart_scan_thing.delete_scan(fake_scan_name, LOGGER)
-        # Should raise a 400 error if the scan doesn't exist, not a 404 as the server
-        # was not expecting to receive the scan files
-        assert exc_info.value.status_code == 400
-        assert len(caplog.records) == 1
-        assert caplog.records[0].levelname == "WARNING"
-        assert caplog.records[0].name == "mock-invocation_logger"
+    # Should raise a 400 error if the scan doesn't exist, not a 404 as the server
+    # was not expecting to receive the scan files
+    assert exc_info.value.status_code == 400
+    assert len(caplog.records) == 1
+    assert caplog.records[0].levelname == "WARNING"
+    assert caplog.records[0].name == "mock-invocation_logger"
 
-        # Make a dir for the fake scan and delete it.
-        os.makedirs(fake_scan_path)
-        assert os.path.exists(fake_scan_path)
-        smart_scan_thing.delete_scan(fake_scan_name, LOGGER)
-        assert not os.path.exists(fake_scan_path)
-        # Check no extra logs generated
-        assert len(caplog.records) == 1
+    # Make a dir for the fake scan and delete it.
+    os.makedirs(fake_scan_path)
+    assert os.path.exists(fake_scan_path)
+    smart_scan_thing.delete_scan(fake_scan_name, LOGGER)
+    assert not os.path.exists(fake_scan_path)
+    # Check no extra logs generated
+    assert len(caplog.records) == 1
 
 
 def test_delete_all_scans(smart_scan_thing, caplog):

tests/test_version_strings.py

@@ -82,7 +82,9 @@ def git_repo(temp_dir):
     _git("add -A")
     _git("commit -m 'message2'")
 
-    yield temp_dir
+    # Yielding here as temdir is yielding and we want to stay in the tempdir
+    # context manager. Silencing ruff saying it should be return.
+    yield temp_dir  # noqa: PT022
 
 
 def test_version_data_git_and_toml(mocker, git_repo):