Fix unreleated unresolved symbols (PP-1161) #1808
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1808 +/- ##
==========================================
+ Coverage 90.00% 90.01% +0.01%
==========================================
Files 298 298
Lines 39642 39638 -4
Branches 8597 8596 -1
==========================================
+ Hits 35680 35682 +2
+ Misses 2630 2625 -5
+ Partials 1332 1331 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| from palace.manager.core.monitor import CustomListEntryLicensePoolUpdateMonitor | ||
| from palace.manager.core.scripts import RunMonitorScript | ||
|
|
||
| RunMonitorScript(CustomListEntryLicensePoolUpdateMonitor).run() |
There was a problem hiding this comment.
CustomListEntryLicensePoolUpdateMonitor doesn't exist, and looks at git history hasn't for a long time (maybe not ever?). So remove the script.
| @@ -13,7 +13,6 @@ | |||
| import logging | |||
| import re | |||
| from collections import Counter | |||
| from importlib.resources import files | |||
|
|
||
| if self.primary_identifier in self.recommendations: | ||
| self.recommendations.remove(identifier_data) |
There was a problem hiding this comment.
identifier_data here was unresolved. This hasn't happened enough to have gotten to the top errors we see in prod, so it must not happen very often, but good to fix it.
I updated this to be a little safer as well. Since the identifiers could overlap, we put them into a set, combine them, and then remove the primary identifier if needed. Finally convert to list and update self.recommendations.
| @@ -143,14 +143,6 @@ def is_corporate_name(display_name): | |||
| return False | |||
|
|
|||
|
|
|||
| def is_one_name(human_name): | |||
There was a problem hiding this comment.
human_name 👽 was unresolved here. The function isn't used, so just remove it.
| @@ -249,25 +244,3 @@ def test__is_url(self): | |||
| # You can make specific URLs go through even if they | |||
| # wouldn't normally pass. | |||
| assert True == m("Not a URL", ["Not a URL", "Also not a URL"]) | |||
|
|
|||
|
|
|||
| class PatronAuthenticationValidatorFactoryTest: | |||
There was a problem hiding this comment.
This test class was never called because of its name: PatronAuthenticationValidatorFactoryTest rather then TestPatronAuthenticationValidatorFactory.
PatronAuthenticationValidatorFactory here is undefined and has long been removed, so remove this old test. It was also the only thing referencing the dummy_validator functions, so 🔥 those as well.
| primary_identifier, | ||
| ] | ||
| metadata.filter_recommendations(db.session) | ||
| assert [known_identifier] == metadata.recommendations |
There was a problem hiding this comment.
Add another test case here, so that we actually exercise the if statement that was broken in filter_recommendations.
jonathangreen
force-pushed
the
feature/put-into-package
branch
from
b1d07ad to
d30fc4c
Compare
jonathangreen
force-pushed
the
bugfix/unresolved-symbols
branch
from
445b20c to
6b7b6a8
Compare
Description
While testing #1802 I used the pycharm analysis to look through our code for unresolved symbols. I was trying to find any imports I had broken. I didn't find any broken imports, but I did find a number of issues that this PR resolves.
Motivation and Context
Testing for PP-1161
How Has This Been Tested?
Running tests in CI.
Checklist