diff --git a/src/spyglass/utils/mixins/base.py b/src/spyglass/utils/mixins/base.py index b9c7ed2d2..e10791dfb 100644 --- a/src/spyglass/utils/mixins/base.py +++ b/src/spyglass/utils/mixins/base.py @@ -36,24 +36,19 @@ def _graph_deps(self) -> list: return [TableChain, RestrGraph] - @property + @cached_property def _test_mode(self) -> bool: """Return True if in test mode. Avoids circular import. Prevents prompt on delete. - Note: Using @property instead of @cached_property so we always get - current value from dj.config, even if test_mode changes after first access. - Used by ... - BaseMixin._spyglass_version - HelpersMixin """ - import datajoint as dj + from spyglass.settings import test_mode - # Check dj.config directly instead of importing module-level variable - # which gets stale if load_config() is called after initial import - return dj.config.get("custom", {}).get("test_mode", False) + return test_mode @cached_property def _spyglass_version(self): diff --git a/tests/utils/test_dj_helper_fn.py b/tests/utils/test_dj_helper_fn.py index aa063648f..c688e04fd 100644 --- a/tests/utils/test_dj_helper_fn.py +++ b/tests/utils/test_dj_helper_fn.py @@ -9,3 +9,42 @@ def test_deprecation_factory(caplog, common): assert ( "Deprecation:" in caplog.text ), "No deprecation warning logged on migrated table." + + +def test_str_to_bool(): + """Test str_to_bool function handles various string inputs correctly. + + This test verifies that str_to_bool properly converts string values + to booleans, which is essential for fixing issue #1528 where string + "false" was incorrectly evaluated as True. + """ + from spyglass.utils.dj_helper_fn import str_to_bool + + # Test truthy strings (case insensitive) + assert str_to_bool("true") is True + assert str_to_bool("True") is True + assert str_to_bool("TRUE") is True + assert str_to_bool("t") is True + assert str_to_bool("T") is True + assert str_to_bool("yes") is True + assert str_to_bool("YES") is True + assert str_to_bool("y") is True + assert str_to_bool("Y") is True + assert str_to_bool("1") is True + + # Test falsy strings + assert str_to_bool("false") is False + assert str_to_bool("False") is False + assert str_to_bool("FALSE") is False + assert str_to_bool("no") is False + assert str_to_bool("n") is False + assert str_to_bool("0") is False + assert str_to_bool("") is False + assert str_to_bool("random_string") is False + + # Test non-string inputs + assert str_to_bool(True) is True + assert str_to_bool(False) is False + assert str_to_bool(None) is False + assert str_to_bool(0) is False + assert str_to_bool(1) is True diff --git a/tests/utils/test_mixin.py b/tests/utils/test_mixin.py index 9197c67b7..afde038c7 100644 --- a/tests/utils/test_mixin.py +++ b/tests/utils/test_mixin.py @@ -173,3 +173,25 @@ def test_mixin_del_orphans(dj_conn, Mixin, MixinChild): Mixin().delete_orphans(dry_run=False, safemode=False) post_del = Mixin().fetch1("id") assert post_del == 0, "Delete orphans not working." + + +def test_test_mode_property_uses_settings(schema_test, Mixin): + """Test that _test_mode property uses spyglass.settings.config. + + Verifies fix for issue #1528 where string "false" was incorrectly + evaluated as True. The property should now use spyglass.settings.config.test_mode + which properly converts strings to booleans via str_to_bool(). + """ + schema_test(Mixin) + + # The _test_mode property should return a boolean + test_mode_value = Mixin()._test_mode + assert isinstance(test_mode_value, bool), ( + "_test_mode should return a boolean value" + ) + + # In test environment, test_mode should be True + # (set by load_config fixture in conftest.py) + assert test_mode_value is True, ( + "_test_mode should be True in test environment" + )