Remove MockOverdriveAPI in favor of fixture-seeded token caches (PP-4938)

[jonathangreen]

Description

Removes tests/mocks/overdrive.py and moves what it did into OverdriveAPIFixture.

MockOverdriveAPI did not mock any HTTP: it subclassed OverdriveAPI purely to pre-seed the client OAuth and collection token caches on construction, so tests would not spend queued responses on a token fetch. HTTP itself is mocked at HTTP.request_with_timeout by MockHttpClientFixture.

That seeding is now OverdriveAPIFixture.seed_token_caches, with create_api building an OverdriveAPI and seeding it. create_mock_api is renamed create_api to match.

One test in tests/manager/celery/tasks/test_overdrive.py patches the importer's OverdriveAPI reference with the class itself, and the patched symbol is passed to registry.equivalent(), so it has to remain a class rather than a fixture instance. That test gets a small local SeededOverdriveAPI subclass.

Motivation and Context

tests/mocks is being phased out per the project conventions, and this was one of the remaining users. Tests now exercise the real OverdriveAPI class rather than a subclass that only exists in the test tree.

This is the second of a series of PRs separating the Overdrive integration's HTTP concerns from its business logic (PP-4938). The only production change is dropping three docstrings on _do_get, _do_post and _do_patron_request that said the method was overridden in MockOverdriveAPI — they were already inaccurate, and now point at a class that no longer exists. No behavior changes.

How Has This Been Tested?

No behavior changed, so the existing tests are the check. Ran the Overdrive suite and everything else that touched the removed module:

tox -e py312-docker -- tests/manager/integration/license/overdrive/ tests/manager/celery/tasks/test_overdrive.py tests/manager/scripts/test_overdrive.py tests/manager/core/test_coverage.py

263 passed. mypy clean.

Checklist

• I have updated the documentation accordingly.
• All new and existing tests passed.

[claude]

Claude finished @jonathangreen's task in 2m 46s —— View job

---

Code review

• Gather context (read the diff and touched files)
• Review fixture changes in tests/fixtures/overdrive.py
• Review test call-site migrations and the local SeededOverdriveAPI
• Verify no lingering references to the removed module
• Post review

Summary

This is a clean, behavior-preserving refactor: seed_token_caches reproduces exactly what MockOverdriveAPI.__init__ did (same token strings, same 1-hour expiry), create_api is a faithful replacement for the partial(MockOverdriveAPI, db.session) factory, and self.db is assigned before create_api is called in OverdriveAPIFixture.__init__, so the in-constructor call is safe. I grepped the whole tree (.py/.md/.toml/.cfg) for MockOverdriveAPI, create_mock_api, and tests.mocks.overdrive — no references remain, and nothing in tests/mocks/__init__.py or the tooling config pointed at the deleted module. SeededOverdriveAPI's signature (_db, collection) matches the keyword call in importer.py:84 (OverdriveAPI(_db=self._db, collection=self._collection)), so the patched construction still works. The three removed docstrings were dead references to a test-only class; dropping them changes nothing at runtime. I found no bugs. (I could not run mypy or the suite here — this checkout has no installed dependencies and no database container — so I reviewed statically and relied on the CI run.)

One small note on the PR description rather than the code: it says the patched symbol must stay a class because it is passed to registry.equivalent(), but that test also patches LicenseProvidersRegistry.equivalent to return True (tests/manager/celery/tasks/test_overdrive.py:1195-1198), so the real constraint is just that the importer instantiates the patched symbol. The class docstring you added states this correctly.

Details

Nit: tests/fixtures/overdrive.py:60-75

seed_token_caches takes no fixture state, and its only out-of-module caller reaches through the class (OverdriveAPIFixture.seed_token_caches(self)) purely to get at a static helper. Making it a module-level function in this file would read more naturally at that call site, with create_api calling it the same way.

def seed_token_caches(api: OverdriveAPI) -> None:
    ...

# tests/manager/celery/tasks/test_overdrive.py
from tests.fixtures.overdrive import seed_token_caches
...
    seed_token_caches(self)

circulation/tests/fixtures/overdrive.py

Lines 60 to 75 in d0ddeb1

	@staticmethod
	def seed_token_caches(api: OverdriveAPI) -> None:
	"""Pre-seed the API's in-memory token caches.
	Almost every test would otherwise trigger a client token request (and
	a library document fetch for the collection token) as a side effect of
	its first API call, so seed both caches up front.
	"""
	api._cached_collection_token = OverdriveToken(
	token="fake collection token",
	expires=utc_now() + timedelta(hours=1),
	)
	api._cached_client_oauth_token = OverdriveToken(
	token="fake client oauth token",
	expires=utc_now() + timedelta(hours=1),
	)

• branch chore/overdrive-remove-mock-api

[greptile-apps]

Greptile Summary

The PR removes the test-only MockOverdriveAPI subclass and centralizes equivalent token-cache seeding in OverdriveAPIFixture; production behavior is unchanged.

• Constructs the real OverdriveAPI in OverDrive fixtures and seeds both token caches.
• Adds a local seeded subclass where the importer test must patch a class.
• Migrates fixture callers and removes obsolete mock references and docstrings.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
tests/fixtures/overdrive.py	Replaces the mock subclass factory with real OverdriveAPI construction and equivalent fixture-owned token-cache seeding.
tests/manager/celery/tasks/test_overdrive.py	Introduces a local seeded subclass that preserves the class-based importer patch and constructor contract.
src/palace/manager/integration/license/overdrive/api.py	Removes three obsolete docstrings without changing production behavior.
tests/mocks/overdrive.py	Deletes the redundant test subclass after migrating its cache-seeding responsibility to the fixture.

_{Reviews (4): Last reviewed commit: "Update tests/fixtures/overdrive.py" | Re-trigger Greptile}

[tdilauro]

Looks great! 🔥 🚀

One minor suggestion below for clarity.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.55%. Comparing base (f93b325) to head (d0ddeb1).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3678   +/-   ##
=======================================
  Coverage   93.55%   93.55%           
=======================================
  Files         513      513           
  Lines       46907    46907           
  Branches     6405     6405           
=======================================
+ Hits        43884    43885    +1     
+ Misses       1954     1953    -1     
  Partials     1069     1069           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.