From d7d1f42b2c05175ff96c6c880631374fc38b4806 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Thu, 18 Sep 2025 15:43:22 +0100 Subject: [PATCH] Enable PyTest ruff checks --- .../picamera2/test_acquisition.py | 4 +-- .../picamera2/test_calibration.py | 4 +-- .../picamera2/test_tuning.py | 3 --- pyproject.toml | 4 ++- tests/test_camera.py | 6 ++++- tests/test_dummy_server.py | 2 +- tests/test_logging.py | 12 ++++----- tests/test_serve_static_files.py | 10 +++---- tests/test_server_config.py | 4 +-- tests/test_smart_scan.py | 26 +++++++++---------- tests/test_version_strings.py | 4 ++- 11 files changed, 42 insertions(+), 37 deletions(-) diff --git a/hardware-specific-tests/picamera2/test_acquisition.py b/hardware-specific-tests/picamera2/test_acquisition.py index 3ab404eb..caf8849a 100644 --- a/hardware-specific-tests/picamera2/test_acquisition.py +++ b/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. diff --git a/hardware-specific-tests/picamera2/test_calibration.py b/hardware-specific-tests/picamera2/test_calibration.py index 7c56a9c8..6c803aed 100644 --- a/hardware-specific-tests/picamera2/test_calibration.py +++ b/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. diff --git a/hardware-specific-tests/picamera2/test_tuning.py b/hardware-specific-tests/picamera2/test_tuning.py index b193131d..d2733b8f 100644 --- a/hardware-specific-tests/picamera2/test_tuning.py +++ b/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 diff --git a/pyproject.toml b/pyproject.toml index e2a439da..7ee7f85c 100644 --- a/pyproject.toml +++ b/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] diff --git a/tests/test_camera.py b/tests/test_camera.py index f3cfd1c7..2965b4d2 100644 --- a/tests/test_camera.py +++ b/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())) diff --git a/tests/test_dummy_server.py b/tests/test_dummy_server.py index 66230e1b..cf745764 100644 --- a/tests/test_dummy_server.py +++ b/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 diff --git a/tests/test_logging.py b/tests/test_logging.py index bdeec6d2..271939b5 100644 --- a/tests/test_logging.py +++ b/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, [], ["", ""]], - [FAKE_UVICORN_LOGGER.info, [""], [""]], - [FAKE_UVICORN_LOGGER.warning, [""], [""]], - [FAKE_UVICORN_LOGGER.error, [""], [""]], - [FAKE_UVICORN_LOGGER.exception, [""], [""]], + (FAKE_UVICORN_LOGGER.debug, [], ["", ""]), + (FAKE_UVICORN_LOGGER.info, [""], [""]), + (FAKE_UVICORN_LOGGER.warning, [""], [""]), + (FAKE_UVICORN_LOGGER.error, [""], [""]), + (FAKE_UVICORN_LOGGER.exception, [""], [""]), ], ) def test_uvicorn_error_only_says_error_on_error( diff --git a/tests/test_serve_static_files.py b/tests/test_serve_static_files.py index 8ec1a82c..9755cc1c 100644 --- a/tests/test_serve_static_files.py +++ b/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): diff --git a/tests/test_server_config.py b/tests/test_server_config.py index 565f02c6..3074d707 100644 --- a/tests/test_server_config.py +++ b/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.""" diff --git a/tests/test_smart_scan.py b/tests/test_smart_scan.py index ab619e8a..394778eb 100644 --- a/tests/test_smart_scan.py +++ b/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): diff --git a/tests/test_version_strings.py b/tests/test_version_strings.py index ea700618..e1719c33 100644 --- a/tests/test_version_strings.py +++ b/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):