From 86d99643dad8887ad010485961e78224681c2e49 Mon Sep 17 00:00:00 2001 From: tobiaskleiner Date: Thu, 26 Mar 2026 15:55:15 +0100 Subject: [PATCH 1/5] add --- tests/unit_tests/conftest.py | 44 +++++++++++++++++++++++++++++------- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index 71f6e3d7c8..42f64f19fe 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -32,7 +32,39 @@ logger = logging.getLogger() -UNIT_TEST_DB = "unit_tests/db" +UNIT_TEST_DB = "unit_tests/db/" + + +def _is_db_unit_test(request): + """Return True for tests under tests/unit_tests/db across path styles.""" + + def _normalize(value): + return str(value).replace("\\", "/") if value else "" + + node = getattr(request, "node", None) + if node is None: + return False + + candidates = [ + _normalize(getattr(node, "path", "")), + _normalize(getattr(node, "fspath", "")), + _normalize(getattr(getattr(node, "module", None), "__file__", "")), + _normalize(getattr(node, "nodeid", "")), + _normalize(getattr(getattr(node, "module", None), "__name__", "")), + ] + + for candidate in candidates: + if not candidate: + continue + normalized = candidate.strip("/") + if UNIT_TEST_DB in normalized: + return True + if normalized.startswith("tests/unit_tests/db/"): + return True + if "tests.unit_tests.db" in normalized: + return True + + return False @functools.lru_cache @@ -355,8 +387,7 @@ def mock_db_handler(request): Returns a MagicMock configured with typical DatabaseHandler methods. Tests in tests/unit_tests/db/ receive a real DatabaseHandler instance. """ - test_file_path = str(request.node.fspath) - if UNIT_TEST_DB in test_file_path: + if _is_db_unit_test(request): db_instance = request.getfixturevalue("db") db_instance.get_model_versions = MagicMock(return_value=["1.0.0", "5.0.0", "6.0.0"]) return db_instance @@ -442,10 +473,8 @@ def patch_database_handler(request, mocker): Tests in tests/unit_tests/db/ are excluded from this mocking. Also patches model parameter schema validation to avoid schema version mismatches. """ - test_file_path = str(request.node.fspath) - # Skip mocking for tests in db/ directory - if UNIT_TEST_DB in test_file_path: + if _is_db_unit_test(request): yield return @@ -462,8 +491,7 @@ def patch_database_handler(request, mocker): def db(request): """Database object with configuration from settings.config.db_handler.""" - test_file_path = str(request.node.fspath) - if UNIT_TEST_DB not in test_file_path: + if not _is_db_unit_test(request): db_instance = db_handler.DatabaseHandler() yield db_instance return From e32e595e190e31e8676b5ec0d4c932fc96147d7f Mon Sep 17 00:00:00 2001 From: tobiaskleiner Date: Thu, 26 Mar 2026 16:01:49 +0100 Subject: [PATCH 2/5] add --- docs/changes/2089.bugfix.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/changes/2089.bugfix.md diff --git a/docs/changes/2089.bugfix.md b/docs/changes/2089.bugfix.md new file mode 100644 index 0000000000..c2b280dca7 --- /dev/null +++ b/docs/changes/2089.bugfix.md @@ -0,0 +1 @@ +Fixed CI-only DB unit-test flakiness by hardening conftest.py database-test detection so DatabaseHandler is no longer accidentally globally mocked for tests/unit_tests/db/*. From 6d38e72f82ff7c7e8ae2ce457810580b696e33a0 Mon Sep 17 00:00:00 2001 From: tobiaskleiner Date: Thu, 26 Mar 2026 16:26:55 +0100 Subject: [PATCH 3/5] more robust with collection time tagging --- tests/unit_tests/conftest.py | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index 42f64f19fe..3fe830e280 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -35,22 +35,36 @@ UNIT_TEST_DB = "unit_tests/db/" +def pytest_collection_modifyitems(items): + """Tag db unit tests with a stable keyword for fixture routing.""" + for item in items: + nodeid = str(getattr(item, "nodeid", "")).replace("\\", "/") + if "/unit_tests/db/" in f"/{nodeid}": + item.keywords["db_unit_test"] = True + + def _is_db_unit_test(request): """Return True for tests under tests/unit_tests/db across path styles.""" - def _normalize(value): - return str(value).replace("\\", "/") if value else "" - node = getattr(request, "node", None) if node is None: return False + if "db_unit_test" in getattr(node, "keywords", {}): + return True + + def _normalize(value): + return str(value).replace("\\", "/") if value else "" + candidates = [ _normalize(getattr(node, "path", "")), _normalize(getattr(node, "fspath", "")), + _normalize(getattr(node, "location", [""])[0] if getattr(node, "location", None) else ""), _normalize(getattr(getattr(node, "module", None), "__file__", "")), _normalize(getattr(node, "nodeid", "")), _normalize(getattr(getattr(node, "module", None), "__name__", "")), + _normalize(getattr(request, "fspath", "")), + _normalize(getattr(request, "nodeid", "")), ] for candidate in candidates: From ba454cbb314e974b3c27204e0cacf5dddbbfe278 Mon Sep 17 00:00:00 2001 From: tobiaskleiner Date: Thu, 26 Mar 2026 16:59:57 +0100 Subject: [PATCH 4/5] modify and remove duplicate fixture --- tests/unit_tests/conftest.py | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index 3fe830e280..45cb699332 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -47,6 +47,27 @@ def _is_db_unit_test(request): """Return True for tests under tests/unit_tests/db across path styles.""" node = getattr(request, "node", None) + nodeid = str(getattr(node, "nodeid", "")).replace("\\", "/") + if nodeid.startswith("tests/unit_tests/db/"): + return True + + def _contains_db_test_path(value): + normalized = str(value).replace("\\", "/").strip("/") if value else "" + if not normalized: + return False + return ( + UNIT_TEST_DB in normalized + or normalized.startswith("tests/unit_tests/db/") + or "tests.unit_tests.db" in normalized + ) + + # Most robust execution-time signal during fixture setup. + if _contains_db_test_path(os.environ.get("PYTEST_CURRENT_TEST", "")): + return True + + if _contains_db_test_path(getattr(getattr(request, "module", None), "__file__", "")): + return True + if node is None: return False @@ -68,14 +89,7 @@ def _normalize(value): ] for candidate in candidates: - if not candidate: - continue - normalized = candidate.strip("/") - if UNIT_TEST_DB in normalized: - return True - if normalized.startswith("tests/unit_tests/db/"): - return True - if "tests.unit_tests.db" in normalized: + if _contains_db_test_path(candidate): return True return False From 36f0699b1313876f42679b555e9e8676291edcb4 Mon Sep 17 00:00:00 2001 From: tobiaskleiner Date: Fri, 27 Mar 2026 14:33:17 +0100 Subject: [PATCH 5/5] update --- pyproject.toml | 1 + tests/unit_tests/conftest.py | 81 +++++++-------------- tests/unit_tests/db/test_db_handler.py | 7 +- tests/unit_tests/db/test_db_model_upload.py | 2 + tests/unit_tests/db/test_mongo_db.py | 5 +- 5 files changed, 39 insertions(+), 57 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index b7852c3ade..3a96c367ed 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -280,6 +280,7 @@ ignore-words-list = "chec,arrang,livetime" [tool.pytest] ini_options.markers = [ "uses_model_database: test uses model parameter database.", + "db_unit_test: test belongs to unit_tests/db and must not use global DatabaseHandler patching.", ] ini_options.minversion = "6.0" ini_options.norecursedirs = [ diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index 45cb699332..0b2de4b9f8 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -31,68 +31,39 @@ from simtools.runners.corsika_runner import CorsikaRunner logger = logging.getLogger() - -UNIT_TEST_DB = "unit_tests/db/" +REAL_DATABASE_HANDLER_CLASS = db_handler.DatabaseHandler def pytest_collection_modifyitems(items): - """Tag db unit tests with a stable keyword for fixture routing.""" - for item in items: - nodeid = str(getattr(item, "nodeid", "")).replace("\\", "/") - if "/unit_tests/db/" in f"/{nodeid}": - item.keywords["db_unit_test"] = True - - -def _is_db_unit_test(request): - """Return True for tests under tests/unit_tests/db across path styles.""" + """Tag db unit tests with a stable marker for fixture routing.""" - node = getattr(request, "node", None) - nodeid = str(getattr(node, "nodeid", "")).replace("\\", "/") - if nodeid.startswith("tests/unit_tests/db/"): - return True - - def _contains_db_test_path(value): - normalized = str(value).replace("\\", "/").strip("/") if value else "" - if not normalized: + def _is_db_item(item): + path = getattr(item, "path", None) + if path is None: return False - return ( - UNIT_TEST_DB in normalized - or normalized.startswith("tests/unit_tests/db/") - or "tests.unit_tests.db" in normalized - ) + parts = tuple(path.parts) + db_parts = ("tests", "unit_tests", "db") + return any(parts[idx : idx + 3] == db_parts for idx in range(max(len(parts) - 2, 0))) - # Most robust execution-time signal during fixture setup. - if _contains_db_test_path(os.environ.get("PYTEST_CURRENT_TEST", "")): - return True + for item in items: + if _is_db_item(item): + item.add_marker("db_unit_test") - if _contains_db_test_path(getattr(getattr(request, "module", None), "__file__", "")): - return True +def _is_db_unit_test(request): + """Return True when the current test carries the db_unit_test marker.""" + node = getattr(request, "node", None) if node is None: return False - if "db_unit_test" in getattr(node, "keywords", {}): + if node.get_closest_marker("db_unit_test") is not None: return True - def _normalize(value): - return str(value).replace("\\", "/") if value else "" - - candidates = [ - _normalize(getattr(node, "path", "")), - _normalize(getattr(node, "fspath", "")), - _normalize(getattr(node, "location", [""])[0] if getattr(node, "location", None) else ""), - _normalize(getattr(getattr(node, "module", None), "__file__", "")), - _normalize(getattr(node, "nodeid", "")), - _normalize(getattr(getattr(node, "module", None), "__name__", "")), - _normalize(getattr(request, "fspath", "")), - _normalize(getattr(request, "nodeid", "")), - ] - - for candidate in candidates: - if _contains_db_test_path(candidate): - return True - - return False + test_file = "" + location = getattr(node, "location", None) + if location: + test_file = str(location[0]).replace("\\", "/") + return test_file.startswith("tests/unit_tests/db/") @functools.lru_cache @@ -416,9 +387,7 @@ def mock_db_handler(request): Tests in tests/unit_tests/db/ receive a real DatabaseHandler instance. """ if _is_db_unit_test(request): - db_instance = request.getfixturevalue("db") - db_instance.get_model_versions = MagicMock(return_value=["1.0.0", "5.0.0", "6.0.0"]) - return db_instance + return request.getfixturevalue("db") # Load mock data from JSON files mock_parameters = _apply_mock_param_defaults(_load_mock_db_json("mock_parameters.json")) @@ -503,7 +472,11 @@ def patch_database_handler(request, mocker): """ # Skip mocking for tests in db/ directory if _is_db_unit_test(request): - yield + with mock.patch( + "simtools.db.db_handler.DatabaseHandler", + new=REAL_DATABASE_HANDLER_CLASS, + ): + yield return mock_db_handler = request.getfixturevalue("mock_db_handler") @@ -559,7 +532,7 @@ def db(request): mock_mongo_client.close = MagicMock() with patch("simtools.db.mongo_db.MongoClient", return_value=mock_mongo_client): - db_instance = db_handler.DatabaseHandler() + db_instance = REAL_DATABASE_HANDLER_CLASS() MongoDBHandler.db_client = MongoDBHandler.db_client or mock_mongo_client yield db_instance # Explicitly close the mock client to avoid un-raisable exception warnings diff --git a/tests/unit_tests/db/test_db_handler.py b/tests/unit_tests/db/test_db_handler.py index 0b3a518be2..c83ca4f24d 100644 --- a/tests/unit_tests/db/test_db_handler.py +++ b/tests/unit_tests/db/test_db_handler.py @@ -10,8 +10,11 @@ from simtools.db.mongo_db import MongoDBHandler from simtools.utils import names -# Suppress warnings -pytestmark = pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning") +# Suppress warnings and mark this module as db unit tests. +pytestmark = [ + pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning"), + pytest.mark.db_unit_test, +] logger = logging.getLogger() diff --git a/tests/unit_tests/db/test_db_model_upload.py b/tests/unit_tests/db/test_db_model_upload.py index 4c7c382fac..e95ce1f679 100644 --- a/tests/unit_tests/db/test_db_model_upload.py +++ b/tests/unit_tests/db/test_db_model_upload.py @@ -8,6 +8,8 @@ import simtools.db.db_model_upload as db_model_upload +pytestmark = pytest.mark.db_unit_test + @patch("simtools.db.db_model_upload.ascii_handler.collect_data_from_file") def test_add_values_from_json_to_db(mock_collect_data_from_file): diff --git a/tests/unit_tests/db/test_mongo_db.py b/tests/unit_tests/db/test_mongo_db.py index 6eb5e13e41..eefd8a1304 100644 --- a/tests/unit_tests/db/test_mongo_db.py +++ b/tests/unit_tests/db/test_mongo_db.py @@ -7,7 +7,10 @@ from simtools.db import mongo_db -pytestmark = pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning") +pytestmark = [ + pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning"), + pytest.mark.db_unit_test, +] @pytest.fixture(autouse=True)