871 increase test coverage for holdings_csv_transformer#936
871 increase test coverage for holdings_csv_transformer#936
Conversation
|
@mtrineyev Should this one still be considered "in-progress"? |
mtrineyev
changed the title
…le not found scenarios
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #936 +/- ##
==========================================
+ Coverage 74.83% 74.92% +0.08%
==========================================
Files 106 106
Lines 12841 12883 +42
Branches 1677 1680 +3
==========================================
+ Hits 9610 9652 +42
Misses 2900 2900
Partials 331 331
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| ) | ||
| else: | ||
| # Regular holding. Merge according to criteria | ||
| # Regular holding. Merge, according to criteria |
| call_number_type_map_f, "Found %s rows in call number type map" | ||
| ) | ||
| except FileNotFoundError: | ||
| logging.critical( |
There was a problem hiding this comment.
@mtrineyev The entry point function in main.py catches file not found errors. We should re-raise and let let the exception propagate, rather than calling sys.exit here, directly.
There was a problem hiding this comment.
Just raise should be sufficient. We want to preserve the original exception context.
| f"Location map file {self.task_configuration.location_map_file_name} " | ||
| "not found. Halting..." | ||
| ) | ||
| sys.exit(1) |
| "not found. Halting..." | ||
| ) | ||
| return mapped_fields, holdings_map | ||
| sys.exit(1) |
There was a problem hiding this comment.
You'll need to adjust the tests to account for allowing the exception to propagate.
…instead of SystemExit
|
| ) | ||
| from folio_migration_tools.migration_report import MigrationReport | ||
| from folio_migration_tools.migration_tasks.holdings_csv_transformer import ( | ||
| from src.folio_migration_tools.migration_tasks.holdings_csv_transformer import ( |
There was a problem hiding this comment.
Not sure why you've added src to the beginning of the import statement(s) here. The test should be run after the project is "installed" in the environment, so the regular imports should work. Or am I misunderstanding something?
2 participants
Purpose
Fixes #871
Changes Made in this PR
Code Review Specifics
Task Checklist
nox -rs safety.pre-commit run --all-filesnox -rs testspython -m folio_migration_tools -hHow to Verify