From d19d44ba4f44f2b65e7e616c5dd032c125ca9aa9 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Thu, 10 Jul 2025 18:52:17 +0100 Subject: [PATCH 01/11] Remove unused complex functionality from settings_manager.py. --- .../things/settings_manager.py | 99 +------------------ 1 file changed, 3 insertions(+), 96 deletions(-) diff --git a/src/openflexure_microscope_server/things/settings_manager.py b/src/openflexure_microscope_server/things/settings_manager.py index 8ae47a8f..aecc6631 100644 --- a/src/openflexure_microscope_server/things/settings_manager.py +++ b/src/openflexure_microscope_server/things/settings_manager.py @@ -7,42 +7,12 @@ the server. from collections.abc import Mapping from socket import gethostname -from typing import Annotated, Any, MutableMapping, Optional, Sequence +from typing import Optional from uuid import UUID, uuid4 -from fastapi import Depends, HTTPException, Request + import labthings_fastapi as lt -def thing_server_from_request(request: Request) -> lt.ThingServer: - """Wrap lt.find_thing_server.""" - return lt.find_thing_server(request.app) - - -ThingServerDep = Annotated[lt.ThingServer, Depends(thing_server_from_request)] - - -def recursive_update(old_dict: MutableMapping, update: Mapping): - """Update a dictionary recursively.""" - for k, v in update.items(): - if isinstance(v, Mapping): - if k in old_dict and isinstance(old_dict[k], MutableMapping): - recursive_update(old_dict[k], v) - else: - old_dict[k] = dict(**v) - else: - old_dict[k] = v - - -def nested_dict_get(data: dict, key: Sequence[str], create=False) -> Any: - """Index a dict-of-dicts with a sequence of strings.""" - subdict = data - for k in key: - if k not in subdict and create: - subdict[k] = {} - subdict = subdict[k] - return subdict - - class SettingsManager(lt.Thing): """Provides functionality to other Things about the current server state. @@ -50,59 +20,6 @@ class SettingsManager(lt.Thing): and the state of other Things. """ - external_metadata = lt.ThingSetting( - initial_value={}, - model=Mapping, - ) - """External metadata stored in the server's settings.""" - - external_metadata_in_state = lt.ThingSetting( - initial_value=[], - model=Sequence[str], - ) - """Keys from ``external_metadata`` that are included in the "state" metadata.""" - - @lt.thing_action - def update_external_metadata( - self, data: Mapping, key: Optional[str] = None - ) -> None: - """Add or replace keys in the external metadata. - - The data supplied will be merged into the existing dictionary - recursively, i.e. if a key exists, it will be added to rather than - replaced. - - If a key is supplied, we will treat ``data`` as being relative to that - key, i.e. calling this action with ``data={"a":1 }, key="foo/bar"`` is - equivalent to calling it with ``data={"foo": {"bar": {"a": 1}}}``. - """ - metadata = self.external_metadata - subdict = metadata - # If we're operating only on a part of the dict, make sure - # that key exists, and find it. - if key: - subdict = nested_dict_get(subdict, key.split("/"), create=True) - recursive_update(subdict, data) - self.external_metadata = metadata - - @lt.thing_action - def delete_external_metadata(self, key: str) -> None: - """Delete a key from the stored metadata. - - The key may contain forward slashes, which are understood to separate - levels of the dictionary - i.e. ``'a/c'`` will remove the ``c`` key from a - dictionary that looks like: ``{'a': {'c': 1}, 'b': {'d': 2}}`` - """ - metadata = self.external_metadata - try: - subdict = nested_dict_get(metadata, key.split("/")[:-1]) - del subdict[key.split("/")[-1]] - except KeyError: - raise HTTPException( - status_code=404, detail="The specified key '{key}' was not found" - ) - self.external_metadata = metadata - _microscope_id: Optional[str] = None @lt.thing_setting @@ -130,17 +47,7 @@ class SettingsManager(lt.Thing): @property def thing_state(self) -> Mapping: """Summary metadata describing the current state of the Thing.""" - state = { + return { "hostname": self.hostname, "microscope-uuid": str(self.microscope_id), } - metadata = self.external_metadata - for k in self.external_metadata_in_state: - try: - kl = k.split("/") - v = nested_dict_get(metadata, kl) - subdict = nested_dict_get(state, kl[:-1], create=True) - subdict[kl[-1]] = v - except KeyError: - continue - return state From 1dd0db888db9dd5a4512e397659defb778d49073 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Fri, 11 Jul 2025 00:13:09 +0100 Subject: [PATCH 02/11] Starting to create version reading code --- .../utilities.py | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/src/openflexure_microscope_server/utilities.py b/src/openflexure_microscope_server/utilities.py index aceb7eac..69a6b2a0 100644 --- a/src/openflexure_microscope_server/utilities.py +++ b/src/openflexure_microscope_server/utilities.py @@ -1,6 +1,19 @@ """Utility functions and classes.""" +import os +import re from threading import Thread +import logging +from importlib.metadata import version +import tomllib + +LOGGER = logging.getLogger(__name__) + +REPO_DIR = os.path.dirname(os.path.dirname(os.path.dirname(__file__))) +# Regex for a full commit hash +COMMIT_REGEX = re.compile(r"[0-9a-f]{40}") +# Regex for a reference in the git HEAD file. Group 1 is the path. +REF_REGEX = re.compile(r"^ref:\s(.*)$") class ErrorCapturingThread(Thread): @@ -74,3 +87,73 @@ def _wrap_and_catch_errors(target, error_buffer, *args, **kwargs): target(*args, **kwargs) except BaseException as e: error_buffer.append(e) + + +def robust_version_strings() -> tuple[str, str]: + project_toml_path = os.path.join(REPO_DIR, "pyproject.toml") + git_dir_path = os.path.join(REPO_DIR, ".git") + has_project_toml = os.path.isfile(project_toml_path) + has_git_dir = os.path.isdir(git_dir_path) + + if has_project_toml and has_git_dir: + # .git dir and pyproject.toml available. This is a development + # installation. + ver = "v" + _get_version_from_toml(project_toml_path) + source = _get_hash_from_git_dir(git_dir_path) + elif has_project_toml: + # Only pyproject.toml available. Likely installed from source. Check + # file directly as `importlib.metadata.version` plays badly with editable + # installs. + ver = "v" + _get_version_from_toml(project_toml_path) + source = "TOML" + elif has_git_dir: + # Only .git dir, that is weird. + LOGGER.warning( + "Unexpected installation cofiguration. Version number cannot be verified." + ) + ver = "Undefined" + source = _get_hash_from_git_dir(git_dir_path) + else: + # "Neither probably installed from wheel. Note this can + # be unreliable if the package is not installed as a distribution. + # This is why it is the last option. + ver = "v" + version("openflexure_microscope_server") + source = "Dist" + return (ver, source) + + +def _get_hash_from_git_dir(git_dir_path: str) -> str: + head_path = os.path.join(git_dir_path, "HEAD") + try: + with open(head_path, "r", encoding="utf-8") as head_file: + head = head_file.read() + except IOError: + LOGGER.error("Problem opening .git/HEAD") + return "Undefined" + # The file HEAD should either be a reference or commit ID. + if match := COMMIT_REGEX.match(head): + return match.group(0) + if match := REF_REGEX.match(head): + ref_path = os.path.join(git_dir_path, match.group(1)) + return _get_hash_from_git_ref(ref_path) + LOGGER.error("Unexpected format for .git/HEAD") + return "Undefined" + + +def _get_hash_from_git_ref(git_ref_path: str) -> str: + with open(git_ref_path, "r", encoding="utf-8") as ref_file: + ref = ref_file.read() + if ref_match := COMMIT_REGEX.match(ref): + return ref_match.group(0) + LOGGER.error("No hash found in Git ref") + return "Undefined" + + +def _get_version_from_toml(toml_path: str) -> str: + try: + with open(toml_path, "rb") as toml_file: + toml_dict = tomllib.load(toml_file) + return toml_dict["project"]["version"] + except (IOError, ValueError, KeyError): + LOGGER.error("Problem opening .git/HEAD") + return "Undefined" From 608c4bd16fada7c97519f902f7facf36a5b272e0 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Fri, 11 Jul 2025 09:59:41 +0100 Subject: [PATCH 03/11] Expose version as thing property and via thing state so it ends up in metadata. --- .../things/settings_manager.py | 21 +++++ .../utilities.py | 86 +++++++++++++++++-- 2 files changed, 101 insertions(+), 6 deletions(-) diff --git a/src/openflexure_microscope_server/things/settings_manager.py b/src/openflexure_microscope_server/things/settings_manager.py index aecc6631..df0e144d 100644 --- a/src/openflexure_microscope_server/things/settings_manager.py +++ b/src/openflexure_microscope_server/things/settings_manager.py @@ -12,6 +12,8 @@ from uuid import UUID, uuid4 import labthings_fastapi as lt +from openflexure_microscope_server.utilities import VersionData, robust_version_strings + class SettingsManager(lt.Thing): """Provides functionality to other Things about the current server state. @@ -39,6 +41,23 @@ class SettingsManager(lt.Thing): """The hostname of the microscope, as reported by its operating system.""" return gethostname() + _version_data: Optional[VersionData] = None + + @lt.thing_property + def version_data(self) -> VersionData: + """The version string and version source for the server. + + The source may be a commit hash if installed from git. "TOML" if installed from + source, or "Dist" if installed from a distribution package. + + If the version or its source cannot be determined an error will be Logged. + """ + # Don't save as a setting as it may change. Get the version string the first + # time the property is requested this boot. + if self._version_data is None: + self._version_data = robust_version_strings() + return self._version_data + @lt.thing_action def get_things_state(self, metadata_getter: lt.deps.GetThingStates) -> Mapping: """Metadata summarising the current state of all Things in the server.""" @@ -50,4 +69,6 @@ class SettingsManager(lt.Thing): return { "hostname": self.hostname, "microscope-uuid": str(self.microscope_id), + "version": self.version_data.version, + "version_source": self.version_data.version_source, } diff --git a/src/openflexure_microscope_server/utilities.py b/src/openflexure_microscope_server/utilities.py index 69a6b2a0..339c777b 100644 --- a/src/openflexure_microscope_server/utilities.py +++ b/src/openflexure_microscope_server/utilities.py @@ -7,6 +7,8 @@ import logging from importlib.metadata import version import tomllib +from pydantic import BaseModel + LOGGER = logging.getLogger(__name__) REPO_DIR = os.path.dirname(os.path.dirname(os.path.dirname(__file__))) @@ -89,7 +91,43 @@ def _wrap_and_catch_errors(target, error_buffer, *args, **kwargs): error_buffer.append(e) +class VersionData(BaseModel): + """A BaseModel containing the information about the server version. + + :param version: The version string for the server. Or "Undefined" if there is an + error. + :param version_source: Either a Git commit hash, "TOML", "Dist" or in the case of + an error - "Undefined". + """ + + version: str + version_source: str + + def robust_version_strings() -> tuple[str, str]: + """Return a version string and information on its source. + + Returning a python version string is not without problems. If the package is + installed in an editable way, often METDATA is never updated. This provides 4 + ways to provide a version string: + + 1. If both a Git directory and a pyproject.toml are available. Return the version + string from the toml file and the git hash from the ``.git`` directory as its + source. + 2. If a pyproject.toml is available but no Git directory then this is likely an + installation from source. Return the version string from the toml file and + return the literal string "TOML" as its source. + 3. If a Git directory is available but no pyproject.toml then something is very + weird log an error. Then return "Undefined" as the version string, but still + return the Git hash as the source. + 4. Finally if neither a Git directory nor a pyproject.toml are available then the + server has been installed from a distribution package (i.e. a wheel). In this + case use the standard library function ``importlib.metadata.version`` for the + version string (prepending a "v") and return "Dist" as its source. + + :returns: a VersionData instance with the version string and source. + + """ project_toml_path = os.path.join(REPO_DIR, "pyproject.toml") git_dir_path = os.path.join(REPO_DIR, ".git") has_project_toml = os.path.isfile(project_toml_path) @@ -108,8 +146,8 @@ def robust_version_strings() -> tuple[str, str]: source = "TOML" elif has_git_dir: # Only .git dir, that is weird. - LOGGER.warning( - "Unexpected installation cofiguration. Version number cannot be verified." + LOGGER.error( + "Unexpected installation configuration. Version number cannot be verified." ) ver = "Undefined" source = _get_hash_from_git_dir(git_dir_path) @@ -119,10 +157,27 @@ def robust_version_strings() -> tuple[str, str]: # This is why it is the last option. ver = "v" + version("openflexure_microscope_server") source = "Dist" - return (ver, source) + return VersionData(version=ver, version_source=source) def _get_hash_from_git_dir(git_dir_path: str) -> str: + """Get the commit hash from a .git directory directly. + + This function doesn't rely on GIT being on the PATH or even installed. It doesn't + require any packages outside the Python standard library. It directly inspects the + .git directory to get the commit hash: + + * If the repository is on a detached HEAD then the hash will be in the file + .git/HEAD. A detached HEAD is could be from checking out a tag or checking out + a commit directly. + * In normal operation .git/HEAD points to a reference or "ref". This ref file + contains the hash. + + :param git_dir_path: The path to the .git directory. + + :returns: The hash, or "Undefined" if there is any error, errors are logged not + raised. + """ head_path = os.path.join(git_dir_path, "HEAD") try: with open(head_path, "r", encoding="utf-8") as head_file: @@ -141,8 +196,21 @@ def _get_hash_from_git_dir(git_dir_path: str) -> str: def _get_hash_from_git_ref(git_ref_path: str) -> str: - with open(git_ref_path, "r", encoding="utf-8") as ref_file: - ref = ref_file.read() + """Get the commit hash from a ref in the .git directory. + + For more detail see `_get_hash_from_git_dir` + + :param git_ref_path: The full path to the ref file in the .git directory. + + :returns: The hash, or "Undefined" if there is any error, errors are logged not + raised. + """ + try: + with open(git_ref_path, "r", encoding="utf-8") as ref_file: + ref = ref_file.read() + except IOError: + LOGGER.error("Problem opening Git branch reference.") + return "Undefined" if ref_match := COMMIT_REGEX.match(ref): return ref_match.group(0) LOGGER.error("No hash found in Git ref") @@ -150,10 +218,16 @@ def _get_hash_from_git_ref(git_ref_path: str) -> str: def _get_version_from_toml(toml_path: str) -> str: + """Get the version string from a pyproject.toml file. + + :param toml_path: The path to the toml file. + + :returns: The version string directly as reported in the toml file. + """ try: with open(toml_path, "rb") as toml_file: toml_dict = tomllib.load(toml_file) return toml_dict["project"]["version"] except (IOError, ValueError, KeyError): - LOGGER.error("Problem opening .git/HEAD") + LOGGER.error("Problem opening pyproject.toml") return "Undefined" From 83a4b72507403229346a6cbf02836c241cd3b4f9 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Fri, 11 Jul 2025 11:57:40 +0100 Subject: [PATCH 04/11] Start writing tests for the version strings --- tests/test_version_strings.py | 77 +++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 tests/test_version_strings.py diff --git a/tests/test_version_strings.py b/tests/test_version_strings.py new file mode 100644 index 00000000..dff89565 --- /dev/null +++ b/tests/test_version_strings.py @@ -0,0 +1,77 @@ +"""Tests that version strings can be produced reliably.""" + +import subprocess +import os +import re +import tempfile + +from hypothesis import strategies as st + +from openflexure_microscope_server import utilities + + +# This is the regex provided by https://semver.org/ +SEMVER_REGEX = re.compile( + r"^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)" + r"(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)" + r"(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?" + r"(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$" +) + +# Use hypothesis to generate random filenames +FILE_NAME_GEN = st.from_regex(r"[a-zA-Z_-]{5,20}\.[a-z]{1,5}", fullmatch=True) +# Use hypothesis to generate random data for files +FILE_Data_GEN = st.from_regex(r"[a-zA-Z_-]{5,20}\.[a-z]{1,5}", fullmatch=True) + + +def _git(command: str) -> str: + """Run a git command in a subprocess.""" + git_command = ["git"] + command.split() + # Use check=True so an error is raised if git has a problem. Only read STDOUT + result = subprocess.run(git_command, shell=True, check=True, capture_output=True) + return result.stdout.decode() + + +def test_reading_hash_from_git(): + """Use create a Git repo and check that hash can be read. + + The performs a series of directory and git operations, commented as they go. + """ + working_dir = os.getcwd() + try: + with tempfile.TemporaryDirectory() as tmpdir: + os.chdir(tmpdir) + git_dir = os.path.join(tmpdir, ".git") + # Git dir not created yet. Returns Undefined + assert utilities._get_hash_from_git_dir(git_dir) == "Undefined" + + # Intialised. Should still be undefined. + _git("init") + assert utilities._get_hash_from_git_dir(git_dir) == "Undefined" + + finally: + # Return to original working dir + os.chdir(working_dir) + + +def test_reading_version_from_toml(): + """Check version string can be read from this project's TOML file. + + As we only expect to ever read our own TOML file there is not a need to do + excessive testing on different generated TOML files. + """ + project_toml_path = os.path.join(utilities.REPO_DIR, "pyproject.toml") + ver_string = utilities._get_version_from_toml(project_toml_path) + # Check version does follow semantic versioning. + assert SEMVER_REGEX.match(ver_string) + # Explicit check + assert ver_string == "3.0.0-alpha1" + + +def test_error_reading_version_from_toml(): + """Check Undefined is returned if error reading TOML file.""" + # Just give it the wrong path. + project_toml_path = os.path.join(utilities.REPO_DIR, "pyppproject.toml") + ver_string = utilities._get_version_from_toml(project_toml_path) + # Explicit check + assert ver_string == "Undefined" From 5c1f8f2e89b9e06eb22973804f96864c7fe0f8ed Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Fri, 11 Jul 2025 16:59:00 +0100 Subject: [PATCH 05/11] Write tests for checking git hash --- .../utilities.py | 4 +- tests/test_version_strings.py | 133 +++++++++++++++++- 2 files changed, 131 insertions(+), 6 deletions(-) diff --git a/src/openflexure_microscope_server/utilities.py b/src/openflexure_microscope_server/utilities.py index 339c777b..13d6ee23 100644 --- a/src/openflexure_microscope_server/utilities.py +++ b/src/openflexure_microscope_server/utilities.py @@ -97,7 +97,7 @@ class VersionData(BaseModel): :param version: The version string for the server. Or "Undefined" if there is an error. :param version_source: Either a Git commit hash, "TOML", "Dist" or in the case of - an error - "Undefined". + an error - "Undefined". """ version: str @@ -108,7 +108,7 @@ def robust_version_strings() -> tuple[str, str]: """Return a version string and information on its source. Returning a python version string is not without problems. If the package is - installed in an editable way, often METDATA is never updated. This provides 4 + installed in an editable way, often METADATA is never updated. This provides 4 ways to provide a version string: 1. If both a Git directory and a pyproject.toml are available. Return the version diff --git a/tests/test_version_strings.py b/tests/test_version_strings.py index dff89565..c0e1df18 100644 --- a/tests/test_version_strings.py +++ b/tests/test_version_strings.py @@ -4,8 +4,13 @@ import subprocess import os import re import tempfile +import shutil +import warnings + from hypothesis import strategies as st +from hypothesis.errors import NonInteractiveExampleWarning +import pytest from openflexure_microscope_server import utilities @@ -21,15 +26,27 @@ SEMVER_REGEX = re.compile( # Use hypothesis to generate random filenames FILE_NAME_GEN = st.from_regex(r"[a-zA-Z_-]{5,20}\.[a-z]{1,5}", fullmatch=True) # Use hypothesis to generate random data for files -FILE_Data_GEN = st.from_regex(r"[a-zA-Z_-]{5,20}\.[a-z]{1,5}", fullmatch=True) +FILE_DATA_GEN = st.from_regex(r"[a-zA-Z_-]{5,20}\.[a-z]{1,5}", fullmatch=True) + + +def create_fake_file() -> None: + """Create a file with a random name and random content in the working directory.""" + with warnings.catch_warnings(): + # Ignore hypothesis warning as we are not using for hypothesis testing but just + # as a convenient way to create random files for Git. + warnings.filterwarnings("ignore", category=NonInteractiveExampleWarning) + + fname = FILE_NAME_GEN.example() + with open(fname, "w", encoding="utf-8") as file_obj: + file_obj.write(FILE_DATA_GEN.example()) def _git(command: str) -> str: """Run a git command in a subprocess.""" git_command = ["git"] + command.split() # Use check=True so an error is raised if git has a problem. Only read STDOUT - result = subprocess.run(git_command, shell=True, check=True, capture_output=True) - return result.stdout.decode() + result = subprocess.run(git_command, check=True, capture_output=True) + return result.stdout.decode().strip() def test_reading_hash_from_git(): @@ -45,15 +62,123 @@ def test_reading_hash_from_git(): # Git dir not created yet. Returns Undefined assert utilities._get_hash_from_git_dir(git_dir) == "Undefined" - # Intialised. Should still be undefined. + # Initialised. Should still be undefined. _git("init") assert utilities._get_hash_from_git_dir(git_dir) == "Undefined" + # Create some fake files and commit them. + create_fake_file() + create_fake_file() + _git("add -A") + _git("commit -m 'message'") + git_hash1 = _git("rev-parse HEAD") + # Check we read the hash the same as the git executable + assert utilities._get_hash_from_git_dir(git_dir) == git_hash1 + + # Create more fake files and check again. + create_fake_file() + create_fake_file() + _git("add -A") + _git("commit -m 'message'") + git_hash2 = _git("rev-parse HEAD") + assert utilities._get_hash_from_git_dir(git_dir) == git_hash2 + + # Save the branch name (robust to the default branch name changing) + branch_name = _git("rev-parse --abbrev-ref HEAD") + + # Check out the old commit in detached HEAD and check it is correct + _git(f"checkout {git_hash1}") + assert utilities._get_hash_from_git_dir(git_dir) == git_hash1 + + # 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 + finally: + # Return to original working dir + os.chdir(working_dir) + + +@pytest.fixture +def git_repo(): + """Return a git repo path (set as working dir) with couple of commits.""" + working_dir = os.getcwd() + try: + with tempfile.TemporaryDirectory() as tmpdir: + os.chdir(tmpdir) + # Initialise, create files and commit a couple of times + _git("init") + create_fake_file() + create_fake_file() + _git("add -A") + _git("commit -m 'message'") + create_fake_file() + create_fake_file() + _git("add -A") + _git("commit -m 'message'") + + yield tmpdir + finally: # Return to original working dir os.chdir(working_dir) +def test_reading_hash_from_broken_git_dir(git_repo): + """Break a git dir and check the hash is "Undefined".""" + git_dir = os.path.join(git_repo, ".git") + + # Break the git directory and check the hash is undefined + shutil.move( + os.path.join(git_dir, "refs"), + os.path.join(git_dir, "refs1"), + copy_function=shutil.copytree, + ) + assert utilities._get_hash_from_git_dir(git_dir) == "Undefined" + + # Fix it and check it works again + shutil.move( + os.path.join(git_dir, "refs1"), + os.path.join(git_dir, "refs"), + copy_function=shutil.copytree, + ) + + commit_hash = utilities._get_hash_from_git_dir(git_dir) + assert utilities.COMMIT_REGEX.match(commit_hash) is not None + + # Break the git directory differently and check the hash is undefined + os.remove(os.path.join(git_dir, "HEAD")) + assert utilities._get_hash_from_git_dir(git_dir) == "Undefined" + + +def test_reading_hash_from_broken_git_ref_file(git_repo): + """Break the git ref file and check the hash is "Undefined".""" + git_dir = os.path.join(git_repo, ".git") + branch_name = _git("rev-parse --abbrev-ref HEAD") + ref_path = os.path.join(git_dir, "refs", "heads", branch_name) + # Check commit can be read + commit_hash = utilities._get_hash_from_git_dir(git_dir) + assert utilities.COMMIT_REGEX.match(commit_hash) is not None + # Write something into the ref file + with open(ref_path, "w", encoding="utf-8") as head_file: + head_file.write("This is now broken") + # It is now undefined + assert utilities._get_hash_from_git_dir(git_dir) == "Undefined" + + +def test_reading_hash_from_broken_git_head_file(git_repo): + """Break the git HEAD file and check the hash is "Undefined".""" + git_dir = os.path.join(git_repo, ".git") + head_path = os.path.join(git_dir, "HEAD") + # Check commit can be read + commit_hash = utilities._get_hash_from_git_dir(git_dir) + assert utilities.COMMIT_REGEX.match(commit_hash) is not None + # Write something into the ref file + with open(head_path, "w", encoding="utf-8") as ref_file: + ref_file.write("This is now broken") + # It is now undefined + assert utilities._get_hash_from_git_dir(git_dir) == "Undefined" + + def test_reading_version_from_toml(): """Check version string can be read from this project's TOML file. From ea1c1df4ebc8c44c06bd631b41aec549bd4c4aa1 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Fri, 11 Jul 2025 22:19:33 +0100 Subject: [PATCH 06/11] Improve testing of version strings and git hashes --- tests/test_version_strings.py | 93 +++++++++++++++++++++++++---------- 1 file changed, 66 insertions(+), 27 deletions(-) diff --git a/tests/test_version_strings.py b/tests/test_version_strings.py index c0e1df18..8d891043 100644 --- a/tests/test_version_strings.py +++ b/tests/test_version_strings.py @@ -6,7 +6,7 @@ import re import tempfile import shutil import warnings - +import logging from hypothesis import strategies as st from hypothesis.errors import NonInteractiveExampleWarning @@ -14,6 +14,12 @@ import pytest from openflexure_microscope_server import utilities +# Useful for really finding the repo dir when we mock where it is. +THIS_DIR = os.path.dirname(__file__) +TRUE_REPO_DIR = os.path.dirname(THIS_DIR) + +# For explicit version checking. +VER_STRING = "3.0.0-alpha1" # This is the regex provided by https://semver.org/ SEMVER_REGEX = re.compile( @@ -49,6 +55,64 @@ def _git(command: str) -> str: return result.stdout.decode().strip() +@pytest.fixture +def temp_dir(): + """Return the path of a temporary directory (set as working dir).""" + working_dir = os.getcwd() + try: + with tempfile.TemporaryDirectory() as tmpdir: + os.chdir(tmpdir) + yield tmpdir + finally: + # Return to original working dir + os.chdir(working_dir) + + +@pytest.fixture +def git_repo(temp_dir): + """Return a git repo path (set as working dir) with couple of commits.""" + # Initialise, create files and commit a couple of times + _git("init") + create_fake_file() + create_fake_file() + _git("add -A") + _git("commit -m 'message'") + create_fake_file() + create_fake_file() + _git("add -A") + _git("commit -m 'message'") + + yield temp_dir + + +def test_version_data_git_and_toml(mocker, git_repo): + """Check expected version reported if git repo and pyproject.toml are available.""" + mocker.patch.object(utilities, "REPO_DIR", new=git_repo) + git_hash = _git("rev-parse HEAD") + shutil.copy(os.path.join(TRUE_REPO_DIR, "pyproject.toml"), git_repo) + version_data = utilities.robust_version_strings() + # Check versions are as expected. Prepending the v to the semantic version. + assert version_data.version == "v" + VER_STRING + assert version_data.version_source == git_hash + + +def test_version_data_git_only(mocker, git_repo, caplog): + """In the odd case of a git repo and no pyproject.toml. Check the hash is reported. + + The version string should just be reported as "Undefined". This is weird, so we + expect there to be errors logged. + """ + with caplog.at_level(logging.ERROR): + mocker.patch.object(utilities, "REPO_DIR", new=git_repo) + git_hash = _git("rev-parse HEAD") + version_data = utilities.robust_version_strings() + # Check versions are as expected. Prepending the v to the semantic version. + assert version_data.version == "Undefined" + assert version_data.version_source == git_hash + # There should be 1 error level log + assert len(caplog.records) == 1 + + def test_reading_hash_from_git(): """Use create a Git repo and check that hash can be read. @@ -98,31 +162,6 @@ def test_reading_hash_from_git(): os.chdir(working_dir) -@pytest.fixture -def git_repo(): - """Return a git repo path (set as working dir) with couple of commits.""" - working_dir = os.getcwd() - try: - with tempfile.TemporaryDirectory() as tmpdir: - os.chdir(tmpdir) - # Initialise, create files and commit a couple of times - _git("init") - create_fake_file() - create_fake_file() - _git("add -A") - _git("commit -m 'message'") - create_fake_file() - create_fake_file() - _git("add -A") - _git("commit -m 'message'") - - yield tmpdir - - finally: - # Return to original working dir - os.chdir(working_dir) - - def test_reading_hash_from_broken_git_dir(git_repo): """Break a git dir and check the hash is "Undefined".""" git_dir = os.path.join(git_repo, ".git") @@ -190,7 +229,7 @@ def test_reading_version_from_toml(): # Check version does follow semantic versioning. assert SEMVER_REGEX.match(ver_string) # Explicit check - assert ver_string == "3.0.0-alpha1" + assert ver_string == VER_STRING def test_error_reading_version_from_toml(): From d3732aef2f85c2f18e3399db4a83394087a54c4c Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Sat, 12 Jul 2025 00:27:40 +0100 Subject: [PATCH 07/11] Finish testing new version functionality in utilities. --- tests/test_version_strings.py | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/tests/test_version_strings.py b/tests/test_version_strings.py index 8d891043..121bb065 100644 --- a/tests/test_version_strings.py +++ b/tests/test_version_strings.py @@ -87,6 +87,7 @@ def git_repo(temp_dir): def test_version_data_git_and_toml(mocker, git_repo): """Check expected version reported if git repo and pyproject.toml are available.""" + # Patch the variable for this repo to the repo provided by the fixture. mocker.patch.object(utilities, "REPO_DIR", new=git_repo) git_hash = _git("rev-parse HEAD") shutil.copy(os.path.join(TRUE_REPO_DIR, "pyproject.toml"), git_repo) @@ -96,14 +97,25 @@ def test_version_data_git_and_toml(mocker, git_repo): assert version_data.version_source == git_hash +def test_version_data_toml_only(mocker, temp_dir): + """In the case of a pyproject.toml but not git dir, check version is reported.""" + # Patch the variable for this repo to the temp dir provided by the fixture. + mocker.patch.object(utilities, "REPO_DIR", new=temp_dir) + shutil.copy(os.path.join(TRUE_REPO_DIR, "pyproject.toml"), temp_dir) + version_data = utilities.robust_version_strings() + # Check versions are as expected. Prepending the v to the semantic version. + assert version_data.version == "v" + VER_STRING + assert version_data.version_source == "TOML" + + def test_version_data_git_only(mocker, git_repo, caplog): """In the odd case of a git repo and no pyproject.toml. Check the hash is reported. The version string should just be reported as "Undefined". This is weird, so we expect there to be errors logged. """ + mocker.patch.object(utilities, "REPO_DIR", new=git_repo) with caplog.at_level(logging.ERROR): - mocker.patch.object(utilities, "REPO_DIR", new=git_repo) git_hash = _git("rev-parse HEAD") version_data = utilities.robust_version_strings() # Check versions are as expected. Prepending the v to the semantic version. @@ -113,6 +125,26 @@ def test_version_data_git_only(mocker, git_repo, caplog): assert len(caplog.records) == 1 +def test_version_distribution(mocker, temp_dir): + """Simulate a distribution package with no pyproject.toml or git directory. + + Check the returned result is from importlib. + """ + mocker.patch.object(utilities, "REPO_DIR", new=temp_dir) + # As `openflexure_microscope_server.utilities` imported `version` from + # `importlib.metadata` to mock the already imported: + # `openflexure_microscope_server.utilities.version`, this is `version` from + # `importlib.metadata` + mocker.patch( + "openflexure_microscope_server.utilities.version", return_value="oobar" + ) + version_data = utilities.robust_version_strings() + # Expect the version is the output from importlib.metadata.version, prepended by a + # "v". So, voobar, as we mocked the version to be oobar.! + assert version_data.version == "voobar" + assert version_data.version_source == "Dist" + + def test_reading_hash_from_git(): """Use create a Git repo and check that hash can be read. From 225ee5bb8239357e28719988713c86a42d5597d1 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Sat, 12 Jul 2025 23:28:11 +0100 Subject: [PATCH 08/11] Tweak CI script to have a Git name and email as this is now needed for unit testing. --- .gitlab-ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 0a2f663e..49b5dd0e 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -125,6 +125,10 @@ pytest: stage: testing extends: .python script: + # Our unit tests create fake Git repositories. This doesn't work unless + # Git is configured with a name and email. + - git config --global user.name "Sir Unit of Test" + - git config --global user.email "fake@email.no" - pytest # --gitlab-code-quality-report=pytest-warnings.json # can restore when it installs ok coverage: '/(?i)total.*? (100(?:\.0+)?\%|[1-9]?\d(?:\.\d+)?\%)$/' From e614ec577ee52a3b183de4c04a983bbf76daacc6 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Sun, 13 Jul 2025 10:26:01 +0100 Subject: [PATCH 09/11] Expose server version strings in webapp. --- .../aboutComponents/statusPane.vue | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/webapp/src/components/tabContentComponents/aboutComponents/statusPane.vue b/webapp/src/components/tabContentComponents/aboutComponents/statusPane.vue index 47cf4463..6314666f 100644 --- a/webapp/src/components/tabContentComponents/aboutComponents/statusPane.vue +++ b/webapp/src/components/tabContentComponents/aboutComponents/statusPane.vue @@ -25,7 +25,8 @@
Server Version:
- v{{ app_version() }} + {{ version }}
+ ({{ version_source }})
@@ -66,11 +67,40 @@ export default { name: "StatusPane", components: { ActionButton }, + data: function() { + return { + version: undefined, + version_source: undefined + } + }, + + async mounted() { + let version_data = await this.readThingProperty( + "settings", + "version_data" + ); + this.version = version_data.version; + this.version_source = this.truncate(version_data.version_source); + + }, + computed: { things: function() { return this.$store.getters["wot/thingDescriptions"]; } }, + + methods: { + truncate(string, max_length=15) { + if (!(typeof string === 'string' || string instanceof String)) { + return string; + } + if (string.length <= max_length) { + return string; + } + return string.slice(0, max_length - 3) + "..."; + } + } }; From 34f8f86ef2a487198c011ebe3bef1e41632d74a3 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Mon, 14 Jul 2025 15:46:13 +0000 Subject: [PATCH 10/11] Apply suggestions from code review of branch update-setting-manager Co-authored-by: Joe Knapper Co-authored-by: Beth Probert --- src/openflexure_microscope_server/utilities.py | 14 ++++++++------ tests/test_version_strings.py | 4 ++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/openflexure_microscope_server/utilities.py b/src/openflexure_microscope_server/utilities.py index 13d6ee23..60a2fc5b 100644 --- a/src/openflexure_microscope_server/utilities.py +++ b/src/openflexure_microscope_server/utilities.py @@ -104,7 +104,7 @@ class VersionData(BaseModel): version_source: str -def robust_version_strings() -> tuple[str, str]: +def robust_version_strings() -> VersionData: """Return a version string and information on its source. Returning a python version string is not without problems. If the package is @@ -118,7 +118,7 @@ def robust_version_strings() -> tuple[str, str]: installation from source. Return the version string from the toml file and return the literal string "TOML" as its source. 3. If a Git directory is available but no pyproject.toml then something is very - weird log an error. Then return "Undefined" as the version string, but still + weird - log an error. Then return "Undefined" as the version string, but still return the Git hash as the source. 4. Finally if neither a Git directory nor a pyproject.toml are available then the server has been installed from a distribution package (i.e. a wheel). In this @@ -152,9 +152,10 @@ def robust_version_strings() -> tuple[str, str]: ver = "Undefined" source = _get_hash_from_git_dir(git_dir_path) else: - # "Neither probably installed from wheel. Note this can - # be unreliable if the package is not installed as a distribution. - # This is why it is the last option. + # Has neither .git nor pyproject.toml. It is probably installed from wheel. + # This will be the expected method of packaging in the future. This option + # is handled last as ``importlib.metadata.version`` can be unreliable if + # the package is no installed as a distribution package. ver = "v" + version("openflexure_microscope_server") source = "Dist" return VersionData(version=ver, version_source=source) @@ -222,7 +223,8 @@ def _get_version_from_toml(toml_path: str) -> str: :param toml_path: The path to the toml file. - :returns: The version string directly as reported in the toml file. + :returns: The version string directly as reported in the toml file, or "Undefined" + if there is any error, errors are logged not raised. """ try: with open(toml_path, "rb") as toml_file: diff --git a/tests/test_version_strings.py b/tests/test_version_strings.py index 121bb065..19d3a74c 100644 --- a/tests/test_version_strings.py +++ b/tests/test_version_strings.py @@ -80,7 +80,7 @@ def git_repo(temp_dir): create_fake_file() create_fake_file() _git("add -A") - _git("commit -m 'message'") + _git("commit -m 'message2'") yield temp_dir @@ -175,7 +175,7 @@ def test_reading_hash_from_git(): create_fake_file() create_fake_file() _git("add -A") - _git("commit -m 'message'") + _git("commit -m 'message2'") git_hash2 = _git("rev-parse HEAD") assert utilities._get_hash_from_git_dir(git_dir) == git_hash2 From c64fe170ccbb8c4e79a8830efa42ed8c4744b0af Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Mon, 14 Jul 2025 17:09:50 +0100 Subject: [PATCH 11/11] Update pyproject.toml to be clear where version strings need to be changed on release. --- pyproject.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index e45c6eaf..e3ea5afc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,5 +1,8 @@ [project] name = "openflexure-microscope-server" + +# Note that version needs to match a string stored in a unit test, and +# the package.json file in the webapp or the CI will fail. version = "3.0.0-alpha1" authors = [ { name="Richard Bowman", email="richard.bowman@cantab.net" },