Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #58 +/- ##
==========================================
- Coverage 64.86% 64.80% -0.07%
==========================================
Files 28 28
Lines 6469 6469
==========================================
- Hits 4196 4192 -4
- Misses 2273 2277 +4
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:
|
…systematically remove the commented out imports since we are feelinmg better about this idea
…the workflow. pylint threshold to 7.0
There was a problem hiding this comment.
Pull request overview
This PR refactors the test suite to eliminate wildcard imports (from . import *) and replace them with explicit imports, improving code maintainability and pylint compliance. The changes also include a bug fix and CI workflow optimization.
Changes:
- Replaced wildcard imports with explicit imports across 13 test files
- Emptied
test/__init__.py(removed 45-line import block) - Fixed typo in
epmt_query.py(unavailable__metrics→unavailable_metrics) - Optimized CI workflow by moving pylint checks after tests and increasing threshold from 6.5 to 7.0
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/init.py | Removed all imports, now empty file |
| test_submit.py | Added explicit imports for unittest, glob, datetime, settings, query, models, and orm functions |
| test_stat.py | Removed commented wildcard import and shebang |
| test_shell.py | Added explicit unittest import, removed wildcard import |
| test_settings.py | Added explicit imports, reordered imports for clarity |
| test_run.py | Added explicit imports for os, environ, shutil, and epmt modules |
| test_query.py | Added comprehensive explicit imports for all dependencies |
| test_outliers.py | Added explicit imports including json, glob, unittest, and epmt modules |
| test_lib.py | Added explicit imports using multi-line import statement |
| test_explore.py | Added explicit imports for epmt modules and utilities |
| test_db_schema.py | Added explicit imports for unittest and orm modules |
| test_db_migration.py | Added explicit imports for settings and orm modules |
| test_cmds.py | Added comprehensive explicit imports for all test dependencies |
| test_anysh.py | Added explicit imports and simplified comments |
| epmt_query.py | Fixed typo in variable name (double to single underscore) |
| build_and_test_epmt.yml | Moved pylint checks to end of workflow, increased threshold to 7.0 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
|
|
||
| # the import below is crucial to get a sane test environment | ||
| from epmt.orm.sqlalchemy.general import orm_get, orm_to_dict |
There was a problem hiding this comment.
The function orm_to_dict is imported here but used inconsistently in the file. It's called as eq.orm_to_dict on line 40 but as orm_to_dict on line 642. Since it's imported directly, both usages should be orm_to_dict (without the eq. prefix) for consistency.
| (path.getsize(install_root + '/settings.py') > 0)) | ||
| try: | ||
| import epmt.epmt_settings as settings | ||
| assert settings is not None |
There was a problem hiding this comment.
This assertion is unnecessary. If the import statement on line 32 succeeds, settings will be defined and not None. If the import fails, the except block on line 34 will catch it. This assertion adds no value and should be removed.
| assert settings is not None |
| from epmt.epmtlib import timing, capture, epmt_logging_init, get_install_root, str_dict | ||
| import epmt.epmt_query as eq | ||
|
|
||
| install_root=get_install_root() |
There was a problem hiding this comment.
Missing spaces around the assignment operator. Should be install_root = get_install_root() to comply with PEP 8 style guidelines, as seen in other test files like test_explore.py (line 12), test_submit.py (line 15), and test_run.py (line 15).
| install_root=get_install_root() | |
| install_root = get_install_root() |
| from epmt.epmt_cmds import epmt_submit | ||
| from epmt.orm.sqlalchemy.models import Job | ||
|
|
||
| install_root=get_install_root() |
There was a problem hiding this comment.
Missing spaces around the assignment operator. Should be install_root = get_install_root() to comply with PEP 8 style guidelines, as seen in other test files like test_explore.py (line 12), test_submit.py (line 15), and test_run.py (line 15).
| install_root=get_install_root() | |
| install_root = get_install_root() |
|
|
||
| # from epmt.orm.sqlalchemy.models import UnprocessedJob | ||
| # from os import path | ||
| install_root=get_install_root() |
There was a problem hiding this comment.
Missing spaces around the assignment operator. Should be install_root = get_install_root() to comply with PEP 8 style guidelines, as seen in other test files like test_explore.py (line 12), test_submit.py (line 15), and test_run.py (line 15).
| install_root=get_install_root() | |
| install_root = get_install_root() |
| @@ -1,11 +1,13 @@ | |||
| #!/usr/bin/env python | |||
| import os | |||
There was a problem hiding this comment.
Module 'os' is imported with both 'import' and 'import from'.
|
|
||
| from . import * | ||
| # import os | ||
| import os |
There was a problem hiding this comment.
Module 'os' is imported with both 'import' and 'import from'.
Describe your changes
This PR refactors the test suite to replace wildcard imports (
from . import *) with explicit imports, improving code clarity and pylint compliance.Key Changes:
Removed wildcard imports from
epmt.test: All test modules now use explicit imports instead offrom . import *test/__init__.pynow empty: Removed the 45-line monolithic import blockUpdated CI workflow: Moved
pylintchecks to end of workflow andpylintthreshold increased from 6.5 to 7.0Bug fix: Corrected typo in
epmt_query.py(unavailable__metrics→unavailable_metrics)Benefits:
Issue ticket number, link (if applicable)
No associated issue
Checklist