From 385c61535c3446b2fbe025d271296a1ab38c1d00 Mon Sep 17 00:00:00 2001 From: Samuel Jones Date: Thu, 12 Dec 2024 09:05:36 +0000 Subject: [PATCH] Use API to get specifications (#343) * Initial commit for specifications * Formatting and linting commit * Add specification tests * Formatting and linting commit * Give the e2e tests a first go over * Fix MARI rules tests * Fix Specifications _rule_old * Update tests.yml * Adjust e2e to stop requests being made to the FIA-API * Adjust e2e to stop requests being made to the FIA-API * Further adjust test files * Create a fake api for e2e tests to use instead of using the real one * Fix ruff linting error * Fix mypy errors * Ensure that order does not matter as rules that should be last self declare * Formatting and linting commit * Fix pytest error * Review comments/mypy fixes * Quick comment addition * Fix some errors with linting and long comments * Formatting and linting commit --------- Co-authored-by: github-actions --- .github/workflows/tests.yml | 5 + .gitignore | 2 + pyproject.toml | 4 +- rundetection/rules/common_rules.py | 5 +- rundetection/rules/inter_rules.py | 4 + rundetection/rules/mari_rules.py | 24 +-- rundetection/rules/rule.py | 6 +- rundetection/specifications.py | 60 ++++-- test/docker-compose.yml | 19 ++ test/e2e_components/Dockerfile | 12 ++ test/e2e_components/fake_fia_api/__init__.py | 0 test/e2e_components/fake_fia_api/fia_api.py | 14 ++ test/e2e_components/fake_fia_api/router.py | 32 ++++ test/e2e_components/pyproject.toml | 53 ++++++ test/rules/test_factory.py | 4 +- test/rules/test_mari_rules.py | 24 +-- .../specifications/alf_specification.json | 0 .../specifications/argus_specification.json | 0 .../specifications/chipip_specification.json | 0 .../specifications/chipir_specification.json | 0 .../specifications/chronus_specification.json | 0 .../specifications/crisp_specification.json | 0 .../specifications/deva_specification.json | 0 .../specifications/emma-a_specification.json | 0 .../specifications/emma_specification.json | 0 .../specifications/emmaa_specification.json | 0 .../specifications/emu_specification.json | 0 .../specifications/enginx_specification.json | 0 .../specifications/evs_specification.json | 0 .../specifications/gem_specification.json | 0 .../specifications/het_specification.json | 0 .../specifications/hifi_specification.json | 0 .../specifications/hrpd_specification.json | 0 .../specifications/imat_specification.json | 0 .../specifications/ines_specification.json | 0 .../specifications/inter_specification.json | 0 .../specifications/iris_specification.json | 0 .../specifications/lad_specification.json | 0 .../specifications/larmor_specification.json | 0 .../specifications/let_specification.json | 0 .../specifications/loq_specification.json | 0 .../specifications/maps_specification.json | 0 .../specifications/mari_specification.json | 4 +- .../specifications/merlin_specification.json | 0 .../specifications/musr_specification.json | 0 .../specifications/nile_specification.json | 0 .../specifications/nimrod_specification.json | 0 .../specifications/offspec_specification.json | 0 .../specifications/osiris_specification.json | 0 .../specifications/pearl_specification.json | 0 .../specifications/polaris_specification.json | 0 .../specifications/polref_specification.json | 0 .../specifications/prisma_specification.json | 0 .../specifications/rotax_specification.json | 0 .../specifications/sandals_specification.json | 0 .../specifications/sans2d_specification.json | 0 .../specifications/surf_specification.json | 0 .../specifications/sxd_specification.json | 0 .../specifications/tfxa_specification.json | 0 .../specifications/tosca_specification.json | 0 .../specifications/vesuvio_specification.json | 0 .../specifications/wish_specification.json | 0 .../specifications/zoom_specification.json | 0 test/test_e2e.py | 21 ++- test/test_specifications.py | 178 +++++++++--------- 65 files changed, 321 insertions(+), 150 deletions(-) create mode 100644 test/e2e_components/Dockerfile create mode 100644 test/e2e_components/fake_fia_api/__init__.py create mode 100644 test/e2e_components/fake_fia_api/fia_api.py create mode 100644 test/e2e_components/fake_fia_api/router.py create mode 100644 test/e2e_components/pyproject.toml rename {rundetection => test/test_data}/specifications/alf_specification.json (100%) rename {rundetection => test/test_data}/specifications/argus_specification.json (100%) rename {rundetection => test/test_data}/specifications/chipip_specification.json (100%) rename {rundetection => test/test_data}/specifications/chipir_specification.json (100%) rename {rundetection => test/test_data}/specifications/chronus_specification.json (100%) rename {rundetection => test/test_data}/specifications/crisp_specification.json (100%) rename {rundetection => test/test_data}/specifications/deva_specification.json (100%) rename {rundetection => test/test_data}/specifications/emma-a_specification.json (100%) rename {rundetection => test/test_data}/specifications/emma_specification.json (100%) rename {rundetection => test/test_data}/specifications/emmaa_specification.json (100%) rename {rundetection => test/test_data}/specifications/emu_specification.json (100%) rename {rundetection => test/test_data}/specifications/enginx_specification.json (100%) rename {rundetection => test/test_data}/specifications/evs_specification.json (100%) rename {rundetection => test/test_data}/specifications/gem_specification.json (100%) rename {rundetection => test/test_data}/specifications/het_specification.json (100%) rename {rundetection => test/test_data}/specifications/hifi_specification.json (100%) rename {rundetection => test/test_data}/specifications/hrpd_specification.json (100%) rename {rundetection => test/test_data}/specifications/imat_specification.json (100%) rename {rundetection => test/test_data}/specifications/ines_specification.json (100%) rename {rundetection => test/test_data}/specifications/inter_specification.json (100%) rename {rundetection => test/test_data}/specifications/iris_specification.json (100%) rename {rundetection => test/test_data}/specifications/lad_specification.json (100%) rename {rundetection => test/test_data}/specifications/larmor_specification.json (100%) rename {rundetection => test/test_data}/specifications/let_specification.json (100%) rename {rundetection => test/test_data}/specifications/loq_specification.json (100%) rename {rundetection => test/test_data}/specifications/maps_specification.json (100%) rename {rundetection => test/test_data}/specifications/mari_specification.json (80%) rename {rundetection => test/test_data}/specifications/merlin_specification.json (100%) rename {rundetection => test/test_data}/specifications/musr_specification.json (100%) rename {rundetection => test/test_data}/specifications/nile_specification.json (100%) rename {rundetection => test/test_data}/specifications/nimrod_specification.json (100%) rename {rundetection => test/test_data}/specifications/offspec_specification.json (100%) rename {rundetection => test/test_data}/specifications/osiris_specification.json (100%) rename {rundetection => test/test_data}/specifications/pearl_specification.json (100%) rename {rundetection => test/test_data}/specifications/polaris_specification.json (100%) rename {rundetection => test/test_data}/specifications/polref_specification.json (100%) rename {rundetection => test/test_data}/specifications/prisma_specification.json (100%) rename {rundetection => test/test_data}/specifications/rotax_specification.json (100%) rename {rundetection => test/test_data}/specifications/sandals_specification.json (100%) rename {rundetection => test/test_data}/specifications/sans2d_specification.json (100%) rename {rundetection => test/test_data}/specifications/surf_specification.json (100%) rename {rundetection => test/test_data}/specifications/sxd_specification.json (100%) rename {rundetection => test/test_data}/specifications/tfxa_specification.json (100%) rename {rundetection => test/test_data}/specifications/tosca_specification.json (100%) rename {rundetection => test/test_data}/specifications/vesuvio_specification.json (100%) rename {rundetection => test/test_data}/specifications/wish_specification.json (100%) rename {rundetection => test/test_data}/specifications/zoom_specification.json (100%) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 8ab76326..41e1cb44 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -39,21 +39,26 @@ jobs: e2e: runs-on: ubuntu-latest + steps: - name: Checkout project uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 + - name: Set up python uses: actions/setup-python@39cd14951b08e74b54015e9e001cdefcf80e669f # v5.1.1 with: python-version: '3.12' + - name: Install dependencies run: | python -m pip install --upgrade pip python -m pip install .[test] + - name: Start e2e docker compose environment run: | cd test docker compose up -d + - name: Run e2e test run: pytest -l -v --random-order --random-order-bucket=global test/test_e2e.py diff --git a/.gitignore b/.gitignore index 585a5262..95bcea54 100644 --- a/.gitignore +++ b/.gitignore @@ -130,3 +130,5 @@ dmypy.json # Pyre type checker .pyre/ + +fia-api/ \ No newline at end of file diff --git a/pyproject.toml b/pyproject.toml index 28ef8622..e6c75a67 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -30,7 +30,9 @@ test = [ "pytest==8.2.2", "pytest-cov==5.0.0", "requests==2.32.3", - "pytest-random-order==1.1.1" + "pytest-random-order==1.1.1", + "fastapi[all]==0.111.1", + "pydantic==2.8.2", ] dev = [ diff --git a/rundetection/rules/common_rules.py b/rundetection/rules/common_rules.py index 9cbca862..cf419af3 100644 --- a/rundetection/rules/common_rules.py +++ b/rundetection/rules/common_rules.py @@ -56,6 +56,10 @@ class MolSpecStitchRule(Rule[bool]): Enables Tosca, Osiris, and Iris Run stitching """ + def __init__(self, value: bool) -> None: + super().__init__(value) + self.should_be_last = True + @staticmethod def _is_title_similar(title: str, other_title: str) -> bool: """ @@ -89,7 +93,6 @@ def _get_runs_to_stitch(self, run_path: Path, run_number: int, run_title: str, i else: next_run_number = f"{instrument.upper()}{run_number:08d}.nxs" run_path = Path(run_path.parent, next_run_number) - logger.info("Run path %s does not exist", run_path) logger.info("Returning run numbers %s", run_numbers) return run_numbers diff --git a/rundetection/rules/inter_rules.py b/rundetection/rules/inter_rules.py index f44b9e9c..99fa6925 100644 --- a/rundetection/rules/inter_rules.py +++ b/rundetection/rules/inter_rules.py @@ -12,6 +12,10 @@ class InterStitchRule(Rule[bool]): Rule for collecting each related inter run and including them into the additional values """ + def __init__(self, value: bool) -> None: + super().__init__(value) + self.should_be_last = True + @staticmethod def _get_run_group(job_request: JobRequest) -> str: """ diff --git a/rundetection/rules/mari_rules.py b/rundetection/rules/mari_rules.py index 81194454..82314781 100644 --- a/rundetection/rules/mari_rules.py +++ b/rundetection/rules/mari_rules.py @@ -2,11 +2,9 @@ Mari Rules """ -import json import logging from copy import deepcopy from pathlib import Path -from typing import Any from rundetection.ingestion.ingest import get_run_title from rundetection.job_requests import JobRequest @@ -17,12 +15,12 @@ class MariStitchRule(Rule[bool]): """ - The MariStitchRule is the rule that applies + The MariStitchRule is the rule that applies, dependent on the other rules running first. This runs last. """ def __init__(self, value: bool) -> None: super().__init__(value) - self._spec_values = self._load_mari_spec() + self.should_be_last = True @staticmethod def _get_runs_to_stitch(run_path: Path, run_number: int, run_title: str) -> list[int]: @@ -35,20 +33,6 @@ def _get_runs_to_stitch(run_path: Path, run_number: int, run_title: str) -> list run_path = Path(run_path.parent, f"MAR{run_number}.nxs") return run_numbers - @staticmethod - def _load_mari_spec() -> Any: - """ - Load the entire mari specification into a dictionary - :return: Mari spec as dict - """ - try: - path = Path("rundetection/specifications/mari_specification.json") - with path.open(encoding="utf-8") as spec_file: - return json.load(spec_file) - except FileNotFoundError as exc: - logger.warning("Mari Specification could not be reloaded") - raise RuntimeError("Mari specification is no longer available") from exc - def verify(self, job_request: JobRequest) -> None: if not self._value: # if the stitch rule is set to false, skip return @@ -62,8 +46,8 @@ def verify(self, job_request: JobRequest) -> None: additional_request.additional_values["sum_runs"] = True # We must reapply the common mari rules manually here, if we apply the whole spec automatically it will # produce an infinite loop - additional_request.additional_values["mask_file_link"] = self._spec_values["marimaskfile"] - additional_request.additional_values["wbvan"] = self._spec_values["mariwbvan"] + additional_request.additional_values["mask_file_link"] = job_request.additional_values["mask_file_link"] + additional_request.additional_values["wbvan"] = job_request.additional_values["wbvan"] job_request.additional_requests.append(additional_request) diff --git a/rundetection/rules/rule.py b/rundetection/rules/rule.py index ac93b80c..86d63cc6 100644 --- a/rundetection/rules/rule.py +++ b/rundetection/rules/rule.py @@ -3,7 +3,7 @@ """ from abc import ABC, abstractmethod -from typing import Generic, TypeVar +from typing import Any, Generic, TypeVar from rundetection.job_requests import JobRequest @@ -15,8 +15,12 @@ class Rule(Generic[T], ABC): Abstract Rule, implement to define a rule that must be followed to allow a reduction to be run on a nexus file """ + def __eq__(self, other: Any) -> bool: + return isinstance(other, type(self)) and self._value == other._value + def __init__(self, value: T): self._value: T = value + self.should_be_last = False @abstractmethod def verify(self, job_request: JobRequest) -> None: diff --git a/rundetection/specifications.py b/rundetection/specifications.py index 6f077180..fc243933 100644 --- a/rundetection/specifications.py +++ b/rundetection/specifications.py @@ -2,10 +2,12 @@ Contains the InstrumentSpecification class, the abstract Rule Class and Rule Implementations """ -import json +import datetime import logging +import os import typing -from pathlib import Path + +import requests from rundetection.exceptions import RuleViolationError from rundetection.job_requests import JobRequest @@ -18,6 +20,9 @@ logger = logging.getLogger(__name__) +FIA_API_URL = os.getenv("FIA_API_URL", "http://localhost:8000") +SPEC_REQUEST_TIMEOUT_MINS = 10 + class InstrumentSpecification: """ @@ -26,20 +31,40 @@ class InstrumentSpecification: """ def __init__(self, instrument: str) -> None: - logger.info("Loading instrument specification for: %s", instrument) self._instrument = instrument self._rules: list[Rule[Any]] = [] - self._load_rules() - - def _load_rules(self) -> None: - try: - path = Path(f"rundetection/specifications/{self._instrument.lower()}_specification.json") - with path.open(encoding="utf-8") as spec_file: - spec: dict[str, Any] = json.load(spec_file) - self._rules = [rule_factory(key, value) for key, value in spec.items()] - except FileNotFoundError: - logger.error("No specification for file: %s", self._instrument) - raise + self.loaded_time: datetime.datetime | None = None + self._load_rules_from_api() + + def _load_rules_from_api(self) -> None: + logger.info("Requesting specification from API for %s", self._instrument) + fia_api_api_key = os.environ["FIA_API_API_KEY"] + headers: dict[str, Any] = {"Authorization": f"Bearer {fia_api_api_key}", "accept": "application/json"} + response = requests.get( + url=f"{FIA_API_URL}/instrument/{self._instrument.upper()}/specification", headers=headers, timeout=1 + ) + response.raise_for_status() + spec: dict[str, Any] = response.json() + logger.info("Response from API for spec is: \n%s", spec) + self._rules = [rule_factory(key, value) for key, value in spec.items()] + self._order_rules() + self.loaded_time = datetime.datetime.now(tz=datetime.UTC) + logger.info("Loaded instrument specification for: %s at: %s", self._instrument, self.loaded_time) + + def _order_rules(self) -> None: + """ + Sometimes we need to ensure some rules end up at the end of the list, notably those with stitch in the name + """ + for rule in self._rules: + # We need to ensure rules that do a stitch, or any that added extra jobs, need to come last. + if rule.should_be_last: + self._rules.remove(rule) + self._rules.append(rule) + + def _rule_old(self) -> bool: + return self.loaded_time is None or datetime.timedelta(minutes=SPEC_REQUEST_TIMEOUT_MINS) < ( + datetime.datetime.now(tz=datetime.UTC) - self.loaded_time + ) def verify(self, job_request: JobRequest) -> None: """ @@ -48,6 +73,13 @@ def verify(self, job_request: JobRequest) -> None: :param job_request: A JobRequest :return: whether the specification is met """ + if self._rule_old(): + logger.info( + "Rule for instrument %s is older than %s minutes, reloading rule from API", + self._instrument, + SPEC_REQUEST_TIMEOUT_MINS, + ) + self._load_rules_from_api() if len(self._rules) == 0: job_request.will_reduce = False for rule in self._rules: diff --git a/test/docker-compose.yml b/test/docker-compose.yml index 70c95a90..f68a1739 100644 --- a/test/docker-compose.yml +++ b/test/docker-compose.yml @@ -13,6 +13,19 @@ services: RABBITMQ_USER: guest RABBITMQ_PASSWORD: guest + fake-fia-api: + build: + context: e2e_components/ + dockerfile: Dockerfile + healthcheck: + test: curl --fail http://localhost:80/healthz || exit 1 + interval: 1s + timeout: 1s + retries: 3 + start_period: 1s + volumes: + - ../test/test_data/specifications:/data + run-detection: build: context: ../ @@ -20,9 +33,15 @@ services: depends_on: rabbit-mq: condition: service_healthy + fake-fia-api: + condition: service_healthy environment: QUEUE_HOST: "rabbit-mq" + QUEUE_USER: "guest" + QUEUE_PASSWORD: "guest" INGRESS_QUEUE_NAME: "watched-files" EGRESS_QUEUE_NAME: "scheduled-jobs" + FIA_API_API_KEY: "shh" + FIA_API_URL: http://fake-fia-api:80 volumes: - ../test/test_data/e2e_data:/archive diff --git a/test/e2e_components/Dockerfile b/test/e2e_components/Dockerfile new file mode 100644 index 00000000..8f8dd7ef --- /dev/null +++ b/test/e2e_components/Dockerfile @@ -0,0 +1,12 @@ +FROM python:3.12 + +WORKDIR /fia_api + +COPY . /fia_api + +RUN apt-get update \ + && apt-get -y install libpq-dev gcc \ + && python -m pip install --upgrade pip \ + && python -m pip install --no-cache-dir . + +CMD ["uvicorn", "fake_fia_api.fia_api:app", "--host", "0.0.0.0", "--port", "80"] \ No newline at end of file diff --git a/test/e2e_components/fake_fia_api/__init__.py b/test/e2e_components/fake_fia_api/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/test/e2e_components/fake_fia_api/fia_api.py b/test/e2e_components/fake_fia_api/fia_api.py new file mode 100644 index 00000000..1f47a88d --- /dev/null +++ b/test/e2e_components/fake_fia_api/fia_api.py @@ -0,0 +1,14 @@ +""" +Fake API app to be used for FastAPI +""" + +from fastapi import FastAPI + +from fake_fia_api.router import ROUTER + +app = FastAPI() + +# This must be updated before exposing outside the vpn +ALLOWED_ORIGINS = ["*"] + +app.include_router(ROUTER) diff --git a/test/e2e_components/fake_fia_api/router.py b/test/e2e_components/fake_fia_api/router.py new file mode 100644 index 00000000..abd13222 --- /dev/null +++ b/test/e2e_components/fake_fia_api/router.py @@ -0,0 +1,32 @@ +import json +from pathlib import Path +from typing import Any, Literal + +from fastapi import APIRouter + +ROUTER = APIRouter() + + +def get_specification_from_file(instrument: str) -> Any: + """ + Given an instrument, return the specification + :param instrument: The instrument for which specification to get + :return: The specification file contents + """ + path = Path(f"/data/{instrument.lower()}_specification.json") + with path.open(encoding="utf-8") as fle: + return json.load(fle) + + +@ROUTER.get("/instrument/{instrument_name}/specification", response_model=None) +async def get_instrument_specification(instrument_name: str) -> Any: + """ + This is a fake API for the e2e tests to be deployed to provide specifications for rundetection e2e tests + """ + return get_specification_from_file(instrument_name) + + +@ROUTER.get("/healthz") +async def get() -> Literal["ok"]: + """Health Check endpoint.""" + return "ok" diff --git a/test/e2e_components/pyproject.toml b/test/e2e_components/pyproject.toml new file mode 100644 index 00000000..fcf27992 --- /dev/null +++ b/test/e2e_components/pyproject.toml @@ -0,0 +1,53 @@ +[project] +name = "fia-api-fake" +version = "0.0.1" +requires-python = ">= 3.11" +dependencies = [ + "fastapi[all]==0.111.1", + "pydantic==2.8.2", +] + +[tool.ruff] +line-length = 120 + +[tool.ruff.lint] +select = [ + "F", # flake8 - Basic initial rules + "E", # pycodestyle (Error) - pep8 compliance + "W", # pycodestyle (Warning) - pep8 compliance + "C90", # mccabe - flags extremely complex functions + "I", # isort - Sort imports and flag missing imports + "N", # pep8-naming - Ensures pep8 compliance for naming + "UP", # pyupgrade - Automatically upgrade syntax for newer versions + "S", # flake8-bandit - Flake8 security + "B", # flake8-bugbear - Finding likely bugs and design problems + "A", # flake8-builtins - Finds code shadowing builtins + "COM", # flake8-commas - Find and fixes issues with commas in lists/dicts + "C4", # flake8-comprehensions - Simplify list/dict comprehension + "DTZ", # flake8-datetimez - Ensure timezones are enforced for code + "EXE", # flake8-executable - Fix issues around shebangs and executable files + "ISC", # flake8-implicit-str-concat - Find implicitly concatenated strings + "LOG", # flake8-logging - Enforce basic rules with builtin logger + "T20", # flake8-print - Remove print statements + "PT", # flake8-pytest-style - Fix issues with pytest + "Q", # flake8-quotes - Bad quote handling + "RET", # flake8-return - Fix issues with return values + "SIM", # flake8-simplify - Simplify parts of the code + "TCH", # flake8-type-checking - Move imports only for typing behind TYPE_CHECKING + "PTH", # flake8-use-pathlib - Replace os with pathlib + "TD", # flake8-todos - Enforce basic TODOs + "FIX", # flake8-fix me - Resolve the issue instead of a fix me + "ERA", # eradicate - Remove commented out code. + "PL", "C", "E", "R", "W", # Pylint - does a lot + "FLY", # flynt - prefer f string over .format + "PERF", # Perflint - Flag performance antipatterns + "RUF", # Ruff specific rules +] +ignore = [ + "S101", # flake8-bandit - Use of assert (all over pytest tests) + "ISC001", # Conflicts with the formatter + "COM812", # Conflicts with the formatter +] + +[tool.ruff.lint.pylint] +max-args = 10 \ No newline at end of file diff --git a/test/rules/test_factory.py b/test/rules/test_factory.py index b8c2c5c8..85e323b0 100644 --- a/test/rules/test_factory.py +++ b/test/rules/test_factory.py @@ -4,7 +4,6 @@ import unittest from typing import Any -from unittest.mock import patch import pytest @@ -64,8 +63,7 @@ def test_rule_factory_returns_correct_rule(rule_key, rule_value, expected_rule): :param expected_rule: The expected rule class :return: None """ - with patch("rundetection.rules.mari_rules.MariStitchRule._load_mari_spec"): - assert_correct_rule(rule_key, rule_value, expected_rule) + assert_correct_rule(rule_key, rule_value, expected_rule) def test_raises_exception_for_missing_rule_class() -> None: diff --git a/test/rules/test_mari_rules.py b/test/rules/test_mari_rules.py index 2f2efa10..107c5a72 100644 --- a/test/rules/test_mari_rules.py +++ b/test/rules/test_mari_rules.py @@ -2,10 +2,8 @@ Test for mari rules """ -import json import os from pathlib import Path -from typing import Any from unittest.mock import patch import pytest @@ -14,18 +12,6 @@ from rundetection.rules.mari_rules import MariMaskFileRule, MariStitchRule, MariWBVANRule -def get_specification_value(key: str) -> Any: - """ - Given a key, return the specification value - :param key: The key for the rule - :return: The rule value - """ - path = Path("rundetection/specifications/mari_specification.json") - with path.open(encoding="utf-8") as fle: - spec = json.load(fle) - return spec[key] - - @pytest.fixture(autouse=True) def _working_directory_fix(): # Set dir to repo root for purposes of the test. @@ -110,14 +96,16 @@ def test_verify_multiple_runs(mari_stitch_rule_true, job_request): :param job_request: job request fixture :return: None """ + rule = MariMaskFileRule("some link") + rule.verify(job_request) + rule = MariWBVANRule(1234567) + rule.verify(job_request) with patch("rundetection.rules.mari_rules.MariStitchRule._get_runs_to_stitch", return_value=[1, 2, 3]): mari_stitch_rule_true.verify(job_request) assert len(job_request.additional_requests) == 1 - assert job_request.additional_requests[0].additional_values["mask_file_link"] == get_specification_value( - "marimaskfile" - ) - assert job_request.additional_requests[0].additional_values["wbvan"] == get_specification_value("mariwbvan") + assert job_request.additional_requests[0].additional_values["mask_file_link"] == "some link" + assert job_request.additional_requests[0].additional_values["wbvan"] == 1234567 # noqa: PLR2004 def test_mari_mask_rule(job_request): diff --git a/rundetection/specifications/alf_specification.json b/test/test_data/specifications/alf_specification.json similarity index 100% rename from rundetection/specifications/alf_specification.json rename to test/test_data/specifications/alf_specification.json diff --git a/rundetection/specifications/argus_specification.json b/test/test_data/specifications/argus_specification.json similarity index 100% rename from rundetection/specifications/argus_specification.json rename to test/test_data/specifications/argus_specification.json diff --git a/rundetection/specifications/chipip_specification.json b/test/test_data/specifications/chipip_specification.json similarity index 100% rename from rundetection/specifications/chipip_specification.json rename to test/test_data/specifications/chipip_specification.json diff --git a/rundetection/specifications/chipir_specification.json b/test/test_data/specifications/chipir_specification.json similarity index 100% rename from rundetection/specifications/chipir_specification.json rename to test/test_data/specifications/chipir_specification.json diff --git a/rundetection/specifications/chronus_specification.json b/test/test_data/specifications/chronus_specification.json similarity index 100% rename from rundetection/specifications/chronus_specification.json rename to test/test_data/specifications/chronus_specification.json diff --git a/rundetection/specifications/crisp_specification.json b/test/test_data/specifications/crisp_specification.json similarity index 100% rename from rundetection/specifications/crisp_specification.json rename to test/test_data/specifications/crisp_specification.json diff --git a/rundetection/specifications/deva_specification.json b/test/test_data/specifications/deva_specification.json similarity index 100% rename from rundetection/specifications/deva_specification.json rename to test/test_data/specifications/deva_specification.json diff --git a/rundetection/specifications/emma-a_specification.json b/test/test_data/specifications/emma-a_specification.json similarity index 100% rename from rundetection/specifications/emma-a_specification.json rename to test/test_data/specifications/emma-a_specification.json diff --git a/rundetection/specifications/emma_specification.json b/test/test_data/specifications/emma_specification.json similarity index 100% rename from rundetection/specifications/emma_specification.json rename to test/test_data/specifications/emma_specification.json diff --git a/rundetection/specifications/emmaa_specification.json b/test/test_data/specifications/emmaa_specification.json similarity index 100% rename from rundetection/specifications/emmaa_specification.json rename to test/test_data/specifications/emmaa_specification.json diff --git a/rundetection/specifications/emu_specification.json b/test/test_data/specifications/emu_specification.json similarity index 100% rename from rundetection/specifications/emu_specification.json rename to test/test_data/specifications/emu_specification.json diff --git a/rundetection/specifications/enginx_specification.json b/test/test_data/specifications/enginx_specification.json similarity index 100% rename from rundetection/specifications/enginx_specification.json rename to test/test_data/specifications/enginx_specification.json diff --git a/rundetection/specifications/evs_specification.json b/test/test_data/specifications/evs_specification.json similarity index 100% rename from rundetection/specifications/evs_specification.json rename to test/test_data/specifications/evs_specification.json diff --git a/rundetection/specifications/gem_specification.json b/test/test_data/specifications/gem_specification.json similarity index 100% rename from rundetection/specifications/gem_specification.json rename to test/test_data/specifications/gem_specification.json diff --git a/rundetection/specifications/het_specification.json b/test/test_data/specifications/het_specification.json similarity index 100% rename from rundetection/specifications/het_specification.json rename to test/test_data/specifications/het_specification.json diff --git a/rundetection/specifications/hifi_specification.json b/test/test_data/specifications/hifi_specification.json similarity index 100% rename from rundetection/specifications/hifi_specification.json rename to test/test_data/specifications/hifi_specification.json diff --git a/rundetection/specifications/hrpd_specification.json b/test/test_data/specifications/hrpd_specification.json similarity index 100% rename from rundetection/specifications/hrpd_specification.json rename to test/test_data/specifications/hrpd_specification.json diff --git a/rundetection/specifications/imat_specification.json b/test/test_data/specifications/imat_specification.json similarity index 100% rename from rundetection/specifications/imat_specification.json rename to test/test_data/specifications/imat_specification.json diff --git a/rundetection/specifications/ines_specification.json b/test/test_data/specifications/ines_specification.json similarity index 100% rename from rundetection/specifications/ines_specification.json rename to test/test_data/specifications/ines_specification.json diff --git a/rundetection/specifications/inter_specification.json b/test/test_data/specifications/inter_specification.json similarity index 100% rename from rundetection/specifications/inter_specification.json rename to test/test_data/specifications/inter_specification.json diff --git a/rundetection/specifications/iris_specification.json b/test/test_data/specifications/iris_specification.json similarity index 100% rename from rundetection/specifications/iris_specification.json rename to test/test_data/specifications/iris_specification.json diff --git a/rundetection/specifications/lad_specification.json b/test/test_data/specifications/lad_specification.json similarity index 100% rename from rundetection/specifications/lad_specification.json rename to test/test_data/specifications/lad_specification.json diff --git a/rundetection/specifications/larmor_specification.json b/test/test_data/specifications/larmor_specification.json similarity index 100% rename from rundetection/specifications/larmor_specification.json rename to test/test_data/specifications/larmor_specification.json diff --git a/rundetection/specifications/let_specification.json b/test/test_data/specifications/let_specification.json similarity index 100% rename from rundetection/specifications/let_specification.json rename to test/test_data/specifications/let_specification.json diff --git a/rundetection/specifications/loq_specification.json b/test/test_data/specifications/loq_specification.json similarity index 100% rename from rundetection/specifications/loq_specification.json rename to test/test_data/specifications/loq_specification.json diff --git a/rundetection/specifications/maps_specification.json b/test/test_data/specifications/maps_specification.json similarity index 100% rename from rundetection/specifications/maps_specification.json rename to test/test_data/specifications/maps_specification.json diff --git a/rundetection/specifications/mari_specification.json b/test/test_data/specifications/mari_specification.json similarity index 80% rename from rundetection/specifications/mari_specification.json rename to test/test_data/specifications/mari_specification.json index a694e3ea..be40cc67 100644 --- a/rundetection/specifications/mari_specification.json +++ b/test/test_data/specifications/mari_specification.json @@ -1,6 +1,6 @@ { "enabled": true, - "maristitch": true, "marimaskfile": "https://raw.githubusercontent.com/pace-neutrons/InstrumentFiles/fb8f1bb2b2430211c20a3cd8d4e2566ce24b755e/mari/mari_mask2023_4.xml", - "mariwbvan": 30031 + "mariwbvan": 30031, + "maristitch": true } diff --git a/rundetection/specifications/merlin_specification.json b/test/test_data/specifications/merlin_specification.json similarity index 100% rename from rundetection/specifications/merlin_specification.json rename to test/test_data/specifications/merlin_specification.json diff --git a/rundetection/specifications/musr_specification.json b/test/test_data/specifications/musr_specification.json similarity index 100% rename from rundetection/specifications/musr_specification.json rename to test/test_data/specifications/musr_specification.json diff --git a/rundetection/specifications/nile_specification.json b/test/test_data/specifications/nile_specification.json similarity index 100% rename from rundetection/specifications/nile_specification.json rename to test/test_data/specifications/nile_specification.json diff --git a/rundetection/specifications/nimrod_specification.json b/test/test_data/specifications/nimrod_specification.json similarity index 100% rename from rundetection/specifications/nimrod_specification.json rename to test/test_data/specifications/nimrod_specification.json diff --git a/rundetection/specifications/offspec_specification.json b/test/test_data/specifications/offspec_specification.json similarity index 100% rename from rundetection/specifications/offspec_specification.json rename to test/test_data/specifications/offspec_specification.json diff --git a/rundetection/specifications/osiris_specification.json b/test/test_data/specifications/osiris_specification.json similarity index 100% rename from rundetection/specifications/osiris_specification.json rename to test/test_data/specifications/osiris_specification.json diff --git a/rundetection/specifications/pearl_specification.json b/test/test_data/specifications/pearl_specification.json similarity index 100% rename from rundetection/specifications/pearl_specification.json rename to test/test_data/specifications/pearl_specification.json diff --git a/rundetection/specifications/polaris_specification.json b/test/test_data/specifications/polaris_specification.json similarity index 100% rename from rundetection/specifications/polaris_specification.json rename to test/test_data/specifications/polaris_specification.json diff --git a/rundetection/specifications/polref_specification.json b/test/test_data/specifications/polref_specification.json similarity index 100% rename from rundetection/specifications/polref_specification.json rename to test/test_data/specifications/polref_specification.json diff --git a/rundetection/specifications/prisma_specification.json b/test/test_data/specifications/prisma_specification.json similarity index 100% rename from rundetection/specifications/prisma_specification.json rename to test/test_data/specifications/prisma_specification.json diff --git a/rundetection/specifications/rotax_specification.json b/test/test_data/specifications/rotax_specification.json similarity index 100% rename from rundetection/specifications/rotax_specification.json rename to test/test_data/specifications/rotax_specification.json diff --git a/rundetection/specifications/sandals_specification.json b/test/test_data/specifications/sandals_specification.json similarity index 100% rename from rundetection/specifications/sandals_specification.json rename to test/test_data/specifications/sandals_specification.json diff --git a/rundetection/specifications/sans2d_specification.json b/test/test_data/specifications/sans2d_specification.json similarity index 100% rename from rundetection/specifications/sans2d_specification.json rename to test/test_data/specifications/sans2d_specification.json diff --git a/rundetection/specifications/surf_specification.json b/test/test_data/specifications/surf_specification.json similarity index 100% rename from rundetection/specifications/surf_specification.json rename to test/test_data/specifications/surf_specification.json diff --git a/rundetection/specifications/sxd_specification.json b/test/test_data/specifications/sxd_specification.json similarity index 100% rename from rundetection/specifications/sxd_specification.json rename to test/test_data/specifications/sxd_specification.json diff --git a/rundetection/specifications/tfxa_specification.json b/test/test_data/specifications/tfxa_specification.json similarity index 100% rename from rundetection/specifications/tfxa_specification.json rename to test/test_data/specifications/tfxa_specification.json diff --git a/rundetection/specifications/tosca_specification.json b/test/test_data/specifications/tosca_specification.json similarity index 100% rename from rundetection/specifications/tosca_specification.json rename to test/test_data/specifications/tosca_specification.json diff --git a/rundetection/specifications/vesuvio_specification.json b/test/test_data/specifications/vesuvio_specification.json similarity index 100% rename from rundetection/specifications/vesuvio_specification.json rename to test/test_data/specifications/vesuvio_specification.json diff --git a/rundetection/specifications/wish_specification.json b/test/test_data/specifications/wish_specification.json similarity index 100% rename from rundetection/specifications/wish_specification.json rename to test/test_data/specifications/wish_specification.json diff --git a/rundetection/specifications/zoom_specification.json b/test/test_data/specifications/zoom_specification.json similarity index 100% rename from rundetection/specifications/zoom_specification.json rename to test/test_data/specifications/zoom_specification.json diff --git a/test/test_e2e.py b/test/test_e2e.py index 21e2fe12..d4e0c464 100644 --- a/test/test_e2e.py +++ b/test/test_e2e.py @@ -57,6 +57,21 @@ def produce_message(message: str, channel: BlockingChannel) -> None: channel.basic_publish("watched-files", "", body=message.encode()) +def get_specification_from_file(instrument: str) -> Any: + """ + Given an instrument, return the specification + :param instrument: The instrument for which specification to get + :return: The specification file contents + """ + # This is ran in 2 places the CI and locally. On the CI, it has a different working directory to the default + # selected by IDEs for local testing this works with either, if this fails raising is fine. + path = Path(f"test/test_data/specifications/{instrument.lower()}_specification.json") + if not path.exists(): + path = Path(f"test_data/specifications/{instrument.lower()}_specification.json") + with path.open(encoding="utf-8") as fle: + return json.load(fle) + + def get_specification_value(instrument: str, key: str) -> Any: """ Given an instrument and key, return the specification value @@ -64,10 +79,8 @@ def get_specification_value(instrument: str, key: str) -> Any: :param key: The key for the rule :return: The rule value """ - path = Path(f"rundetection/specifications/{instrument.lower()}_specification.json") - with path.open(encoding="utf-8") as fle: - spec = json.load(fle) - return spec[key] + spec = get_specification_from_file(instrument) + return spec[key] def consume_all_messages(consumer_channel: BlockingChannel) -> list[dict[str, Any]]: diff --git a/test/test_specifications.py b/test/test_specifications.py index f015f6e0..edf1fcd9 100644 --- a/test/test_specifications.py +++ b/test/test_specifications.py @@ -2,17 +2,20 @@ Specification unit test module """ +import datetime import os from pathlib import Path -from unittest.mock import Mock, patch +from unittest import mock +from unittest.mock import patch import pytest -from _pytest.logging import LogCaptureFixture from rundetection.exceptions import RuleViolationError from rundetection.ingestion.ingest import JobRequest -from rundetection.rules.common_rules import EnabledRule -from rundetection.rules.mari_rules import MariStitchRule +from rundetection.rules.common_rules import MolSpecStitchRule +from rundetection.rules.iris_rules import IrisCalibrationRule, IrisReductionRule +from rundetection.rules.loq_rules import LoqUserFile +from rundetection.rules.mari_rules import MariWBVANRule from rundetection.specifications import InstrumentSpecification @@ -26,13 +29,21 @@ def job_request(): @pytest.fixture() -@patch("rundetection.specifications.InstrumentSpecification._load_rules") +@patch("rundetection.specifications.InstrumentSpecification._load_rules_from_api") def specification(_) -> InstrumentSpecification: """ InstrumentSpecification Fixture :return: InstrumentSpecification """ - return InstrumentSpecification("foo") + spec = InstrumentSpecification("foo") + spec.loaded_time = datetime.datetime.now(tz=datetime.UTC) + spec._rules = [mock.MagicMock(), mock.MagicMock(), mock.MagicMock()] + return spec + + +@pytest.fixture(autouse=True) +def _set_api_key() -> None: + os.environ["FIA_API_API_KEY"] = "shh" @pytest.fixture() @@ -43,101 +54,96 @@ def _working_directory_fix(): os.chdir(current_working_directory / "..") -def test_run_will_be_reduced_when_all_rules_are_ment(specification: InstrumentSpecification, job_request) -> None: - """ - Test that a job_request.will_reduce remains set to true when its relevant specification is met - :param specification: specification fixture - :param job_request: JobRequest fixture - :return: None - """ - mock_rule = Mock() - specification._rules = [mock_rule, mock_rule, mock_rule] - specification.verify(job_request) - assert job_request.will_reduce +@mock.patch("rundetection.specifications.InstrumentSpecification._load_rules_from_api") +def test_instrument_specification_tries_to_load_api_from_db(load_rules_from_api): + InstrumentSpecification("mari") + load_rules_from_api.assert_called_once_with() -def test_run_will_not_be_reduced_when_a_rule_is_not_met(specification, job_request) -> None: - """ - Test that a job_request.will_reduce will be marked as false when its relvant specification is not met. - :param specification: specification fixture - :param job_request: JobRequest fixture - :return: None - """ - mock_rule = Mock() - mock_rule.verify.side_effect = lambda r: setattr(r, "will_reduce", False) - specification._rules = [mock_rule] - specification.verify(job_request) - assert job_request.will_reduce is False +@mock.patch("rundetection.specifications.requests") +def test_instrument_specification_load_rules_for_api(requests, specification): + headers: dict = {"Authorization": "Bearer shh", "accept": "application/json"} + requests.get.return_value.json.return_value = { + "molspecstitch": True, + "mariwbvan": 100, + "loquserfile": "user_file.toml", + } + specification._load_rules_from_api() + + requests.get.assert_called_once_with( + url="http://localhost:8000/instrument/FOO/specification", headers=headers, timeout=1 + ) + assert specification._rules == [MariWBVANRule(100), LoqUserFile("user_file.toml"), MolSpecStitchRule(True)] + + +@mock.patch("rundetection.specifications.requests") +def test_instrument_specification_load_rules_for_api_sets_loaded_time(requests, specification): + headers: dict = {"Authorization": "Bearer shh", "accept": "application/json"} + specification._load_rules_from_api() + + requests.get.assert_called_once_with( + url="http://localhost:8000/instrument/FOO/specification", headers=headers, timeout=1 + ) + assert specification.loaded_time is not None + assert datetime.timedelta(minutes=1) > datetime.datetime.now(tz=datetime.UTC) - specification.loaded_time + + +def test_rule_old(specification): + specification.loaded_time = datetime.datetime.now(tz=datetime.UTC) - datetime.timedelta(minutes=11) + assert specification._rule_old() + + specification.loaded_time = datetime.datetime.now(tz=datetime.UTC) + assert not specification._rule_old() + + specification.loaded_time = None + assert specification._rule_old() + + +def test_specification_rule_old(specification, job_request): + specification._rule_old = mock.MagicMock(return_value=True) + specification._load_rules_from_api = mock.MagicMock() -def test_run_will_not_reduce_when_a_rule_violation_occurs(specification, job_request) -> None: - """ - When a rule violation is raised, a job_request.will_reduce will be marked as false. - :param specification: spec fixture - :param job_request: job request fixture - :return: None - """ - mock_rule = Mock() - mock_rule.verify.side_effect = RuleViolationError - specification._rules = [mock_rule] specification.verify(job_request) - assert job_request.will_reduce is False + specification._load_rules_from_api.assert_called_once_with() + + +def test_specification__rules_len_0(specification, job_request): + specification._rules = [] -def test_specification_will_stop_checking_rules_on_first_failure(specification, job_request) -> None: - """ - Tests that no further rules will be checked as soon as one fails - :param specification: specification fixutre - :param job_request: JobRequest fixture - :return: None - """ - first_rule = Mock() - first_rule.verify.side_effect = lambda r: setattr(r, "will_reduce", False) - second_rule = Mock() - specification._rules = [first_rule, second_rule] specification.verify(job_request) - first_rule.verify.assert_called_once_with(job_request) - second_rule.verify.assert_not_called() - assert job_request.will_reduce is False + assert not job_request.will_reduce -def test_run_will_not_be_reduced_for_a_no_rule_specification(specification, job_request) -> None: - """ - Test that job_request.will_reduce will be set to false when there are no rules in the relevant specification - :param specification: Specification fixture - :param job_request: JobRequest fixture - :return: None - """ + +def test_runs_verify_on_all_rules(specification, job_request): specification.verify(job_request) - assert job_request.will_reduce is False + assert job_request.will_reduce + for rule in specification._rules: + rule.verify.assert_called_once_with(job_request) -@pytest.mark.usefixtures("_working_directory_fix") -def test_specification_rule_loading(job_request) -> None: - """ - Test that the correct spec for each instrument is loaded. - :param job_request: Run Fixture - :return: None - """ - mari_specification = InstrumentSpecification("mari") - chronus_specification = InstrumentSpecification("chronus") - assert isinstance(mari_specification._rules[0], EnabledRule) - assert mari_specification._rules[0]._value - assert isinstance(mari_specification._rules[1], MariStitchRule) - assert mari_specification._rules[1]._value +def test_specification_verify_rule_violation_doesnt_verify_more(specification, job_request): + def raise_exception(_): + raise RuleViolationError() - assert isinstance(chronus_specification._rules[0], EnabledRule) - assert chronus_specification._rules[0]._value is False + specification._rules[1].verify = mock.MagicMock(side_effect=raise_exception) + specification.verify(job_request) -def test_specification_file_missing(caplog: LogCaptureFixture): - """ - Test logging and exception raised when specification file is missing - :param caplog: - :return: - """ - with pytest.raises(FileNotFoundError): - InstrumentSpecification("foo") + assert not job_request.will_reduce + specification._rules[0].verify.assert_called_once() + specification._rules[1].verify.assert_called_once() + specification._rules[2].verify.assert_not_called() + + +def test_specification_ensures_rule_order_respected(specification): + specification._rules = [IrisReductionRule(True), MolSpecStitchRule(True), IrisCalibrationRule({})] + + specification._order_rules() - assert "No specification for file: foo" in caplog.text + assert specification._rules[0] == IrisReductionRule(True) + assert specification._rules[1] == IrisCalibrationRule({}) + assert specification._rules[-1] == MolSpecStitchRule(True)