From 6a4be7baa7443fa600b73cf28ff206efb6dbb3b7 Mon Sep 17 00:00:00 2001 From: diehlbw Date: Thu, 27 Jun 2024 18:27:35 +0000 Subject: [PATCH] fix test state leakage --- src/seismometer/core/io.py | 6 ++++-- src/seismometer/data/loader/prediction.py | 1 + tests/configuration/test_model.py | 8 +++---- tests/core/test_decorators.py | 23 +++++++++++++-------- tests/core/test_io.py | 8 +++---- tests/data/loaders/test_load_events.py | 6 +++--- tests/data/loaders/test_load_pipeline.py | 2 +- tests/data/loaders/test_load_predictions.py | 2 +- tests/html/test_templates.py | 2 +- tests/test_startup.py | 4 ++-- 10 files changed, 35 insertions(+), 27 deletions(-) diff --git a/src/seismometer/core/io.py b/src/seismometer/core/io.py index 1900a37..f4e851c 100644 --- a/src/seismometer/core/io.py +++ b/src/seismometer/core/io.py @@ -12,6 +12,8 @@ Pathlike = str | Path +logger = logging.getLogger("seismometer") + def slugify(value: str) -> str: """ @@ -108,7 +110,7 @@ def resolve_filename( # Create pre-emptively if not basedir.is_dir(): if not create: - logging.warning(f"No directory found for group: {basedir}") + logger.warning(f"No directory found for group: {basedir}") else: basedir.mkdir(parents=True, exist_ok=True) return basedir / filename @@ -317,4 +319,4 @@ def _write(writer: Callable[[Any, "fileobject"], None], content: Any, file: Path with open(file, "w") as fo: writer(content, fo) - logging.info(f"File written: {file.resolve()}") + logger.info(f"File written: {file.resolve()}") diff --git a/src/seismometer/data/loader/prediction.py b/src/seismometer/data/loader/prediction.py index 74901a6..5617701 100644 --- a/src/seismometer/data/loader/prediction.py +++ b/src/seismometer/data/loader/prediction.py @@ -53,6 +53,7 @@ def _log_column_mismatch(actual_columns: list[str], desired_columns: list[str], """Logs warnings if the actual columns and desired columns are a mismatch.""" if len(actual_columns) == len(desired_columns): return + logger.warning( "Not all requested columns are present. " + f"Missing columns are {', '.join(desired_columns-present_columns)}" ) diff --git a/tests/configuration/test_model.py b/tests/configuration/test_model.py index ff2c542..26ef69f 100644 --- a/tests/configuration/test_model.py +++ b/tests/configuration/test_model.py @@ -78,7 +78,7 @@ def test_reduce_events_to_unique_names(self, caplog): undertest.Event(source="event2", display_name="Event 2"), undertest.Event(source="different source", display_name="Event 1"), ] - with caplog.at_level(logging.WARNING): + with caplog.at_level(logging.WARNING, logger="seismometer"): data_usage = undertest.DataUsage(events=events) assert "Duplicate" in caplog.text @@ -93,7 +93,7 @@ def test_reduce_events_eliminates_source_display_collision(self, caplog): undertest.Event(source="event1"), undertest.Event(source="event2", display_name="event1"), ] - with caplog.at_level(logging.WARNING): + with caplog.at_level(logging.WARNING, logger="seismometer"): data_usage = undertest.DataUsage(events=events) assert "Duplicate" in caplog.text @@ -107,7 +107,7 @@ def test_reduce_cohorts_to_unique_names(self, caplog): undertest.Cohort(source="cohort2", display_name="Cohort 2"), undertest.Cohort(source="different source", display_name="Cohort 1"), ] - with caplog.at_level(logging.WARNING): + with caplog.at_level(logging.WARNING, logger="seismometer"): data_usage = undertest.DataUsage(cohorts=cohorts) assert "Duplicate" in caplog.text @@ -123,7 +123,7 @@ def test_reduce_cohorts_eliminates_source_display_collision(self, caplog): undertest.Cohort(source="cohort2", display_name="cohort1"), ] - with caplog.at_level(logging.WARNING): + with caplog.at_level(logging.WARNING, logger="seismometer"): data_usage = undertest.DataUsage(cohorts=cohorts) assert "Duplicate" in caplog.text diff --git a/tests/core/test_decorators.py b/tests/core/test_decorators.py index 5d10864..fbc05cf 100644 --- a/tests/core/test_decorators.py +++ b/tests/core/test_decorators.py @@ -8,11 +8,14 @@ from seismometer.core.decorators import DiskCachedFunction, export -def foo(arg1, kwarg1=None): - if kwarg1 is None: - return arg1 + 1 - else: - return kwarg1 + 1 +def get_test_function(): + def foo(arg1, kwarg1=None): + if kwarg1 is None: + return arg1 + 1 + else: + return kwarg1 + 1 + + return foo class Test_Export: @@ -26,14 +29,15 @@ def test_export(self): global __all__ __all__ = [] - new_fn = export(foo) + test_fn = get_test_function() + new_fn = export(test_fn) assert new_fn(1) == 2 assert new_fn(1, 5) == 6 assert __all__ == ["foo"] with pytest.raises(ImportError): - export(foo) + export(test_fn) def test_mod_none(self): """ @@ -45,8 +49,9 @@ def test_mod_none(self): global __all__ __all__ = [] - foo.__module__ = None - new_fn = export(foo) + test_fn = get_test_function() + test_fn.__module__ = None + new_fn = export(test_fn) assert new_fn(1) == 2 assert new_fn(1, 5) == 6 diff --git a/tests/core/test_io.py b/tests/core/test_io.py index 55dd551..36d8697 100644 --- a/tests/core/test_io.py +++ b/tests/core/test_io.py @@ -188,7 +188,7 @@ def test_write_new_file_in_nonexistent_directory(tmp_as_current): def test_write_logs_file_written(tmp_as_current, caplog): file = Path("new_file.txt") - with caplog.at_level(logging.INFO): + with caplog.at_level(logging.INFO, logger="seismometer"): undertest._write(lambda content, fo: fo.write(content), "test content", file, overwrite=False) assert f"File written: {file.resolve()}" in caplog.text @@ -227,7 +227,7 @@ def test_create_makesdir_on_none(self, tmp_path, caplog): filename = "new_file" expected = Path("output/attr") / "age1_0-3_0-self+strip" / filename - with caplog.at_level(30): + with caplog.at_level(30, logger="seismometer"): _ = undertest.resolve_filename("new_file", attribute, subgroups) assert not caplog.text @@ -239,7 +239,7 @@ def test_no_create_warns_on_nonexistent(self, caplog): filename = "new_file" expected = Path("output/attr") / "age1_0-3_0-self+strip" / filename - with caplog.at_level(30): + with caplog.at_level(30, logger="seismometer"): _ = undertest.resolve_filename("new_file", attribute, subgroups, create=False) assert "No directory" in caplog.text @@ -252,7 +252,7 @@ def test_no_create_existent_does_not_warn(self, caplog): expected = Path("output/attr") / "gg" / filename expected.parent.mkdir(parents=True, exist_ok=False) - with caplog.at_level(30): + with caplog.at_level(30, logger="seismometer"): _ = undertest.resolve_filename("new_file", attribute, subgroups, create=False) assert not caplog.text diff --git a/tests/data/loaders/test_load_events.py b/tests/data/loaders/test_load_events.py index a480399..0c607da 100644 --- a/tests/data/loaders/test_load_events.py +++ b/tests/data/loaders/test_load_events.py @@ -68,7 +68,7 @@ def test_nofile_warns_and_returns_empty(self, setup_fn, load_fn, caplog): config = setup_fn() config.event_path = "not_a_file.parquet" - with caplog.at_level(logging.DEBUG): + with caplog.at_level(logging.DEBUG, logger="seismometer"): actual = load_fn(config) print(caplog.text) assert "No events found" in caplog.text @@ -144,7 +144,7 @@ def test_merge_info_logged(self, event, str_inclusions, caplog): } ) - with caplog.at_level(logging.DEBUG): + with caplog.at_level(logging.DEBUG, logger="seismometer"): _ = undertest.merge_onto_predictions(config, event_df, predictions) for pattern in str_inclusions: @@ -166,7 +166,7 @@ def test_imputation_on_target(self, caplog): } ) - with caplog.at_level(logging.WARNING): + with caplog.at_level(logging.WARNING, logger="seismometer"): _ = undertest.merge_onto_predictions(config, event_df, predictions) assert "Event a specified impute" in caplog.text diff --git a/tests/data/loaders/test_load_pipeline.py b/tests/data/loaders/test_load_pipeline.py index 1c81eb9..7939d52 100644 --- a/tests/data/loaders/test_load_pipeline.py +++ b/tests/data/loaders/test_load_pipeline.py @@ -71,7 +71,7 @@ def test_loading_can_log_location(self, fake_config, caplog): input = "INPUT" loader = undertest.SeismogramLoader(fake_config) - with caplog.at_level("INFO"): + with caplog.at_level("INFO", logger="seismometer"): _ = loader.load_data(input) assert "Configuration speficies" in caplog.text diff --git a/tests/data/loaders/test_load_predictions.py b/tests/data/loaders/test_load_predictions.py index 445e189..7c886e0 100644 --- a/tests/data/loaders/test_load_predictions.py +++ b/tests/data/loaders/test_load_predictions.py @@ -140,7 +140,7 @@ def test_column_mismatch_logs_warning( config = setup_fn() config.features = desired_columns - with caplog.at_level(log_level): + with caplog.at_level(log_level, logger="seismometer"): _ = load_fn(config) assert ("Not all requested columns are present" in caplog.text) == warning_present diff --git a/tests/html/test_templates.py b/tests/html/test_templates.py index 165da8e..60a7956 100644 --- a/tests/html/test_templates.py +++ b/tests/html/test_templates.py @@ -29,7 +29,7 @@ def cohort_summaries_template(res): class Test_Templates: def test_nonexistent_template(self, caplog): - with caplog.at_level(logging.WARNING): + with caplog.at_level(logging.WARNING, logger="seismometer"): undertest.render_into_template("unknown") assert "HTML template unknown not found" in caplog.text diff --git a/tests/test_startup.py b/tests/test_startup.py index 6777c08..4a57d20 100644 --- a/tests/test_startup.py +++ b/tests/test_startup.py @@ -10,7 +10,7 @@ from seismometer.seismogram import Seismogram -def fake_load_config(self, *args): +def fake_load_config(self, *args, definitions=None): mock_config = Mock(autospec=ConfigProvider) mock_config.output_dir.return_value self.config = mock_config @@ -19,7 +19,7 @@ def fake_load_config(self, *args): # TODO: update this to create testing Loader and have factory return it -def fake_load_data(self, *args): +def fake_load_data(self, predictions=None, events=None, reset=False): self.dataframe = pd.DataFrame()