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/*. 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 71f6e3d7c8..0b2de4b9f8 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -31,8 +31,39 @@ from simtools.runners.corsika_runner import CorsikaRunner logger = logging.getLogger() +REAL_DATABASE_HANDLER_CLASS = db_handler.DatabaseHandler -UNIT_TEST_DB = "unit_tests/db" + +def pytest_collection_modifyitems(items): + """Tag db unit tests with a stable marker for fixture routing.""" + + def _is_db_item(item): + path = getattr(item, "path", None) + if path is None: + return False + 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))) + + for item in items: + if _is_db_item(item): + item.add_marker("db_unit_test") + + +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 node.get_closest_marker("db_unit_test") is not None: + return True + + 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 @@ -355,11 +386,8 @@ 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: - 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 + if _is_db_unit_test(request): + return request.getfixturevalue("db") # Load mock data from JSON files mock_parameters = _apply_mock_param_defaults(_load_mock_db_json("mock_parameters.json")) @@ -442,11 +470,13 @@ 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: - yield + if _is_db_unit_test(request): + with mock.patch( + "simtools.db.db_handler.DatabaseHandler", + new=REAL_DATABASE_HANDLER_CLASS, + ): + yield return mock_db_handler = request.getfixturevalue("mock_db_handler") @@ -462,8 +492,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 @@ -503,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)