From 52293ba948c1b86ecde4efc413b16b7201a293dc Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 24 Dec 2025 15:56:33 +0000 Subject: [PATCH 1/6] Normalise expected paths in windows tests --- tests/unit_tests/test_config_utilities.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/unit_tests/test_config_utilities.py b/tests/unit_tests/test_config_utilities.py index 1d0aa408..cd72b7d0 100644 --- a/tests/unit_tests/test_config_utilities.py +++ b/tests/unit_tests/test_config_utilities.py @@ -53,13 +53,15 @@ def test_merge_patch(target, patch, outcome, err_if_enforce): @pytest.mark.parametrize( ("base_conf", "resolves_to"), [ - ("/var/base.json", "/var/base.json"), - ("base.json", "/var/openflexure/settings/base.json"), - ("./base.json", "/var/openflexure/settings/base.json"), - ("../base.json", "/var/openflexure/base.json"), + ("/var/base.json", os.path.normpath("/var/base.json")), + ("base.json", os.path.normpath("/var/openflexure/settings/base.json")), + ("./base.json", os.path.normpath("/var/openflexure/settings/base.json")), + ("../base.json", os.path.normpath("/var/openflexure/base.json")), ( "$OFM_LIB/base.json", - "/var/openflexure/application/openflexure-microscope-server/base.json", + os.path.normpath( + "/var/openflexure/application/openflexure-microscope-server/base.json" + ), ), ], ) From 00a0301e0c8720ee417dc378c3ff52648cd808cb Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 24 Dec 2025 15:57:12 +0000 Subject: [PATCH 2/6] Close file logging hander in unit tests before tmpdir cleanup --- tests/unit_tests/test_logging.py | 37 +++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/tests/unit_tests/test_logging.py b/tests/unit_tests/test_logging.py index d9509f8e..56de711f 100644 --- a/tests/unit_tests/test_logging.py +++ b/tests/unit_tests/test_logging.py @@ -3,6 +3,8 @@ import logging import os import tempfile +from contextlib import contextmanager +from logging.handlers import RotatingFileHandler import pytest from fastapi import HTTPException @@ -11,6 +13,25 @@ from fastapi.responses import PlainTextResponse from openflexure_microscope_server import logging as ofm_logging +@contextmanager +def tmp_logging_dir(): + """Yield a temporary logging dir, and delete rotating file handlers before closing. + + This is needed for unit tests on Windows for the temp dir to cleanup without an + error. + """ + with tempfile.TemporaryDirectory() as tmpdir: + yield tmpdir + root_logger = logging.getLogger() + # Get the rotating file handlers. There should only be 1 (or none if mocked) + file_handlers = [ + h for h in root_logger.handlers if isinstance(h, RotatingFileHandler) + ] + for handler in file_handlers: + handler.close() + root_logger.removeHandler(handler) + + def test_no_warnings_if_correct_permissions(caplog): """Check that configure_logging no warnings on normal operation. @@ -22,7 +43,7 @@ def test_no_warnings_if_correct_permissions(caplog): """ # Reset handler at start of test ofm_logging.OFM_HANDLER = ofm_logging.OFMHandler() - with caplog.at_level(logging.WARNING), tempfile.TemporaryDirectory() as tmpdir: + with caplog.at_level(logging.WARNING), tmp_logging_dir() as tmpdir: ofm_logging.configure_logging(tmpdir) assert len(caplog.records) == 0 with open(ofm_logging.OFM_LOG_FILE, "r", encoding="utf-8") as log_file: @@ -40,7 +61,7 @@ def test_permission_error_raises_warning(mocker, caplog): ) # Reset handler at start of test ofm_logging.OFM_HANDLER = ofm_logging.OFMHandler() - with caplog.at_level(logging.WARNING), tempfile.TemporaryDirectory() as tmpdir: + with caplog.at_level(logging.WARNING), tmp_logging_dir() as tmpdir: ofm_logging.configure_logging(tmpdir) assert len(caplog.records) == 1 # Check OFM logger is added even if the file logger couldn't be. @@ -52,7 +73,7 @@ def test_making_log_dir(caplog): """Check that configure_logging will make a dir if needed.""" # Reset handler at start of test ofm_logging.OFM_HANDLER = ofm_logging.OFMHandler() - with caplog.at_level(logging.WARNING), tempfile.TemporaryDirectory() as tmpdir: + with caplog.at_level(logging.WARNING), tmp_logging_dir() as tmpdir: log_dir = os.path.join(tmpdir, "new_dir") assert not os.path.isdir(log_dir) ofm_logging.configure_logging(log_dir) @@ -64,7 +85,7 @@ def test_max_logs(): """Proclaim that only the most recent 250 logs are stored in memory.""" # Reset handler at start of test ofm_logging.OFM_HANDLER = ofm_logging.OFMHandler() - with tempfile.TemporaryDirectory() as tmpdir: + with tmp_logging_dir() as tmpdir: ofm_logging.configure_logging(tmpdir) # But I would log five hundred times. for i in range(500): @@ -90,7 +111,7 @@ def test_ofm_handler_ignores_uvicorn_access(): """ # Reset handler at start of test ofm_logging.OFM_HANDLER = ofm_logging.OFMHandler() - with tempfile.TemporaryDirectory() as tmpdir: + with tmp_logging_dir() as tmpdir: ofm_logging.configure_logging(tmpdir) logs = ofm_logging.OFM_HANDLER.log_history.split("\n") starting_len = len(logs) @@ -114,7 +135,7 @@ def test_server_responses(): """ # Reset handler at start of test ofm_logging.OFM_HANDLER = ofm_logging.OFMHandler() - with tempfile.TemporaryDirectory() as tmpdir: + with tmp_logging_dir() as tmpdir: ofm_logging.configure_logging(tmpdir) for i in range(500): logging.info("log %s", i) @@ -142,7 +163,7 @@ def test_server_response_with_no_log_file(): def test_server_response_with_no_log_dir(): """Check that an HTTP exception is raised if the log file cannot be accessed.""" ofm_logging.OFM_HANDLER = ofm_logging.OFMHandler() - with tempfile.TemporaryDirectory() as tmpdir: + with tmp_logging_dir() as tmpdir: ofm_logging.configure_logging(tmpdir) for i in range(500): logging.info("log %s", i) @@ -176,7 +197,7 @@ def test_uvicorn_error_only_says_error_on_error( error logs as . """ ofm_logging.OFM_HANDLER = ofm_logging.OFMHandler() - with tempfile.TemporaryDirectory() as tmpdir: + with tmp_logging_dir() as tmpdir: ofm_logging.configure_logging(tmpdir) log_command("Mockety mock mock!") with open(ofm_logging.OFM_LOG_FILE, "r", encoding="utf-8") as log_file: From ec29a1999f39fc6091b3a5043ac45e4dcd86b4b8 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 24 Dec 2025 17:05:10 +0000 Subject: [PATCH 3/6] Update how stitching is called in Windows --- src/openflexure_microscope_server/stitching.py | 11 +++++++++-- tests/unit_tests/test_stitching.py | 1 + 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/openflexure_microscope_server/stitching.py b/src/openflexure_microscope_server/stitching.py index 8eb7be1e..e505971f 100644 --- a/src/openflexure_microscope_server/stitching.py +++ b/src/openflexure_microscope_server/stitching.py @@ -18,6 +18,8 @@ import labthings_fastapi as lt from openflexure_microscope_server.utilities import make_path_safe +IS_WINDOWS = os.name == "nt" + STITCHING_CMD = "openflexure-stitch" STITCHING_RESOLUTION = (820, 616) STITCH_TILE_SIZE = 8192 @@ -95,7 +97,8 @@ class BaseStitcher: self.validate_path() # The command, and the mode - initial_args = shlex.split(STITCHING_CMD) + ["--stitching_mode", self._mode] + base_args = shlex.split(STITCHING_CMD, posix=not IS_WINDOWS) + initial_args = base_args + ["--stitching_mode", self._mode] # Use float() just to really ensure that anything input is a float setting_args = [ "--minimum_overlap", @@ -181,7 +184,11 @@ class PreviewStitcher(BaseStitcher): except lt.exceptions.InvocationCancelledError as e: with self._popen_lock: if self._popen_obj is not None: - self._popen_obj.send_signal(signal.SIGKILL) + if IS_WINDOWS: + # Windows has no SIGKILL + self._popen_obj.kill() + else: + self._popen_obj.send_signal(signal.SIGKILL) raise (e) diff --git a/tests/unit_tests/test_stitching.py b/tests/unit_tests/test_stitching.py index 31918f41..12bcfbfb 100644 --- a/tests/unit_tests/test_stitching.py +++ b/tests/unit_tests/test_stitching.py @@ -235,6 +235,7 @@ class StitchingTestThing(lt.Thing): """Run the final stitcher.""" stitcher = FinalStitcher(FAKE_DIR, logger=self.logger) # Send in the argument HANG to mock-stitch and it just hang for 10s + stitcher._extra_args = ["HANG"] stitcher.run() From 3e8511f281ad684d50492419148e87e87836e51a Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 24 Dec 2025 17:25:07 +0000 Subject: [PATCH 4/6] Use Python not echo as a mock shutdown command for windows compatibility --- tests/unit_tests/test_system_thing.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/unit_tests/test_system_thing.py b/tests/unit_tests/test_system_thing.py index 4754c8bb..fa1729f7 100644 --- a/tests/unit_tests/test_system_thing.py +++ b/tests/unit_tests/test_system_thing.py @@ -97,12 +97,14 @@ def test_pi_shutdown(mocker): # Check the shutdown command is as expected. assert system.SHUTDOWN_CMD == ["sudo", "shutdown", "-h", "now"] # Mock the shutdown command as we don't want to shutdown when running tests. - mocker.patch.object(system, "SHUTDOWN_CMD", new=["echo", "shutdown"]) + mocker.patch.object( + system, "SHUTDOWN_CMD", new=["python", "-c", "print('shutdown')"] + ) # Call shutdown on a MockPiSystem system_thing = create_thing_without_server(MockPiSystem) result = system_thing.shutdown() - # Check the result of the echo mock command was returned + # Check the result of the mock command was returned assert result.output.strip() == "shutdown" assert result.error.strip() == "" @@ -112,12 +114,12 @@ def test_pi_reboot(mocker): # Check the reboot command is as expected. assert system.REBOOT_CMD == ["sudo", "shutdown", "-r", "now"] # Mock the reboot command as we don't want to reboot when running tests. - mocker.patch.object(system, "REBOOT_CMD", new=["echo", "restart"]) + mocker.patch.object(system, "REBOOT_CMD", new=["python", "-c", "print('restart')"]) # Call reboot on a MockPiSystem system_thing = create_thing_without_server(MockPiSystem) result = system_thing.reboot() - # Check the result of the echo mock command was returned + # Check the result of the mock command was returned assert result.output.strip() == "restart" assert result.error.strip() == "" From d6b00ef1bb1e3479053075382e7526b66189e683 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 24 Dec 2025 17:47:20 +0000 Subject: [PATCH 5/6] Change out of temp dirs in git version tests before closing so windows doesn't error --- tests/unit_tests/test_version_strings.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/unit_tests/test_version_strings.py b/tests/unit_tests/test_version_strings.py index fc8fd817..6c4f935b 100644 --- a/tests/unit_tests/test_version_strings.py +++ b/tests/unit_tests/test_version_strings.py @@ -63,8 +63,10 @@ def temp_dir(): with tempfile.TemporaryDirectory() as tmpdir: os.chdir(tmpdir) yield tmpdir + # Change back to working dir before closing context manager or Windows will error + os.chdir(working_dir) finally: - # Return to original working dir + # Ensure changed to working dir os.chdir(working_dir) @@ -191,8 +193,10 @@ def test_reading_hash_from_git(): # Check out the branch and check commit changed back _git(f"checkout {branch_name}") assert utilities._get_hash_from_git_dir(git_dir) == git_hash2 + # Change back to working dir before closing context manager or Windows will error + os.chdir(working_dir) finally: - # Return to original working dir + # Ensure changed to working dir os.chdir(working_dir) From a15458c02d0b62f0f1d6b1e1169d3d37d30d799a Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Mon, 12 Jan 2026 21:40:58 +0000 Subject: [PATCH 6/6] Apply suggestions from code review of branch Windows-Testing Co-authored-by: Richard Bowman --- tests/unit_tests/test_version_strings.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/unit_tests/test_version_strings.py b/tests/unit_tests/test_version_strings.py index 6c4f935b..f71e59b8 100644 --- a/tests/unit_tests/test_version_strings.py +++ b/tests/unit_tests/test_version_strings.py @@ -59,15 +59,13 @@ def _git(command: str) -> str: def temp_dir(): """Return the path of a temporary directory (set as working dir).""" working_dir = os.getcwd() - try: - with tempfile.TemporaryDirectory() as tmpdir: + with tempfile.TemporaryDirectory() as tmpdir: + try: os.chdir(tmpdir) yield tmpdir + finally: # Change back to working dir before closing context manager or Windows will error os.chdir(working_dir) - finally: - # Ensure changed to working dir - os.chdir(working_dir) @pytest.fixture