From c202f15d8cdf321da5355cc4c1e75407b0b70084 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 29 Jun 2026 15:40:22 +0000 Subject: [PATCH] fix broken unit tests --- .../modelling_e2e/test_handler.py | 27 +++++++++---------- tests/orchestration/fakes.py | 14 +++++++++- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/tests/applications/modelling_e2e/test_handler.py b/tests/applications/modelling_e2e/test_handler.py index 880ac9f4..a09fe266 100644 --- a/tests/applications/modelling_e2e/test_handler.py +++ b/tests/applications/modelling_e2e/test_handler.py @@ -359,7 +359,7 @@ def test_lodged_epc_path_saves_epc_plan_and_marks_modelled( mock_uow.epc.save_batch.assert_called_once_with( [EpcSaveRequest(mock_epc, property_id=PROPERTY_ID, portfolio_id=PORTFOLIO_ID, source="lodged")] ) - mock_uow.plan.save.assert_called_once() + mock_uow.plan.save_batch.assert_called_once() mock_uow.property.mark_modelled.assert_called_once_with( PROPERTY_ID, has_recommendations=False ) @@ -448,7 +448,7 @@ def test_skipped_cohort_certs_do_not_prevent_plan_being_saved() -> None: _call_handler(_BODY) # Assert — plan committed despite the skipped cert - mock_uow.plan.save.assert_called_once() + mock_uow.plan.save_batch.assert_called_once() mock_uow.commit.assert_called_once() @@ -513,7 +513,7 @@ def test_skipped_cohort_certs_are_logged_and_handler_does_not_raise() -> None: _call_handler(_BODY) # Assert — plan committed; skipped cert number surfaced in a log call - mock_uow.plan.save.assert_called_once() + mock_uow.plan.save_batch.assert_called_once() mock_uow.commit.assert_called_once() logged_messages = " ".join( str(c.args) + str(c.kwargs) for c in mock_logger.info.call_args_list @@ -635,7 +635,7 @@ def test_prediction_path_saves_predicted_epc_plan_and_baseline( mock_uow.epc.save_batch.assert_called_once_with( [EpcSaveRequest(mock_predicted_epc, property_id=PROPERTY_ID, portfolio_id=PORTFOLIO_ID, source="predicted")] ) - mock_uow.plan.save.assert_called_once() + mock_uow.plan.save_batch.assert_called_once() mock_uow.commit.assert_called_once() _baseline_orchestrator.return_value.run.assert_called_once_with([PROPERTY_ID]) @@ -842,7 +842,7 @@ def test_empty_own_postcode_broadens_to_nearby_and_predicts() -> None: mock_uow.epc.save_batch.assert_called_once_with( [EpcSaveRequest(mock_predicted_epc, property_id=PROPERTY_ID, portfolio_id=PORTFOLIO_ID, source="predicted")] ) - mock_uow.plan.save.assert_called_once() + mock_uow.plan.save_batch.assert_called_once() mock_uow.commit.assert_called_once() @@ -979,8 +979,9 @@ def test_batch_persists_in_one_transaction_and_one_baseline_run( "scenario_id": SCENARIO_ID, "refetch_solar": False, "dry_run": False} ) - # Assert — all three Plans saved, but a single shared transaction: - assert mock_uow.plan.save.call_count == 3 + # Assert — all three Plans saved in one batch call, but a single shared transaction: + mock_uow.plan.save_batch.assert_called_once() + assert len(mock_uow.plan.save_batch.call_args[0][0]) == 3 assert mock_uow.property.mark_modelled.call_count == 3 mock_uow.commit.assert_called_once() # One write Unit of Work opened for the whole batch, not one per property. @@ -1165,10 +1166,8 @@ def test_refetch_epc_false_with_stored_epc_skips_api_call() -> None: # Assert — API not called; stored EPC flows into run_modelling mock_epc_client.get_by_uprn.assert_not_called() mock_run_modelling.assert_called_once() - # Stored lodged EPC is persisted in the lodged slot - mock_uow.epc.save_batch.assert_called_once_with( - [EpcSaveRequest(stored_epc, property_id=PROPERTY_ID, portfolio_id=PORTFOLIO_ID, source="lodged")] - ) + # Stored EPC is NOT re-saved — it was read from DB unchanged (PR #1353) + mock_uow.epc.save_batch.assert_not_called() def test_refetch_epc_false_without_stored_epc_skips_api_and_goes_to_prediction() -> None: @@ -1388,11 +1387,9 @@ def test_repredict_epc_false_with_stored_predicted_epc_skips_prediction() -> Non # Act _call_handler({**_BODY, "repredict_epc": False}) - # Assert — EpcPrediction.predict never called; stored EPC persisted in predicted slot + # Assert — EpcPrediction.predict never called; stored predicted EPC NOT re-saved (PR #1353) mock_predictor.predict.assert_not_called() - mock_uow.epc.save_batch.assert_called_once_with( - [EpcSaveRequest(stored_predicted, property_id=PROPERTY_ID, portfolio_id=PORTFOLIO_ID, source="predicted")] - ) + mock_uow.epc.save_batch.assert_not_called() def test_repredict_epc_false_without_stored_predicted_epc_falls_back_to_live_prediction() -> None: diff --git a/tests/orchestration/fakes.py b/tests/orchestration/fakes.py index b068874e..d1294291 100644 --- a/tests/orchestration/fakes.py +++ b/tests/orchestration/fakes.py @@ -18,7 +18,7 @@ from domain.property_baseline.property_baseline_performance import PropertyBasel from domain.property.properties import Properties from domain.property.property import Property from repositories.epc.epc_postgres_repository import EpcSaveRequest -from repositories.plan.plan_repository import PlanRepository +from repositories.plan.plan_repository import PlanRepository, PlanSaveRequest from repositories.product.product_repository import ProductRepository from repositories.property_baseline.property_baseline_repository import PropertyBaselineRepository from repositories.epc.epc_repository import EpcRepository, EpcSource @@ -222,6 +222,18 @@ class FakePlanRepository(PlanRepository): self._next_id += 1 return plan_id + def save_batch(self, requests: list[PlanSaveRequest]) -> list[int]: + return [ + self.save( + r.plan, + property_id=r.property_id, + scenario_id=r.scenario_id, + portfolio_id=r.portfolio_id, + is_default=r.is_default, + ) + for r in requests + ] + class _UnsetProductRepo(ProductRepository): """Default for a `FakeUnitOfWork` built without a catalogue — raises if a