Tweak mypy configuration

pyproject.toml

@@ -49,8 +49,11 @@ warn_unused_ignores = true
 # silences those errors, but only for the tests module.
 # See discussion here:
 # https://github.com/python/mypy/issues/2427
-disable_error_code = "method-assign"
+disable_error_code = "method-assign, union-attr"
 module = "tests.*"
+# Mypy seems to have issues in the test code where it thinks code is unreachable
+# when it is not. So we disable this warning for the tests module.
+warn_unreachable = false
 
 [[tool.mypy.overrides]]
 # This override is the equivalent of running mypy with the --strict flag.

tests/manager/api/controller/test_profile.py

@@ -91,11 +91,11 @@ def test_put(self, profile_fixture: ProfileFixture):
             assert request_patron.synchronize_annotations is True
 
             # The other patron is unaffected.
-            assert profile_fixture.other_patron.synchronize_annotations is False  # type: ignore[unreachable]
+            assert profile_fixture.other_patron.synchronize_annotations is False
 
         # Now we can create an annotation for the patron who enabled
         # annotation sync.
-        get_one_or_create(  # type: ignore[unreachable]
+        get_one_or_create(
             profile_fixture.db.session,
             Annotation,
             patron=request_patron,

tests/manager/api/test_authenticator.py

@@ -1549,7 +1549,7 @@ def test_authenticated_patron_only_calls_remote_patron_lookup_once(
             assert isinstance(patron, Patron)
             assert patron.external_type == "xyz"
         else:
-            assert patron is expected  # type: ignore[unreachable]
+            assert patron is expected
 
     def test_update_patron_metadata(
         self, db: DatabaseTransactionFixture, mock_basic: MockBasicFixture
@@ -1572,7 +1572,7 @@ def test_update_patron_metadata(
         # .neighborhood was not stored in .cached_neighborhood.  In
         # this case, it must be cheap to get .neighborhood every time,
         # and it's better not to store information we can get cheaply.
-        assert "Little Homeworld" == patron.neighborhood  # type: ignore[unreachable]
+        assert "Little Homeworld" == patron.neighborhood
         assert patron.cached_neighborhood is None
 
     def test_update_patron_metadata_noop_if_no_remote_metadata(

tests/manager/api/test_axis.py

@@ -1883,7 +1883,7 @@ def test_fetch_audiobook(self, axis360: Axis360Fixture):
         # document.
         assert fulfillment.content_type == DeliveryMechanism.FINDAWAY_DRM
         assert fulfillment.content is not None
-        assert isinstance(fulfillment.content, str)  # type: ignore[unreachable]
+        assert isinstance(fulfillment.content, str)
 
         # The manifest document combines information from the
         # fulfillment document and the metadata document.

tests/manager/api/test_routes.py

@@ -85,7 +85,7 @@ def test_index(self, fixture: RouteTestFixture):
 
     def test_authentication_document(self, fixture: RouteTestFixture):
         url = "/authentication_document"
-        fixture.assert_request_calls(url, fixture.controller.authentication_document)  # type: ignore[union-attr]
+        fixture.assert_request_calls(url, fixture.controller.authentication_document)
 
 
 class TestOPDSFeed:
@@ -99,7 +99,7 @@ def fixture(self, route_test: RouteTestFixture) -> RouteTestFixture:
     def test_acquisition_groups(self, fixture: RouteTestFixture):
         # An incoming lane identifier is passed in to the groups()
         # method.
-        method = fixture.controller.groups  # type: ignore[union-attr]
+        method = fixture.controller.groups
         fixture.assert_request_calls("/groups", method, None)
         fixture.assert_request_calls(
             "/groups/<lane_identifier>", method, "<lane_identifier>"
@@ -109,28 +109,28 @@ def test_feed(self, fixture: RouteTestFixture):
         # An incoming lane identifier is passed in to the feed()
         # method.
         url = "/feed"
-        fixture.assert_request_calls(url, fixture.controller.feed, None)  # type: ignore[union-attr]
+        fixture.assert_request_calls(url, fixture.controller.feed, None)
         url = "/feed/<lane_identifier>"
-        fixture.assert_request_calls(url, fixture.controller.feed, "<lane_identifier>")  # type: ignore[union-attr]
+        fixture.assert_request_calls(url, fixture.controller.feed, "<lane_identifier>")
 
     def test_navigation_feed(self, fixture: RouteTestFixture):
         # An incoming lane identifier is passed in to the navigation_feed()
         # method.
         url = "/navigation"
-        fixture.assert_request_calls(url, fixture.controller.navigation, None)  # type: ignore[union-attr]
+        fixture.assert_request_calls(url, fixture.controller.navigation, None)
         url = "/navigation/<lane_identifier>"
         fixture.assert_request_calls(
-            url, fixture.controller.navigation, "<lane_identifier>"  # type: ignore[union-attr]
+            url, fixture.controller.navigation, "<lane_identifier>"
         )
 
     def test_crawlable_library_feed(self, fixture: RouteTestFixture):
         url = "/crawlable"
-        fixture.assert_request_calls(url, fixture.controller.crawlable_library_feed)  # type: ignore[union-attr]
+        fixture.assert_request_calls(url, fixture.controller.crawlable_library_feed)
 
     def test_crawlable_list_feed(self, fixture: RouteTestFixture):
         url = "/lists/<list_name>/crawlable"
         fixture.assert_request_calls(
-            url, fixture.controller.crawlable_list_feed, "<list_name>"  # type: ignore[union-attr]
+            url, fixture.controller.crawlable_list_feed, "<list_name>"
         )
 
     def test_crawlable_collection_feed(self, fixture: RouteTestFixture):
@@ -143,21 +143,21 @@ def test_crawlable_collection_feed(self, fixture: RouteTestFixture):
 
     def test_lane_search(self, fixture: RouteTestFixture):
         url = "/search"
-        fixture.assert_request_calls(url, fixture.controller.search, None)  # type: ignore[union-attr]
+        fixture.assert_request_calls(url, fixture.controller.search, None)
 
         url = "/search/<lane_identifier>"
         fixture.assert_request_calls(
-            url, fixture.controller.search, "<lane_identifier>"  # type: ignore[union-attr]
+            url, fixture.controller.search, "<lane_identifier>"
         )
 
     def test_qa_feed(self, fixture: RouteTestFixture):
         url = "/feed/qa"
-        fixture.assert_authenticated_request_calls(url, fixture.controller.qa_feed)  # type: ignore[union-attr]
+        fixture.assert_authenticated_request_calls(url, fixture.controller.qa_feed)
 
     def test_qa_series_feed(self, fixture: RouteTestFixture):
         url = "/feed/qa/series"
         fixture.assert_authenticated_request_calls(
-            url, fixture.controller.qa_series_feed  # type: ignore[union-attr]
+            url, fixture.controller.qa_series_feed
         )
 
 
@@ -171,7 +171,7 @@ def fixture(self, route_test: RouteTestFixture) -> RouteTestFixture:
 
     def test_marc_page(self, fixture: RouteTestFixture):
         url = "/marc"
-        fixture.assert_request_calls(url, fixture.controller.download_page)  # type: ignore[union-attr]
+        fixture.assert_request_calls(url, fixture.controller.download_page)
 
 
 class TestProfileController:
@@ -186,7 +186,7 @@ def test_patron_profile(self, fixture: RouteTestFixture):
         url = "/patrons/me"
         fixture.assert_authenticated_request_calls(
             url,
-            fixture.controller.protocol,  # type: ignore[union-attr]
+            fixture.controller.protocol,
         )
 
 
@@ -202,15 +202,15 @@ def test_active_loans(self, fixture: RouteTestFixture):
         url = "/loans"
         fixture.assert_authenticated_request_calls(
             url,
-            fixture.controller.sync,  # type: ignore[union-attr]
+            fixture.controller.sync,
         )
         fixture.assert_supported_methods(url, "GET", "HEAD")
 
     def test_borrow(self, fixture: RouteTestFixture):
         url = "/works/<identifier_type>/<identifier>/borrow"
         fixture.assert_request_calls_method_using_identifier(
             url,
-            fixture.controller.borrow,  # type: ignore[union-attr]
+            fixture.controller.borrow,
             "<identifier_type>",
             "<identifier>",
             None,
@@ -221,7 +221,7 @@ def test_borrow(self, fixture: RouteTestFixture):
         url = "/works/<identifier_type>/<identifier>/borrow/<mechanism_id>"
         fixture.assert_request_calls_method_using_identifier(
             url,
-            fixture.controller.borrow,  # type: ignore[union-attr]
+            fixture.controller.borrow,
             "<identifier_type>",
             "<identifier>",
             "<mechanism_id>",
@@ -235,18 +235,18 @@ def test_fulfill(self, fixture: RouteTestFixture):
         # open-access titles.
         url = "/works/<license_pool_id>/fulfill"
         fixture.assert_request_calls(
-            url, fixture.controller.fulfill, "<license_pool_id>", None  # type: ignore[union-attr]
+            url, fixture.controller.fulfill, "<license_pool_id>", None
         )
 
         url = "/works/<license_pool_id>/fulfill/<mechanism_id>"
         fixture.assert_request_calls(
-            url, fixture.controller.fulfill, "<license_pool_id>", "<mechanism_id>"  # type: ignore[union-attr]
+            url, fixture.controller.fulfill, "<license_pool_id>", "<mechanism_id>"
         )
 
     def test_revoke_loan_or_hold(self, fixture: RouteTestFixture):
         url = "/loans/<license_pool_id>/revoke"
         fixture.assert_authenticated_request_calls(
-            url, fixture.controller.revoke, "<license_pool_id>"  # type: ignore[union-attr]
+            url, fixture.controller.revoke, "<license_pool_id>"
         )
 
         fixture.assert_supported_methods(url, "GET", "PUT")
@@ -255,7 +255,7 @@ def test_loan_or_hold_detail(self, fixture: RouteTestFixture):
         url = "/loans/<identifier_type>/<identifier>"
         fixture.assert_request_calls_method_using_identifier(
             url,
-            fixture.controller.detail,  # type: ignore[union-attr]
+            fixture.controller.detail,
             "<identifier_type>",
             "<identifier>",
             authenticated=True,
@@ -273,21 +273,21 @@ def fixture(self, route_test: RouteTestFixture) -> RouteTestFixture:
 
     def test_annotations(self, fixture: RouteTestFixture):
         url = "/annotations/"
-        fixture.assert_authenticated_request_calls(url, fixture.controller.container)  # type: ignore[union-attr]
+        fixture.assert_authenticated_request_calls(url, fixture.controller.container)
         fixture.assert_supported_methods(url, "HEAD", "GET", "POST")
 
     def test_annotation_detail(self, fixture: RouteTestFixture):
         url = "/annotations/<annotation_id>"
         fixture.assert_authenticated_request_calls(
-            url, fixture.controller.detail, "<annotation_id>"  # type: ignore[union-attr]
+            url, fixture.controller.detail, "<annotation_id>"
         )
         fixture.assert_supported_methods(url, "HEAD", "GET", "DELETE")
 
     def test_annotations_for_work(self, fixture: RouteTestFixture):
         url = "/annotations/<identifier_type>/<identifier>"
         fixture.assert_request_calls_method_using_identifier(
             url,
-            fixture.controller.container_for_work,  # type: ignore[union-attr]
+            fixture.controller.container_for_work,
             "<identifier_type>",
             "<identifier>",
             authenticated=True,
@@ -305,7 +305,7 @@ def fixture(self, route_test: RouteTestFixture) -> RouteTestFixture:
 
     def test_work(self, fixture: RouteTestFixture):
         url = "/works"
-        fixture.assert_request_calls(url, fixture.controller.work_lookup, "work")  # type: ignore[union-attr]
+        fixture.assert_request_calls(url, fixture.controller.work_lookup, "work")
 
 
 class TestWorkController:
@@ -319,14 +319,14 @@ def fixture(self, route_test: RouteTestFixture) -> RouteTestFixture:
     def test_contributor(self, fixture: RouteTestFixture):
         url = "/works/contributor/<contributor_name>"
         fixture.assert_request_calls(
-            url, fixture.controller.contributor, "<contributor_name>", None, None  # type: ignore[union-attr]
+            url, fixture.controller.contributor, "<contributor_name>", None, None
         )
 
     def test_contributor_language(self, fixture: RouteTestFixture):
         url = "/works/contributor/<contributor_name>/<languages>"
         fixture.assert_request_calls(
             url,
-            fixture.controller.contributor,  # type: ignore[union-attr]
+            fixture.controller.contributor,
             "<contributor_name>",
             "<languages>",
             None,
@@ -336,7 +336,7 @@ def test_contributor_language_audience(self, fixture: RouteTestFixture):
         url = "/works/contributor/<contributor_name>/<languages>/<audiences>"
         fixture.assert_request_calls(
             url,
-            fixture.controller.contributor,  # type: ignore[union-attr]
+            fixture.controller.contributor,
             "<contributor_name>",
             "<languages>",
             "<audiences>",
@@ -345,20 +345,20 @@ def test_contributor_language_audience(self, fixture: RouteTestFixture):
     def test_series(self, fixture: RouteTestFixture):
         url = "/works/series/<series_name>"
         fixture.assert_request_calls(
-            url, fixture.controller.series, "<series_name>", None, None  # type: ignore[union-attr]
+            url, fixture.controller.series, "<series_name>", None, None
         )
 
     def test_series_language(self, fixture: RouteTestFixture):
         url = "/works/series/<series_name>/<languages>"
         fixture.assert_request_calls(
-            url, fixture.controller.series, "<series_name>", "<languages>", None  # type: ignore[union-attr]
+            url, fixture.controller.series, "<series_name>", "<languages>", None
         )
 
     def test_series_language_audience(self, fixture: RouteTestFixture):
         url = "/works/series/<series_name>/<languages>/<audiences>"
         fixture.assert_request_calls(
             url,
-            fixture.controller.series,  # type: ignore[union-attr]
+            fixture.controller.series,
             "<series_name>",
             "<languages>",
             "<audiences>",
@@ -367,19 +367,19 @@ def test_series_language_audience(self, fixture: RouteTestFixture):
     def test_permalink(self, fixture: RouteTestFixture):
         url = "/works/<identifier_type>/<identifier>"
         fixture.assert_request_calls_method_using_identifier(
-            url, fixture.controller.permalink, "<identifier_type>", "<identifier>"  # type: ignore[union-attr]
+            url, fixture.controller.permalink, "<identifier_type>", "<identifier>"
         )
 
     def test_recommendations(self, fixture: RouteTestFixture):
         url = "/works/<identifier_type>/<identifier>/recommendations"
         fixture.assert_request_calls_method_using_identifier(
-            url, fixture.controller.recommendations, "<identifier_type>", "<identifier>"  # type: ignore[union-attr]
+            url, fixture.controller.recommendations, "<identifier_type>", "<identifier>"
         )
 
     def test_related_books(self, fixture: RouteTestFixture):
         url = "/works/<identifier_type>/<identifier>/related_books"
         fixture.assert_request_calls_method_using_identifier(
-            url, fixture.controller.related, "<identifier_type>", "<identifier>"  # type: ignore[union-attr]
+            url, fixture.controller.related, "<identifier_type>", "<identifier>"
         )
 
 
@@ -398,7 +398,7 @@ def test_track_analytics_event(self, fixture: RouteTestFixture):
         # unauthenticated.
         fixture.assert_request_calls_method_using_identifier(
             url,
-            fixture.controller.track_event,  # type: ignore[union-attr]
+            fixture.controller.track_event,
             "<identifier_type>",
             "<identifier>",
             "<event_type>",
@@ -417,14 +417,16 @@ def fixture(self, route_test: RouteTestFixture) -> RouteTestFixture:
 
     def test_odl_notify_deprecated(self, fixture: RouteTestFixture):
         url = "/odl_notify/<loan_id>"
-        fixture.assert_request_calls(url, fixture.controller.notify_deprecated, "<loan_id>")  # type: ignore[union-attr]
+        fixture.assert_request_calls(
+            url, fixture.controller.notify_deprecated, "<loan_id>"
+        )
         fixture.assert_supported_methods(url, "GET", "POST")
 
     def test_odl_notify(self, fixture: RouteTestFixture):
         url = "/odl/notify/<patron_identifier>/<license_identifier>"
         fixture.assert_request_calls(
             url,
-            fixture.controller.notify,  # type: ignore[union-attr]
+            fixture.controller.notify,
             "<patron_identifier>",
             "<license_identifier>",
             http_method="POST",
@@ -442,7 +444,7 @@ def fixture(self, route_test: RouteTestFixture) -> RouteTestFixture:
 
     def test_heartbeat(self, fixture: RouteTestFixture):
         url = "/version.json"
-        fixture.assert_request_calls(url, fixture.controller.version)  # type: ignore[union-attr]
+        fixture.assert_request_calls(url, fixture.controller.version)
 
 
 class TestHealthCheck:

tests/manager/api/test_selftest.py

@@ -88,7 +88,7 @@ def test__determine_self_test_patron(
             with pytest.raises(test_patron_lookup_exception) as excinfo:
                 test_patron_lookup_method(db.default_library())
         assert not isinstance(result_patron, (Patron, ProblemDetail))
-        assert expected_message == excinfo.value.message  # type: ignore
+        assert expected_message == excinfo.value.message
         assert excinfo.value.detail is None
 
     def test_default_patrons(

tests/manager/marc/test_uploader.py

@@ -64,10 +64,10 @@ def test_begin(
             assert uploader.locked is True
 
             # The lock is also reflected in the uploads object
-            assert marc_upload_manager_fixture.uploads.locked(by_us=True) is True  # type: ignore[unreachable]
+            assert marc_upload_manager_fixture.uploads.locked(by_us=True) is True
 
         # The lock is released after the context manager exits
-        assert uploader.locked is False  # type: ignore[unreachable]
+        assert uploader.locked is False
         assert marc_upload_manager_fixture.uploads.locked(by_us=True) is False
 
         # If an exception occurs, the lock is deleted and the exception is raised by calling

tests/manager/service/logging/test_configuration.py

@@ -35,7 +35,7 @@ def test_cloudwatch_region_valid() -> None:
 class TestLogLevel:
     def test_level_string(self) -> None:
         assert LogLevel.debug == "DEBUG"
-        assert LogLevel.info == "INFO"  # type: ignore[unreachable]
+        assert LogLevel.info == "INFO"
         assert LogLevel.warning == "WARNING"
         assert LogLevel.error == "ERROR"
 

tests/manager/sqlalchemy/model/test_licensing.py

@@ -719,7 +719,7 @@ def test_update_availability(self, db: DatabaseTransactionFixture):
         assert work.last_update_time is not None
 
         # Updating availability also modified work.last_update_time.
-        assert (utc_now() - work.last_update_time) < datetime.timedelta(seconds=2)  # type: ignore[unreachable]
+        assert (utc_now() - work.last_update_time) < datetime.timedelta(seconds=2)
 
     def test_update_availability_does_nothing_if_given_no_data(
         self, db: DatabaseTransactionFixture