From 1c86b3b33b44470362fc63cb9ccf7efad4af562f Mon Sep 17 00:00:00 2001 From: dbernstein Date: Wed, 11 Sep 2024 09:30:14 -0700 Subject: [PATCH] =?UTF-8?q?[PP-1708]=20Ensures=20that=20the=20overdrive=20?= =?UTF-8?q?new=20titles=20script=20does=20not=20fail=20when=20cover?= =?UTF-8?q?=E2=80=A6=20(#2050)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Ensures that the overdrive new titles script does not fail when coverage records are added. --- src/palace/manager/core/coverage.py | 6 ++---- tests/manager/core/test_coverage.py | 13 +++++++------ tests/mocks/mock.py | 4 ++++ 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/palace/manager/core/coverage.py b/src/palace/manager/core/coverage.py index 6a21cb5aa4..53cf0e5ad8 100644 --- a/src/palace/manager/core/coverage.py +++ b/src/palace/manager/core/coverage.py @@ -383,7 +383,7 @@ def run_once(self, progress, count_as_covered=None): transient_failures, persistent_failures, ), results = self.process_batch_and_handle_results(batch_results) - + self.finalize_batch() # Update the running totals so that the service's eventual timestamp # will have a useful .achievements. progress.successes += successes @@ -486,9 +486,6 @@ def process_batch_and_handle_results(self, batch): num_ignored, ) - # Finalize this batch before moving on to the next one. - self.finalize_batch() - # For all purposes outside this method, treat an ignored identifier # as a transient failure. transient_failures += num_ignored @@ -910,6 +907,7 @@ def run_on_specific_identifiers(self, identifiers): while index < len(need_coverage): batch = need_coverage[index : index + self.batch_size] (s, t, p), r = self.process_batch_and_handle_results(batch) + self.finalize_batch() successes += s transient_failures += t persistent_failures += p diff --git a/tests/manager/core/test_coverage.py b/tests/manager/core/test_coverage.py index 41d809e5e1..fabe00c560 100644 --- a/tests/manager/core/test_coverage.py +++ b/tests/manager/core/test_coverage.py @@ -515,6 +515,7 @@ def process_batch_and_handle_results(self, batch): assert 1 == progress.successes assert 2 == progress.transient_failures assert 3 == progress.persistent_failures + assert provider.finalize_batch_called assert ( "Items processed: 6. Successes: 1, transient failures: 2, persistent failures: 3" @@ -535,9 +536,6 @@ def test_process_batch_and_handle_results(self, db: DatabaseTransactionFixture): class MockProvider1(AlwaysSuccessfulCoverageProvider): OPERATION = "i succeed" - def finalize_batch(self): - self.finalized = True - success_provider = MockProvider1(db.session) batch = [i1, i2] @@ -546,9 +544,6 @@ def finalize_batch(self): # Two successes. assert (2, 0, 0) == counts - # finalize_batch() was called. - assert True == success_provider.finalized - # Each represented with a CoverageRecord with status='success' assert all(isinstance(x, CoverageRecord) for x in successes) assert [CoverageRecord.SUCCESS] * 2 == [x.status for x in successes] @@ -614,6 +609,9 @@ class MockProvider4(NeverSuccessfulCoverageProvider): assert [CoverageRecord.PERSISTENT_FAILURE] * 2 == [x.status for x in results] assert ["i will always fail"] * 2 == [x.operation for x in results] + assert not success_provider.finalize_batch_called + assert not persistent_failure_provider.finalize_batch_called + def test_process_batch(self, db: DatabaseTransactionFixture): class Mock(BaseCoverageProvider): SERVICE_NAME = "Some succeed, some fail." @@ -1156,6 +1154,8 @@ def test_run_on_specific_identifiers(self, db: DatabaseTransactionFixture): for i in not_to_be_tested: assert i not in provider.attempts + assert provider.finalize_batch_called + def test_run_on_specific_identifiers_respects_cutoff_time( self, db: DatabaseTransactionFixture ): @@ -1199,6 +1199,7 @@ def test_run_on_specific_identifiers_respects_cutoff_time( # reflect the failure. assert records[0] == record assert "What did you expect?" == record.exception + assert provider.finalize_batch_called def test_run_never_successful(self, db: DatabaseTransactionFixture): """Verify that NeverSuccessfulCoverageProvider works the diff --git a/tests/mocks/mock.py b/tests/mocks/mock.py index f0fa9af57d..cdca136a35 100644 --- a/tests/mocks/mock.py +++ b/tests/mocks/mock.py @@ -102,11 +102,15 @@ class InstrumentedCoverageProvider(MockCoverageProvider, IdentifierCoverageProvi def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.attempts = [] + self.finalize_batch_called = False def process_item(self, item): self.attempts.append(item) return item + def finalize_batch(self): + self.finalize_batch_called = True + class InstrumentedWorkCoverageProvider(MockCoverageProvider, WorkCoverageProvider): """A WorkCoverageProvider that keeps track of every item it tried