-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PP-1219: Fixes overdrive monitor failures. #1819
PP-1219: Fixes overdrive monitor failures. #1819
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just a couple minor comments in the test.
tests/api/test_overdrive.py
Outdated
@@ -2233,6 +2233,15 @@ def test_no_drm_fulfillment(self, overdrive_api_fixture: OverdriveAPIFixture): | |||
assert fulfill.content_link_redirect is True | |||
assert fulfill.content_link == "https://example.org/epub-redirect" | |||
|
|||
def test_bug_fix_pp_1219(self, overdrive_api_fixture: OverdriveAPIFixture): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we give this test method a more descriptive name? I'm guessing something like test_no_recently_changed_books
.
tests/api/test_overdrive.py
Outdated
) as get_book_list: | ||
get_book_list.return_value = ([], None) | ||
result = overdrive_api_fixture.api.recently_changed_ids(utc_now(), None) | ||
# this should no longer fail. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leftover comment?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## hotfix/19.0.1 #1819 +/- ##
=================================================
- Coverage 90.32% 90.32% -0.01%
=================================================
Files 260 260
Lines 28733 28733
Branches 6525 6525
=================================================
- Hits 25954 25953 -1
Misses 1840 1840
- Partials 939 940 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Description
This update fixes a bug introduced in #1783. It appears that the way I have overridden the NewTitlesOverdriveMonitor construct is resulting in a failure in the dependency injection code when running in production. Additionally there was a string interpolation bug in an area of the code that was not previously covered.
When this PR has been approved I'll deploy a hotfix as well as create a PR against main with these changes and I'll create a 20.0.1 release before deploying out to staging.
Motivation and Context
https://ebce-lyrasis.atlassian.net/browse/PP-1219
How Has This Been Tested?
Checklist