From 0d4aec711f14d306a2afd9ef02f0b3388f464713 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Fri, 26 Jun 2026 00:50:25 -0400 Subject: [PATCH 1/7] Migrate pytest config into pyproject.toml and add src/ build config Move the timeout setting out of tests/pytest.ini into [tool.pytest.ini_options], add testpaths=["tests"] so a bare pytest no longer scans the website node_modules, and add a hatchling build-system that packages src/ so `from src.babel_validation...` imports resolve when installed. Add filelock, used by the Google Sheet disk cache. Co-Authored-By: Claude Opus 4.8 --- pyproject.toml | 16 ++++++++++++++++ tests/pytest.ini | 4 ---- 2 files changed, 16 insertions(+), 4 deletions(-) delete mode 100644 tests/pytest.ini diff --git a/pyproject.toml b/pyproject.toml index 176b30c..c5fbd4e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,6 +8,7 @@ requires-python = ">=3.11" dependencies = [ "black>=25.9.0", "requests>=2.32.5", + "filelock", "deepdiff>=8.6.1", "openapi-spec-validator>=0.7.2", "pytest>=8.4.2", @@ -16,3 +17,18 @@ dependencies = [ [project.urls] Repository = "https://github.com/TranslatorSRI/babel-validation" + +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +[tool.hatch.build.targets.wheel] +# Package the entire `src/` directory so existing imports +# (`from src.babel_validation.X import Y`) continue to work after install. +packages = ["src"] + +[tool.pytest.ini_options] +# Without testpaths, a bare `pytest` would also scan the website directories +# (including node_modules) during collection. +testpaths = ["tests"] +timeout = 300 diff --git a/tests/pytest.ini b/tests/pytest.ini deleted file mode 100644 index 209c8a5..0000000 --- a/tests/pytest.ini +++ /dev/null @@ -1,4 +0,0 @@ -# pytest.ini settings -[pytest] -# Timeout of 5 minutes (300 seconds) -timeout = 300 From f90d162cffc6ff141063db91eefb4b9033ec5c22 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Fri, 26 Jun 2026 00:51:02 -0400 Subject: [PATCH 2/7] Move shared test utilities into src/babel_validation package Relocate the code under tests/common into an importable src/babel_validation package, splitting the monolithic google_sheet_test_cases module into core (TestRow/TestStatus/TestResult), services (CachedNodeNorm, CachedNameRes) and sources/google_sheets (GoogleSheetTestCases, blocklist). Update the Google Sheet test modules to import from the new locations and to parametrize lazily in pytest_generate_tests via the new tests/_pytest_helpers.deselected_by_markexpr, so marker-deselected runs (e.g. `pytest -m unit`) never hit the network. conftest.py wipes the Google Sheet disk cache at the start of each run. Move test_env.py into tests/test_environment/. Co-Authored-By: Claude Opus 4.8 --- CLAUDE.md | 17 +++- {tests/common => src}/__init__.py | 0 src/babel_validation/__init__.py | 0 src/babel_validation/core/__init__.py | 0 src/babel_validation/core/testrow.py | 72 +++++++++++++++ src/babel_validation/services/__init__.py | 0 src/babel_validation/services/nameres.py | 76 ++++++++++++++++ src/babel_validation/services/nodenorm.py | 69 ++++++++++++++ src/babel_validation/sources/__init__.py | 0 .../sources/google_sheets/__init__.py | 0 .../sources/google_sheets}/blocklist.py | 0 .../google_sheets}/google_sheet_test_cases.py | 90 +++++++------------ tests/__init__.py | 0 tests/_pytest_helpers.py | 31 +++++++ tests/conftest.py | 20 +++++ tests/nameres/test_blocklist.py | 26 +++++- tests/nameres/test_nameres_from_gsheet.py | 34 +++++-- tests/nodenorm/test_nodenorm_descriptions.py | 8 +- tests/nodenorm/test_nodenorm_from_gsheet.py | 33 +++++-- tests/{ => test_environment}/test_env.py | 4 +- uv.lock | 13 ++- 21 files changed, 406 insertions(+), 87 deletions(-) rename {tests/common => src}/__init__.py (100%) create mode 100644 src/babel_validation/__init__.py create mode 100644 src/babel_validation/core/__init__.py create mode 100644 src/babel_validation/core/testrow.py create mode 100644 src/babel_validation/services/__init__.py create mode 100644 src/babel_validation/services/nameres.py create mode 100644 src/babel_validation/services/nodenorm.py create mode 100644 src/babel_validation/sources/__init__.py create mode 100644 src/babel_validation/sources/google_sheets/__init__.py rename {tests/common => src/babel_validation/sources/google_sheets}/blocklist.py (100%) rename {tests/common => src/babel_validation/sources/google_sheets}/google_sheet_test_cases.py (57%) create mode 100644 tests/__init__.py create mode 100644 tests/_pytest_helpers.py rename tests/{ => test_environment}/test_env.py (79%) diff --git a/CLAUDE.md b/CLAUDE.md index 5b8c06a..ba81644 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -46,21 +46,29 @@ cd website && npm install && npm run dev # Dev server at localhost:4321 ## Architecture +### Library (`src/babel_validation/`) + +Shared library code used by the tests and potentially other consumers. + +- `core/testrow.py` — `TestRow` dataclass (models a single Google Sheet test row), `TestStatus` enum, `TestResult` dataclass +- `services/nodenorm.py` — `CachedNodeNorm`: wraps the NodeNorm `get_normalized_nodes` API with per-instance caching +- `services/nameres.py` — `CachedNameRes`: wraps the NameRes `lookup`/`bulk-lookup` APIs with per-instance caching +- `sources/google_sheets/google_sheet_test_cases.py` — `GoogleSheetTestCases`: downloads and parses the shared Google Sheet into `TestRow` instances and pytest `ParameterSet` lists + ### Test Framework (`tests/`) The core of this project. Tests validate NodeNorm and NameRes services across multiple deployment environments. **Target system:** `tests/targets.ini` defines endpoints for each environment (dev, prod, test, ci, exp, localhost). Tests use `target_info` fixture to get URLs. The `conftest.py` parametrizes tests across targets via `--target` CLI option; default is `dev`. -**Google Sheet integration:** ~2000+ test cases are pulled from a [shared Google Sheet](https://docs.google.com/spreadsheets/d/11zebx8Qs1Tc3ShQR9nh4HRW8QSoo8k65w_xIaftN0no/). `tests/common/google_sheet_test_cases.py` fetches and parses these into `TestRow` dataclasses. Rows marked as not expected to pass are wrapped with `pytest.mark.xfail(strict=True)`. Tests are parametrized by row, with IDs like `gsheet:row=42`. +**Google Sheet integration:** ~2000+ test cases are pulled from a [shared Google Sheet](https://docs.google.com/spreadsheets/d/11zebx8Qs1Tc3ShQR9nh4HRW8QSoo8k65w_xIaftN0no/). `src/babel_validation/sources/google_sheets/google_sheet_test_cases.py` fetches and parses these into `TestRow` dataclasses. Rows marked as not expected to pass are wrapped with `pytest.mark.xfail(strict=True)`. Tests are parametrized by row, with IDs like `gsheet:row=42`. **Category filtering:** Google Sheet rows have a Category column. The `test_category` fixture (from conftest.py) returns a callable that tests use to `pytest.skip()` rows not matching `--category`/`--category-exclude` filters. **Test modules:** - `tests/nodenorm/` — NodeNorm tests (normalization accuracy, preferred IDs/labels, Biolink types, conflation, descriptions, OpenAPI spec, setid endpoint) - `tests/nameres/` — NameRes tests (label lookup, autocomplete, Biolink type filtering, blocklist, taxon_specific flag) -- `tests/nodenorm/by_issue/` — Tests tied to specific GitHub issues -- `tests/common/` — Shared utilities (`GoogleSheetTestCases`, `TestRow`) +- `tests/nodenorm/by_issue/` — Per-issue regression tests for NodeNorm (hand-written) ### Web Applications @@ -79,4 +87,5 @@ When writing new tests: - Use the `target_info` fixture to get NodeNorm/NameRes URLs from targets.ini - For Google Sheet-based tests, parametrize with `gsheet.test_rows()` and use the `test_category` fixture for category filtering - Use `pytest.mark.xfail(strict=True)` for known failures (strict=True means unexpected passes also fail) -- Issue-specific tests go in `tests/nodenorm/by_issue/` or `tests/github_issues/` +- Hand-written per-issue regression tests go in `tests/nodenorm/by_issue/` +- Import shared classes from `src.babel_validation.*` (e.g. `from src.babel_validation.services.nodenorm import CachedNodeNorm`) diff --git a/tests/common/__init__.py b/src/__init__.py similarity index 100% rename from tests/common/__init__.py rename to src/__init__.py diff --git a/src/babel_validation/__init__.py b/src/babel_validation/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/babel_validation/core/__init__.py b/src/babel_validation/core/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/babel_validation/core/testrow.py b/src/babel_validation/core/testrow.py new file mode 100644 index 0000000..b03a55e --- /dev/null +++ b/src/babel_validation/core/testrow.py @@ -0,0 +1,72 @@ +from dataclasses import dataclass +from enum import Enum + + +@dataclass(frozen=True) +class TestRow: + """ + A TestRow models a single row from a GoogleSheet. + """ + Category: str + ExpectPassInNodeNorm: bool + ExpectPassInNameRes: bool + Flags: set[str] + QueryLabel: str + PreferredLabel: str + AdditionalLabels: list[str] + QueryID: str + PreferredID: str + AdditionalIDs: list[str] + Conflations: set[str] + BiolinkClasses: set[str] + Prefixes: set[str] + Source: str + SourceURL: str + Notes: str + + # Mark as not a test despite starting with Test*. + __test__ = False + + # A string representation of this test row. + def __str__(self): + return f"TestRow of category {self.Category} for preferred {self.PreferredID} ({self.PreferredLabel}) with " + \ + f"query {self.QueryID} ({self.QueryLabel}) from source {self.Source} ({self.SourceURL})" + + + @staticmethod + def from_data_row(row): + return TestRow( + Category=row.get('Category', ''), + ExpectPassInNodeNorm=row.get('Passes in NodeNorm', '').strip().lower() == 'y', + ExpectPassInNameRes=row.get('Passes in NameRes', '').strip().lower() == 'y', + Flags=set(row.get('Flags', '').split('|')), + QueryLabel=row.get('Query Label', ''), + QueryID=row.get('Query ID', ''), + PreferredID=row.get('Preferred ID', ''), + AdditionalIDs=row.get('Additional IDs', '').split('|'), + PreferredLabel=row.get('Preferred Label', ''), + AdditionalLabels=row.get('Additional Labels', '').split('|'), + Conflations=set(row.get('Conflations', '').split('|')), + BiolinkClasses=set(row.get('Biolink Classes', '').split('|')), + Prefixes=set(row.get('Prefixes', '').split('|')), + Source=row.get('Source', ''), + SourceURL=row.get('Source URL', ''), + Notes=row.get('Notes', '') + ) + +class TestStatus(Enum): + Passed = "pass" + Failed = "fail" + Skipped = "skip" + + # Mark as not a test despite starting with Test*. + __test__ = False + +@dataclass +class TestResult: + status: TestStatus + message: str = "" + + # Mark as not a test despite starting with Test*. + __test__ = False + diff --git a/src/babel_validation/services/__init__.py b/src/babel_validation/services/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/babel_validation/services/nameres.py b/src/babel_validation/services/nameres.py new file mode 100644 index 0000000..01d36bb --- /dev/null +++ b/src/babel_validation/services/nameres.py @@ -0,0 +1,76 @@ +import logging +import time + +import requests + +cached_nameres_by_url = {} + +class CachedNameRes: + def __init__(self, nameres_url: str): + self.nameres_url = nameres_url + self.logger = logging.getLogger(str(self)) + self.cache = {} + + def __str__(self): + return f"CachedNameRes({self.nameres_url})" + + @staticmethod + def from_url(nameres_url: str) -> 'CachedNameRes': + if nameres_url not in cached_nameres_by_url: + cached_nameres_by_url[nameres_url] = CachedNameRes(nameres_url) + return cached_nameres_by_url[nameres_url] + + def bulk_lookup(self, queries: list[str], **params) -> dict[str, dict]: + if not queries: + raise ValueError(f"queries must not be empty when calling bulk_lookup({queries}, {params}) on {self}") + if not isinstance(queries, list): + raise ValueError(f"queries must be a list when calling bulk_lookup({queries}, {params}) on {self}") + + time_started = time.time_ns() + params_key = frozenset(params.items()) + queries_set = set(queries) + cached_queries = {q for q in queries_set if (q, params_key) in self.cache} + queries_to_be_queried = queries_set - cached_queries + + result = {} + if queries_to_be_queried: + api_params = dict(params) + api_params['strings'] = list(queries_to_be_queried) + + self.logger.debug("Called NameRes %s with params %s", self, api_params) + response = requests.post(self.nameres_url + "bulk-lookup", json=api_params, timeout=30) + response.raise_for_status() + result = response.json() + + for query in queries_to_be_queried: + self.cache[(query, params_key)] = result.get(query, None) + + for query in cached_queries: + result[query] = self.cache[(query, params_key)] + + time_taken_sec = (time.time_ns() - time_started) / 1E9 + self.logger.info("Looked up %d queries (with %d cached) with params %s on %s in %.3fs", + len(queries_to_be_queried), len(cached_queries), params, self, time_taken_sec) + + return result + + def lookup(self, query, **params): + cache_key = (query, frozenset(params.items())) + if cache_key in self.cache: + return self.cache[cache_key] + + api_params = dict(params) + api_params['string'] = query + self.logger.debug("Querying NameRes with params %s", api_params) + + response = requests.post(self.nameres_url + "lookup", params=api_params, timeout=30) + response.raise_for_status() + result = response.json() + + self.cache[cache_key] = result + return result + + def delete_query(self, query): + keys_to_delete = [k for k in self.cache if k[0] == query] + for k in keys_to_delete: + del self.cache[k] diff --git a/src/babel_validation/services/nodenorm.py b/src/babel_validation/services/nodenorm.py new file mode 100644 index 0000000..633f7a4 --- /dev/null +++ b/src/babel_validation/services/nodenorm.py @@ -0,0 +1,69 @@ +import logging +import time + +import requests + +cached_node_norms_by_url = {} + +class CachedNodeNorm: + def __init__(self, nodenorm_url: str): + self.nodenorm_url = nodenorm_url + self.logger = logging.getLogger(str(self)) + self.cache = {} + + def __str__(self): + return f"CachedNodeNorm({self.nodenorm_url})" + + @staticmethod + def from_url(nodenorm_url: str) -> 'CachedNodeNorm': + if nodenorm_url not in cached_node_norms_by_url: + cached_node_norms_by_url[nodenorm_url] = CachedNodeNorm(nodenorm_url) + return cached_node_norms_by_url[nodenorm_url] + + def normalize_curies(self, curies: list[str], **params) -> dict[str, dict]: + if not curies: + raise ValueError(f"curies must not be empty when calling normalize_curies({curies}, {params}) on {self}") + if not isinstance(curies, list): + raise ValueError(f"curies must be a list when calling normalize_curies({curies}, {params}) on {self}") + + time_started = time.time_ns() + params_key = frozenset(params.items()) + curies_set = set(curies) + cached_curies = {c for c in curies_set if (c, params_key) in self.cache} + curies_to_be_queried = curies_set - cached_curies + + # Make query. + result = {} + if curies_to_be_queried: + api_params = dict(params) + api_params['curies'] = list(curies_to_be_queried) + + self.logger.debug("Called NodeNorm %s with params %s", self, api_params) + response = requests.post(self.nodenorm_url + "get_normalized_nodes", json=api_params, timeout=30) + response.raise_for_status() + result = response.json() + + for curie in curies_to_be_queried: + self.cache[(curie, params_key)] = result.get(curie, None) + + for curie in cached_curies: + result[curie] = self.cache[(curie, params_key)] + + time_taken_sec = (time.time_ns() - time_started) / 1E9 + self.logger.info("Normalizing %d CURIEs %s (with %d CURIEs cached) with params %s on %s in %.3fs", + len(curies_to_be_queried), curies_to_be_queried, len(cached_curies), params, self, time_taken_sec) + + return result + + def normalize_curie(self, curie, **params): + cache_key = (curie, frozenset(params.items())) + if cache_key in self.cache: + return self.cache[cache_key] + # Use .get(): NodeNorm normally echoes every requested CURIE (null when + # unresolvable), but don't crash if it ever omits one. + return self.normalize_curies([curie], **params).get(curie) + + def clear_curie(self, curie): + keys_to_delete = [k for k in self.cache if k[0] == curie] + for k in keys_to_delete: + del self.cache[k] diff --git a/src/babel_validation/sources/__init__.py b/src/babel_validation/sources/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/babel_validation/sources/google_sheets/__init__.py b/src/babel_validation/sources/google_sheets/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/common/blocklist.py b/src/babel_validation/sources/google_sheets/blocklist.py similarity index 100% rename from tests/common/blocklist.py rename to src/babel_validation/sources/google_sheets/blocklist.py diff --git a/tests/common/google_sheet_test_cases.py b/src/babel_validation/sources/google_sheets/google_sheet_test_cases.py similarity index 57% rename from tests/common/google_sheet_test_cases.py rename to src/babel_validation/sources/google_sheets/google_sheet_test_cases.py index aecdda9..935b019 100644 --- a/tests/common/google_sheet_test_cases.py +++ b/src/babel_validation/sources/google_sheets/google_sheet_test_cases.py @@ -3,66 +3,19 @@ # # This library contains classes and methods for accessing those test cases. import csv +import hashlib import io -from dataclasses import dataclass +import tempfile +import time from collections import Counter +from pathlib import Path import pytest import requests from _pytest.mark import ParameterSet +from filelock import FileLock - -@dataclass(frozen=True) -class TestRow: - """ - A TestRow models a single row from a GoogleSheet. - """ - Category: str - ExpectPassInNodeNorm: bool - ExpectPassInNameRes: bool - Flags: set[str] - QueryLabel: str - PreferredLabel: str - AdditionalLabels: list[str] - QueryID: str - PreferredID: str - AdditionalIDs: list[str] - Conflations: set[str] - BiolinkClasses: set[str] - Prefixes: set[str] - Source: str - SourceURL: str - Notes: str - - # Mark as not a test despite starting with TestRow. - __test__ = False - - # A string representation of this test row. - def __str__(self): - return f"TestRow of category {self.Category} for preferred {self.PreferredID} ({self.PreferredLabel}) with " + \ - f"query {self.QueryID} ({self.QueryLabel}) from source {self.Source} ({self.SourceURL})" - - - @staticmethod - def from_data_row(row): - return TestRow( - Category=row.get('Category', ''), - ExpectPassInNodeNorm=row.get('Passes in NodeNorm', '') == 'y', - ExpectPassInNameRes=row.get('Passes in NameRes', '') == 'y', - Flags=set(row.get('Flags', '').split('|')), - QueryLabel=row.get('Query Label', ''), - QueryID=row.get('Query ID', ''), - PreferredID=row.get('Preferred ID', ''), - AdditionalIDs=row.get('Additional IDs', '').split('|'), - PreferredLabel=row.get('Preferred Label', ''), - AdditionalLabels=row.get('Additional Labels', '').split('|'), - Conflations=set(row.get('Conflations', '').split('|')), - BiolinkClasses=set(row.get('Biolink Classes', '').split('|')), - Prefixes=set(row.get('Prefixes', '').split('|')), - Source=row.get('Source', ''), - SourceURL=row.get('Source URL', ''), - Notes=row.get('Notes', '') - ) +from src.babel_validation.core.testrow import TestRow class GoogleSheetTestCases: @@ -73,6 +26,11 @@ class GoogleSheetTestCases: def __str__(self): return f"Google Sheet Test Cases ({len(self.rows)} test cases from {self.google_sheet_id})" + # How long a cached download stays valid. pytest deletes the cache at the + # start of every run (see tests/conftest.py), so this TTL mainly protects + # other consumers (e.g. csv-to-babeltests) from reading stale data forever. + CACHE_TTL_SECONDS = 3600 + def __init__(self, google_sheet_id="11zebx8Qs1Tc3ShQR9nh4HRW8QSoo8k65w_xIaftN0no"): """ Create a Google Sheet test case. @@ -80,9 +38,20 @@ def __init__(self, google_sheet_id="11zebx8Qs1Tc3ShQR9nh4HRW8QSoo8k65w_xIaftN0no """ self.google_sheet_id = google_sheet_id - csv_url = f"https://docs.google.com/spreadsheets/d/{google_sheet_id}/gviz/tq?tqx=out:csv&sheet=Tests" - response = requests.get(csv_url) - self.csv_content = response.text + + sheet_hash = hashlib.md5(google_sheet_id.encode()).hexdigest()[:8] + cache_file = Path(tempfile.gettempdir()) / f"babel_validation_gsheet_{sheet_hash}.csv" + lock_file = cache_file.with_suffix(".lock") + + with FileLock(lock_file): + if cache_file.exists() and time.time() - cache_file.stat().st_mtime < self.CACHE_TTL_SECONDS: + self.csv_content = cache_file.read_text(encoding="utf-8") + else: + csv_url = f"https://docs.google.com/spreadsheets/d/{google_sheet_id}/gviz/tq?tqx=out:csv&sheet=Tests" + response = requests.get(csv_url, timeout=30) + response.raise_for_status() + self.csv_content = response.text + cache_file.write_text(self.csv_content, encoding="utf-8") self.rows = [] with io.StringIO(self.csv_content) as f: @@ -106,7 +75,8 @@ def has_nonempty_value(d: dict): for count, row in enumerate(self.rows): # Note that count is off by two: presumably one for the header row and one because we count from zero # but Google Sheets counts from one. - row_id = f"{test_id_prefix}:row={count + 2}" + row_count = count + 2 + row_id = f"{test_id_prefix}:row={row_count}" if has_nonempty_value(row): tr = TestRow.from_data_row(row) @@ -118,7 +88,7 @@ def has_nonempty_value(d: dict): trows.append(pytest.param( tr, marks=pytest.mark.xfail( - reason=f"Test row {count + 2} is marked as not expected to pass NodeNorm in the " + reason=f"Test row {row_count} is marked as not expected to pass NodeNorm in the " f"Google Sheet: {tr}", strict=True), id=row_id @@ -131,7 +101,7 @@ def has_nonempty_value(d: dict): trows.append(pytest.param( tr, marks=pytest.mark.xfail( - reason=f"Test row {count + 2} is marked as not expected to pass NameRes in the " + reason=f"Test row {row_count} is marked as not expected to pass NameRes in the " f"Google Sheet: {tr}", strict=True), id=row_id @@ -141,4 +111,4 @@ def has_nonempty_value(d: dict): def categories(self): """ Return a dict of all the categories of tests available with their counts. """ - return Counter(map(lambda t: t.get('Category', ''), self.rows)) \ No newline at end of file + return Counter(map(lambda t: t.get('Category', ''), self.rows)) diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/_pytest_helpers.py b/tests/_pytest_helpers.py new file mode 100644 index 0000000..fa84dca --- /dev/null +++ b/tests/_pytest_helpers.py @@ -0,0 +1,31 @@ +"""Shared pytest helpers for deferring network-backed parametrization. + +Several test modules build their parametrization from network sources (the +Google Sheet, GitHub issues) at collection time. pytest only applies ``-m`` +marker deselection *after* test generation, so a run like ``pytest -m unit`` +would still pay for those fetches before discarding the tests. Evaluating the +marker expression ourselves lets us skip the fetch when the test won't run. +""" + + +def deselected_by_markexpr(metafunc) -> bool: + """True if an active ``-m`` marker expression would deselect this test. + + Returns False (i.e. "keep it") whenever there is no ``-m`` filter, the + internal expression API is unavailable, or the expression can't be parsed — + so this never suppresses a test that pytest would otherwise run. + """ + markexpr = metafunc.config.getoption("markexpr") + if not markexpr: + return False + try: + from _pytest.mark.expression import Expression + except ImportError: + # Internal API moved; fall back to the (network-using) default. + return False + own_markers = {m.name for m in metafunc.definition.iter_markers()} + try: + return not Expression.compile(markexpr).evaluate(lambda name: name in own_markers) + except Exception: + # Unparseable expression — let pytest handle it; don't suppress tests. + return False diff --git a/tests/conftest.py b/tests/conftest.py index 2515e22..96e75f7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,7 +1,10 @@ # # conftest.py - pytest configuration settings # +import glob +import os import os.path +import tempfile import pytest import configparser @@ -25,6 +28,23 @@ def get_targets_ini_path(config): return config_path +def _silent_unlink(path: str) -> None: + try: + os.unlink(path) + except FileNotFoundError: + pass + + +def pytest_configure(config): + # Delete the Google Sheet CSV cache at the start of each run so tests always + # use a fresh download. Only the controller does this — xdist workers skip it + # so they can share the cache file written by the controller. + if not os.environ.get('PYTEST_XDIST_WORKER'): + for f in glob.glob(os.path.join(tempfile.gettempdir(), 'babel_validation_gsheet_*.csv')): + _silent_unlink(f) + _silent_unlink(f.removesuffix('.csv') + '.lock') + + def pytest_addoption(parser): # The target environment(s) to target. parser.addoption( diff --git a/tests/nameres/test_blocklist.py b/tests/nameres/test_blocklist.py index 1d6df4d..c1d803d 100644 --- a/tests/nameres/test_blocklist.py +++ b/tests/nameres/test_blocklist.py @@ -3,13 +3,31 @@ import requests import pytest -from tests.common.blocklist import load_blocklist_from_gsheet +from src.babel_validation.sources.google_sheets.blocklist import load_blocklist_from_gsheet +from tests._pytest_helpers import deselected_by_markexpr -# Parameterize blocklist entries. -blocklist_entries = load_blocklist_from_gsheet() +# The blocklist Google Sheet is downloaded lazily in pytest_generate_tests so +# that runs which deselect these tests (e.g. `pytest -m unit`) never hit the +# network. +_blocklist_entries = None + + +def _get_blocklist_entries(): + global _blocklist_entries + if _blocklist_entries is None: + _blocklist_entries = load_blocklist_from_gsheet() + return _blocklist_entries + + +def pytest_generate_tests(metafunc): + if "blocklist_entry" not in metafunc.fixturenames: + return + if deselected_by_markexpr(metafunc): + metafunc.parametrize("blocklist_entry", []) + return + metafunc.parametrize("blocklist_entry", _get_blocklist_entries()) -@pytest.mark.parametrize("blocklist_entry", blocklist_entries) def test_check_blocklist_entry(target_info, blocklist_entry, categories_include): """ Test whether a NameRes instance has blocked every item from a blocklist. diff --git a/tests/nameres/test_nameres_from_gsheet.py b/tests/nameres/test_nameres_from_gsheet.py index 199e074..1b9403f 100644 --- a/tests/nameres/test_nameres_from_gsheet.py +++ b/tests/nameres/test_nameres_from_gsheet.py @@ -1,16 +1,38 @@ import urllib.parse import requests import pytest -from common.google_sheet_test_cases import GoogleSheetTestCases, TestRow +from src.babel_validation.sources.google_sheets.google_sheet_test_cases import GoogleSheetTestCases +from tests._pytest_helpers import deselected_by_markexpr # Configuration options NAMERES_TIMEOUT = 10 # If we don't get a response in 10 seconds, that's a fail. -# We generate a set of tests from the GoogleSheetTestCases. -gsheet = GoogleSheetTestCases() +# The Google Sheet is downloaded lazily in pytest_generate_tests so that runs +# which deselect these tests (e.g. `pytest -m unit`) never hit the network. +_gsheet = None + + +def _get_gsheet() -> GoogleSheetTestCases: + global _gsheet + if _gsheet is None: + _gsheet = GoogleSheetTestCases() + return _gsheet + + +def pytest_generate_tests(metafunc): + if "test_row" not in metafunc.fixturenames: + return + if deselected_by_markexpr(metafunc): + metafunc.parametrize("test_row", []) + return + metafunc.parametrize( + "test_row", + _get_gsheet().test_rows( + 'test_nameres_from_gsheet.test_label', test_nodenorm=False, test_nameres=True + ), + ) -@pytest.mark.parametrize("test_row", gsheet.test_rows('test_nameres_from_gsheet.test_label', test_nodenorm=False, test_nameres=True)) def test_label(target_info, test_row, test_category): nameres_url = target_info['NameResURL'] limit = target_info['NameResLimit'] @@ -56,8 +78,8 @@ def test_label(target_info, test_row, test_category): request = { "string": label, "autocomplete": autocomplete_flag, - "biolink_type": biolink_class, - "limit": limit + "biolink_type": [biolink_class], + "limit": limit, } if test_row.Prefixes: only_prefixes = [] diff --git a/tests/nodenorm/test_nodenorm_descriptions.py b/tests/nodenorm/test_nodenorm_descriptions.py index 7583194..1b82c6f 100644 --- a/tests/nodenorm/test_nodenorm_descriptions.py +++ b/tests/nodenorm/test_nodenorm_descriptions.py @@ -6,15 +6,15 @@ import pytest import requests -IDENTIFIERS_WITH_DESCRIPTIONS = { +IDENTIFIERS_WITH_DESCRIPTIONS = [ 'MESH:D014867', 'NCIT:C34373', 'NCBIGene:1756', -} +] -IDENTIFIERS_WITHOUT_DESCRIPTIONS = { +IDENTIFIERS_WITHOUT_DESCRIPTIONS = [ 'UMLS:C0665297', # natalizumab -} +] @pytest.mark.parametrize('curie', IDENTIFIERS_WITH_DESCRIPTIONS) def test_descriptions(target_info, curie): diff --git a/tests/nodenorm/test_nodenorm_from_gsheet.py b/tests/nodenorm/test_nodenorm_from_gsheet.py index 5dfe43f..057e8a7 100644 --- a/tests/nodenorm/test_nodenorm_from_gsheet.py +++ b/tests/nodenorm/test_nodenorm_from_gsheet.py @@ -1,14 +1,35 @@ -import itertools import urllib.parse import requests import pytest -from common.google_sheet_test_cases import GoogleSheetTestCases, TestRow +from src.babel_validation.sources.google_sheets.google_sheet_test_cases import GoogleSheetTestCases +from tests._pytest_helpers import deselected_by_markexpr -# We generate a set of tests from the GoogleSheetTestCases. -gsheet = GoogleSheetTestCases() +# The Google Sheet is downloaded lazily in pytest_generate_tests so that runs +# which deselect these tests (e.g. `pytest -m unit`) never hit the network. +_gsheet = None + + +def _get_gsheet() -> GoogleSheetTestCases: + global _gsheet + if _gsheet is None: + _gsheet = GoogleSheetTestCases() + return _gsheet + + +def pytest_generate_tests(metafunc): + if "test_row" not in metafunc.fixturenames: + return + if deselected_by_markexpr(metafunc): + metafunc.parametrize("test_row", []) + return + metafunc.parametrize( + "test_row", + _get_gsheet().test_rows( + 'test_nodenorm_from_gsheet.test_row', test_nodenorm=True, test_nameres=False + ), + ) -@pytest.mark.parametrize("test_row", gsheet.test_rows('test_nodenorm_from_gsheet.test_row', test_nodenorm=True, test_nameres=False)) def test_normalization(target_info, test_row, test_category): nodenorm_url = target_info['NodeNormURL'] @@ -89,4 +110,4 @@ def test_normalization(target_info, test_row, test_category): f"found in types: {biolink_types}") else: assert biolink_type in set(biolink_types), (f"{test_summary} biolink type {biolink_type} not found in " - f"types: {biolink_types}") \ No newline at end of file + f"types: {biolink_types}") diff --git a/tests/test_env.py b/tests/test_environment/test_env.py similarity index 79% rename from tests/test_env.py rename to tests/test_environment/test_env.py index 87a4f6a..5302198 100644 --- a/tests/test_env.py +++ b/tests/test_environment/test_env.py @@ -1,7 +1,7 @@ # Test whether the test environment is functional. import json -from common.google_sheet_test_cases import GoogleSheetTestCases +from src.babel_validation.sources.google_sheets.google_sheet_test_cases import GoogleSheetTestCases def test_google_sheet_has_test_cases(): @@ -13,4 +13,4 @@ def test_google_sheet_has_test_cases(): print(f"Found {len(gsheet.rows)} test cases in {gsheet}: {json.dumps(gsheet.rows[:10], indent=2)}") categories = gsheet.categories() - assert 'Unit Tests' in categories \ No newline at end of file + assert 'Unit Tests' in categories diff --git a/uv.lock b/uv.lock index 53443ce..18f74c6 100644 --- a/uv.lock +++ b/uv.lock @@ -23,10 +23,11 @@ wheels = [ [[package]] name = "babel-validation" version = "0.1.0" -source = { virtual = "." } +source = { editable = "." } dependencies = [ { name = "black" }, { name = "deepdiff" }, + { name = "filelock" }, { name = "openapi-spec-validator" }, { name = "pytest" }, { name = "pytest-timeout" }, @@ -37,6 +38,7 @@ dependencies = [ requires-dist = [ { name = "black", specifier = ">=25.9.0" }, { name = "deepdiff", specifier = ">=8.6.1" }, + { name = "filelock" }, { name = "openapi-spec-validator", specifier = ">=0.7.2" }, { name = "pytest", specifier = ">=8.4.2" }, { name = "pytest-timeout", specifier = ">=2.4.0" }, @@ -195,6 +197,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/f7/e6/efe534ef0952b531b630780e19cabd416e2032697019d5295defc6ef9bd9/deepdiff-8.6.1-py3-none-any.whl", hash = "sha256:ee8708a7f7d37fb273a541fa24ad010ed484192cd0c4ffc0fa0ed5e2d4b9e78b", size = 91378, upload-time = "2025-09-03T19:40:39.679Z" }, ] +[[package]] +name = "filelock" +version = "3.29.4" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/e6/dc/be6cbe99670cd6e4ad387123647cb08e0c32975e223f82551e914c5568a6/filelock-3.29.4.tar.gz", hash = "sha256:10cdb3656fc44541cdf30652a93fb10ec6b05325620eb316bd26893e4201538a", size = 63028, upload-time = "2026-06-13T16:12:00.744Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/13/37/a065dc3bd6e49423a6532c642ca7378d3f467b1ef44c2800c937af7f9739/filelock-3.29.4-py3-none-any.whl", hash = "sha256:dac1648087d5115554850d113e7dd8c83ab2d38e3435dde2d4f163847e57b767", size = 42757, upload-time = "2026-06-13T16:11:59.582Z" }, +] + [[package]] name = "idna" version = "3.11" From 8d6016bb9140f8693d5104992d7384fd775d9376 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Fri, 26 Jun 2026 13:52:45 -0400 Subject: [PATCH 3/7] Fix two Copilot-flagged issues in google_sheet_test_cases.py - Switch absolute src.babel_validation import to a relative import (..core.testrow) so internal modules aren't coupled to the top-level packaging layout. - Use full MD5 digest for cache filename instead of truncating to 8 chars, eliminating the theoretical hash-collision risk across different sheet IDs. Co-Authored-By: Claude Sonnet 4.6 --- .../sources/google_sheets/google_sheet_test_cases.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/babel_validation/sources/google_sheets/google_sheet_test_cases.py b/src/babel_validation/sources/google_sheets/google_sheet_test_cases.py index 935b019..ccd7125 100644 --- a/src/babel_validation/sources/google_sheets/google_sheet_test_cases.py +++ b/src/babel_validation/sources/google_sheets/google_sheet_test_cases.py @@ -15,7 +15,7 @@ from _pytest.mark import ParameterSet from filelock import FileLock -from src.babel_validation.core.testrow import TestRow +from ...core.testrow import TestRow class GoogleSheetTestCases: @@ -39,7 +39,7 @@ def __init__(self, google_sheet_id="11zebx8Qs1Tc3ShQR9nh4HRW8QSoo8k65w_xIaftN0no self.google_sheet_id = google_sheet_id - sheet_hash = hashlib.md5(google_sheet_id.encode()).hexdigest()[:8] + sheet_hash = hashlib.md5(google_sheet_id.encode()).hexdigest() cache_file = Path(tempfile.gettempdir()) / f"babel_validation_gsheet_{sheet_hash}.csv" lock_file = cache_file.with_suffix(".lock") From 1359fcf2a86e2d5217746276b998800c842fb791 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Fri, 26 Jun 2026 13:57:07 -0400 Subject: [PATCH 4/7] Renamed _silent_unlink() to the more sensible unlink_if_exists(). --- tests/conftest.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 96e75f7..8c768ab 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -28,7 +28,13 @@ def get_targets_ini_path(config): return config_path -def _silent_unlink(path: str) -> None: +def unlink_if_exists(path: str) -> None: + """ + Unlink the file at `path` if it exists. + + :param path: The path to the file to unlink. + :return: None + """ try: os.unlink(path) except FileNotFoundError: @@ -41,8 +47,8 @@ def pytest_configure(config): # so they can share the cache file written by the controller. if not os.environ.get('PYTEST_XDIST_WORKER'): for f in glob.glob(os.path.join(tempfile.gettempdir(), 'babel_validation_gsheet_*.csv')): - _silent_unlink(f) - _silent_unlink(f.removesuffix('.csv') + '.lock') + unlink_if_exists(f) + unlink_if_exists(f.removesuffix('.csv') + '.lock') def pytest_addoption(parser): @@ -139,4 +145,4 @@ def category_test(cat): return False return True - return category_test \ No newline at end of file + return category_test From 0b8d157b3bf237efaf6b6da3c5c5909224cb8e59 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Fri, 26 Jun 2026 14:10:31 -0400 Subject: [PATCH 5/7] Make cache_ttl_seconds a constructor parameter in GoogleSheetTestCases Co-Authored-By: Claude Sonnet 4.6 --- .../google_sheets/google_sheet_test_cases.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/babel_validation/sources/google_sheets/google_sheet_test_cases.py b/src/babel_validation/sources/google_sheets/google_sheet_test_cases.py index ccd7125..e764b95 100644 --- a/src/babel_validation/sources/google_sheets/google_sheet_test_cases.py +++ b/src/babel_validation/sources/google_sheets/google_sheet_test_cases.py @@ -26,15 +26,13 @@ class GoogleSheetTestCases: def __str__(self): return f"Google Sheet Test Cases ({len(self.rows)} test cases from {self.google_sheet_id})" - # How long a cached download stays valid. pytest deletes the cache at the - # start of every run (see tests/conftest.py), so this TTL mainly protects - # other consumers (e.g. csv-to-babeltests) from reading stale data forever. - CACHE_TTL_SECONDS = 3600 - - def __init__(self, google_sheet_id="11zebx8Qs1Tc3ShQR9nh4HRW8QSoo8k65w_xIaftN0no"): + def __init__(self, google_sheet_id="11zebx8Qs1Tc3ShQR9nh4HRW8QSoo8k65w_xIaftN0no", cache_ttl_seconds: int = 3600): """ Create a Google Sheet test case. - :param google_sheet_id The Google Sheet identifier to download test cases from. + :param google_sheet_id: The Google Sheet identifier to download test cases from. + :param cache_ttl_seconds: How long a cached download stays valid. pytest deletes the cache at the + start of every run (see tests/conftest.py), so this TTL mainly protects other consumers + (e.g. csv-to-babeltests) from reading stale data forever. """ self.google_sheet_id = google_sheet_id @@ -44,7 +42,7 @@ def __init__(self, google_sheet_id="11zebx8Qs1Tc3ShQR9nh4HRW8QSoo8k65w_xIaftN0no lock_file = cache_file.with_suffix(".lock") with FileLock(lock_file): - if cache_file.exists() and time.time() - cache_file.stat().st_mtime < self.CACHE_TTL_SECONDS: + if cache_file.exists() and time.time() - cache_file.stat().st_mtime < cache_ttl_seconds: self.csv_content = cache_file.read_text(encoding="utf-8") else: csv_url = f"https://docs.google.com/spreadsheets/d/{google_sheet_id}/gviz/tq?tqx=out:csv&sheet=Tests" From 60ed76697313d795178fd8acaa59fd0b78384a66 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Fri, 26 Jun 2026 14:22:53 -0400 Subject: [PATCH 6/7] Document caching model and cache-warming pattern in service modules Adds module docstrings explaining the per-(identifier, params) cache key, the no-auto-eviction policy, and the intended cache-warming pattern: call the batch method once for all identifiers in a task, then use the single-item method per assertion at zero HTTP cost. Adds method docstrings to from_url(), the batch methods (normalize_curies / bulk_lookup), the single-item methods (normalize_curie / lookup), and the cache-clearing methods. Includes a note that lookup() targets a distinct NameRes endpoint from bulk_lookup() and does not delegate to it. Co-Authored-By: Claude Sonnet 4.6 --- src/babel_validation/services/nameres.py | 57 +++++++++++++++++++++++ src/babel_validation/services/nodenorm.py | 55 +++++++++++++++++++++- 2 files changed, 110 insertions(+), 2 deletions(-) diff --git a/src/babel_validation/services/nameres.py b/src/babel_validation/services/nameres.py index 01d36bb..aef6ac2 100644 --- a/src/babel_validation/services/nameres.py +++ b/src/babel_validation/services/nameres.py @@ -1,3 +1,27 @@ +""" +Cached client for the NameRes ``bulk-lookup`` and ``lookup`` APIs. + +Caching model +------------- +Each response is stored under the key ``(query, frozenset(params.items()))``. +Entries are never evicted automatically; call ``delete_query()`` to force +a fresh lookup for a specific query string. + +Cache-warming pattern +--------------------- +When you need to look up many query strings for the same logical task, call +``bulk_lookup()`` once with the full list. That issues a single HTTP POST to +the ``bulk-lookup`` endpoint and populates the cache. Subsequent +``bulk_lookup()`` calls for any subset of those queries are served from cache. + +Endpoint differences +-------------------- +``bulk_lookup()`` targets ``/bulk-lookup`` and sends the query list as a JSON +body. ``lookup()`` targets the separate ``/lookup`` endpoint and sends its +parameters as a URL query string. These are distinct API endpoints with +different response shapes; ``lookup()`` does NOT delegate to ``bulk_lookup()``. +""" + import logging import time @@ -16,11 +40,29 @@ def __str__(self): @staticmethod def from_url(nameres_url: str) -> 'CachedNameRes': + """Return the singleton ``CachedNameRes`` for *nameres_url*. + + The singleton ensures that cache entries accumulated during one part of + a test run are reused by later parts that share the same URL. Prefer + this over direct construction unless you explicitly want a fresh cache. + """ if nameres_url not in cached_nameres_by_url: cached_nameres_by_url[nameres_url] = CachedNameRes(nameres_url) return cached_nameres_by_url[nameres_url] def bulk_lookup(self, queries: list[str], **params) -> dict[str, dict]: + """Look up *queries* in bulk, returning a ``{query: result}`` mapping. + + Already-cached queries are served from the cache; the remainder are + fetched from NameRes in a single HTTP POST to ``bulk-lookup``. + The response is merged with the cached results before returning. + + *queries* must be a non-empty list — the NameRes API rejects empty + requests, so this method raises ``ValueError`` immediately. + + Use this as the cache-warming call; subsequent ``bulk_lookup()`` calls + for any subset of these queries will be free. + """ if not queries: raise ValueError(f"queries must not be empty when calling bulk_lookup({queries}, {params}) on {self}") if not isinstance(queries, list): @@ -55,6 +97,16 @@ def bulk_lookup(self, queries: list[str], **params) -> dict[str, dict]: return result def lookup(self, query, **params): + """Look up a single *query* string via the NameRes ``/lookup`` endpoint. + + This targets a different endpoint from ``bulk_lookup()`` — parameters + are sent as URL query string fields, and the response is a list of + result dicts rather than a mapping. Results are cached per + ``(query, params)`` combination. + + This method does NOT delegate to ``bulk_lookup()``. To cache-warm for + single lookups, call this method (or ``bulk_lookup()``) upfront. + """ cache_key = (query, frozenset(params.items())) if cache_key in self.cache: return self.cache[cache_key] @@ -71,6 +123,11 @@ def lookup(self, query, **params): return result def delete_query(self, query): + """Remove all cached results for *query* (across every param variant). + + The next call to ``lookup()`` or ``bulk_lookup()`` for this query will + issue a fresh HTTP request. + """ keys_to_delete = [k for k in self.cache if k[0] == query] for k in keys_to_delete: del self.cache[k] diff --git a/src/babel_validation/services/nodenorm.py b/src/babel_validation/services/nodenorm.py index 633f7a4..5f858c4 100644 --- a/src/babel_validation/services/nodenorm.py +++ b/src/babel_validation/services/nodenorm.py @@ -1,3 +1,21 @@ +""" +Cached client for the NodeNorm ``get_normalized_nodes`` API. + +Caching model +------------- +Each response is stored under the key ``(curie, frozenset(params.items()))``. +Entries are never evicted automatically; call ``clear_curie()`` to force +a fresh lookup for a specific identifier. + +Cache-warming pattern +--------------------- +When you need to normalize many CURIEs for the same logical task (e.g. all +CURIEs referenced in a GitHub issue), call ``normalize_curies()`` once with +the full list. That issues a single HTTP request and populates the cache. +Subsequent ``normalize_curie()`` calls for any of those identifiers return +immediately from cache — no additional HTTP traffic. +""" + import logging import time @@ -16,11 +34,30 @@ def __str__(self): @staticmethod def from_url(nodenorm_url: str) -> 'CachedNodeNorm': + """Return the singleton ``CachedNodeNorm`` for *nodenorm_url*. + + The singleton ensures that cache entries accumulated during one part of + a test run are reused by later parts that share the same URL. Prefer + this over direct construction unless you explicitly want a fresh cache. + """ if nodenorm_url not in cached_node_norms_by_url: cached_node_norms_by_url[nodenorm_url] = CachedNodeNorm(nodenorm_url) return cached_node_norms_by_url[nodenorm_url] def normalize_curies(self, curies: list[str], **params) -> dict[str, dict]: + """Normalize *curies* in bulk, returning a ``{curie: result}`` mapping. + + Already-cached CURIEs are served from the cache; the remainder are + fetched from NodeNorm in a single HTTP POST to ``get_normalized_nodes``. + The response is merged with the cached results before returning. + + *curies* must be a non-empty list — the NodeNorm API rejects empty + requests, so this method raises ``ValueError`` immediately. + + Values in the returned dict are ``None`` for CURIEs NodeNorm could not + resolve. Use this as the cache-warming call; subsequent + ``normalize_curie()`` calls for these identifiers will be free. + """ if not curies: raise ValueError(f"curies must not be empty when calling normalize_curies({curies}, {params}) on {self}") if not isinstance(curies, list): @@ -56,14 +93,28 @@ def normalize_curies(self, curies: list[str], **params) -> dict[str, dict]: return result def normalize_curie(self, curie, **params): + """Normalize a single *curie*, returning the NodeNorm result or ``None``. + + Checks the cache first; on a miss, delegates to ``normalize_curies()`` + (one HTTP call) and returns the result. If you expect to normalize many + CURIEs, call ``normalize_curies()`` upfront so this method never makes + an HTTP call. + + Uses ``.get()`` rather than direct indexing so that a NodeNorm response + that silently omits a requested CURIE returns ``None`` instead of + raising ``KeyError``. + """ cache_key = (curie, frozenset(params.items())) if cache_key in self.cache: return self.cache[cache_key] - # Use .get(): NodeNorm normally echoes every requested CURIE (null when - # unresolvable), but don't crash if it ever omits one. return self.normalize_curies([curie], **params).get(curie) def clear_curie(self, curie): + """Remove all cached results for *curie* (across every param variant). + + The next call to ``normalize_curie()`` or ``normalize_curies()`` for + this identifier will issue a fresh HTTP request. + """ keys_to_delete = [k for k in self.cache if k[0] == curie] for k in keys_to_delete: del self.cache[k] From cde0de15a67bc5e20d76b31357b067f30674a4b3 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Fri, 26 Jun 2026 14:23:50 -0400 Subject: [PATCH 7/7] Add Protocol interfaces and rename cache-invalidation methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NodeNormService and NameResService Protocols document the public interface callers should type against. When the implementation is later replaced by an external library, any code typed against the Protocol will need no changes. Rename clear_curie() → invalidate_curie() and delete_query() → invalidate_query(). "Invalidate" is standard cache vocabulary and makes the full-eviction-across-all-param-variants semantics clearer than "clear/delete". Also tightens type annotations on normalize_curie / lookup signatures (str, return type) to match the Protocol. Co-Authored-By: Claude Sonnet 4.6 --- src/babel_validation/services/nameres.py | 21 ++++++++++++++++++--- src/babel_validation/services/nodenorm.py | 23 +++++++++++++++++++---- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/babel_validation/services/nameres.py b/src/babel_validation/services/nameres.py index aef6ac2..6aadd85 100644 --- a/src/babel_validation/services/nameres.py +++ b/src/babel_validation/services/nameres.py @@ -4,7 +4,7 @@ Caching model ------------- Each response is stored under the key ``(query, frozenset(params.items()))``. -Entries are never evicted automatically; call ``delete_query()`` to force +Entries are never evicted automatically; call ``invalidate_query()`` to force a fresh lookup for a specific query string. Cache-warming pattern @@ -24,11 +24,26 @@ import logging import time +from typing import Protocol import requests cached_nameres_by_url = {} + +class NameResService(Protocol): + """Interface that callers should depend on. + + Type parameters against this Protocol rather than ``CachedNameRes`` + directly so that a future drop-in library replacement requires no caller + changes. + """ + + def bulk_lookup(self, queries: list[str], **params) -> dict[str, dict]: ... + def lookup(self, query: str, **params) -> list[dict]: ... + def invalidate_query(self, query: str) -> None: ... + + class CachedNameRes: def __init__(self, nameres_url: str): self.nameres_url = nameres_url @@ -96,7 +111,7 @@ def bulk_lookup(self, queries: list[str], **params) -> dict[str, dict]: return result - def lookup(self, query, **params): + def lookup(self, query: str, **params) -> list[dict]: """Look up a single *query* string via the NameRes ``/lookup`` endpoint. This targets a different endpoint from ``bulk_lookup()`` — parameters @@ -122,7 +137,7 @@ def lookup(self, query, **params): self.cache[cache_key] = result return result - def delete_query(self, query): + def invalidate_query(self, query: str) -> None: """Remove all cached results for *query* (across every param variant). The next call to ``lookup()`` or ``bulk_lookup()`` for this query will diff --git a/src/babel_validation/services/nodenorm.py b/src/babel_validation/services/nodenorm.py index 5f858c4..9ba3242 100644 --- a/src/babel_validation/services/nodenorm.py +++ b/src/babel_validation/services/nodenorm.py @@ -4,7 +4,7 @@ Caching model ------------- Each response is stored under the key ``(curie, frozenset(params.items()))``. -Entries are never evicted automatically; call ``clear_curie()`` to force +Entries are never evicted automatically; call ``invalidate_curie()`` to force a fresh lookup for a specific identifier. Cache-warming pattern @@ -18,11 +18,26 @@ import logging import time +from typing import Protocol import requests cached_node_norms_by_url = {} + +class NodeNormService(Protocol): + """Interface that callers should depend on. + + Type parameters against this Protocol rather than ``CachedNodeNorm`` + directly so that a future drop-in library replacement requires no caller + changes. + """ + + def normalize_curies(self, curies: list[str], **params) -> dict[str, dict | None]: ... + def normalize_curie(self, curie: str, **params) -> dict | None: ... + def invalidate_curie(self, curie: str) -> None: ... + + class CachedNodeNorm: def __init__(self, nodenorm_url: str): self.nodenorm_url = nodenorm_url @@ -44,7 +59,7 @@ def from_url(nodenorm_url: str) -> 'CachedNodeNorm': cached_node_norms_by_url[nodenorm_url] = CachedNodeNorm(nodenorm_url) return cached_node_norms_by_url[nodenorm_url] - def normalize_curies(self, curies: list[str], **params) -> dict[str, dict]: + def normalize_curies(self, curies: list[str], **params) -> dict[str, dict | None]: """Normalize *curies* in bulk, returning a ``{curie: result}`` mapping. Already-cached CURIEs are served from the cache; the remainder are @@ -92,7 +107,7 @@ def normalize_curies(self, curies: list[str], **params) -> dict[str, dict]: return result - def normalize_curie(self, curie, **params): + def normalize_curie(self, curie: str, **params) -> dict | None: """Normalize a single *curie*, returning the NodeNorm result or ``None``. Checks the cache first; on a miss, delegates to ``normalize_curies()`` @@ -109,7 +124,7 @@ def normalize_curie(self, curie, **params): return self.cache[cache_key] return self.normalize_curies([curie], **params).get(curie) - def clear_curie(self, curie): + def invalidate_curie(self, curie: str) -> None: """Remove all cached results for *curie* (across every param variant). The next call to ``normalize_curie()`` or ``normalize_curies()`` for