Skip to content

Commit

Permalink
[PP-1708] Ensures that the overdrive new titles script does not fail …
Browse files Browse the repository at this point in the history
…when cover… (#2050)

* Ensures that the overdrive new titles script does not fail when coverage records are added.
  • Loading branch information
dbernstein authored Sep 11, 2024
1 parent 3784c8f commit 1c86b3b
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 10 deletions.
6 changes: 2 additions & 4 deletions src/palace/manager/core/coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
13 changes: 7 additions & 6 deletions tests/manager/core/test_coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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]
Expand All @@ -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]
Expand Down Expand Up @@ -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."
Expand Down Expand Up @@ -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
):
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions tests/mocks/mock.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 1c86b3b

Please sign in to comment.