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
DM-35901: Add new test case for importing every python file in a package #165
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #165 +/- ##
==========================================
+ Coverage 93.95% 93.97% +0.01%
==========================================
Files 43 44 +1
Lines 2831 2870 +39
==========================================
+ Hits 2660 2697 +37
- Misses 171 173 +2
☔ View full report in Codecov by Sentry. |
There are a couple of other places using %-format but those are generally cases where it's a bit cleaner to use than an f-string.
resources.contents() has been deprecated.
We are not required to support python 3.8 and the modern API for importlib.resources requires either python 3.9 or a dependency on the importlib_resources package.
try: | ||
doImport(module_name) | ||
except ImportError as e: | ||
raise AssertionError(f"Error importing module {module_name}: {e}") from e |
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.
Is it possible to test the failing case? Maybe by writing out a junk python file to a temp directory?
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.
I'm not entirely sure how I would catch the failure since I'm not writing the test that is failing, the infrastructure is writing it and attaching it directly to this class.
Checklist
doc/changes