-
Notifications
You must be signed in to change notification settings - Fork 86
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
chore(python): refactor unit / system test dependency install #1186
chore(python): refactor unit / system test dependency install #1186
Conversation
session.install("-e", *UNIT_TEST_LOCAL_DEPENDENCIES, *constraints) | ||
|
||
if UNIT_TEST_DEPENDENCIES: | ||
session.install("-e", *UNIT_TEST_DEPENDENCIES, *constraints) |
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.
These aren't supposed to be installed in "editable" mode, are they?
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.
Likely not -- I doubt anybody is using a git:
URL for a dependency.
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.
@@ -81,29 +126,38 @@ def lint_setup_py(session): | |||
session.run("python", "setup.py", "check", "--restructuredtext", "--strict") | |||
|
|||
|
|||
def install_unittest_dependencies(session, *constraints): | |||
session.install(*UNIT_TEST_STANDARD_DEPENDENCIES, *constraints) |
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'd prefer if we installed UNIT_TEST_STANDARD_DEPENDENCIES and UNIT_TEST_DEPENDENCIES all in one go.
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.
Hmm, I'm not sure I see why? The prior template alwasy installed the "standard" deps ("mock", "asyncmock", "pytest", "pytest-cov", "pytest-asyncio"
) in a separate step, prior to any others.
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.
The reason is that the pip resolver can run in one go. Otherwise subsequent pip installs might have to downgrade some packages.
"pytest-cov", | ||
"pytest-asyncio", | ||
] | ||
UNIT_TEST_EXTERNAL_DEPENDENCIES = [{% for v in unit_test_external_dependencies %} |
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.
Do we want to deprecate unit_test_external_dependencies
? It seems quire redundant to me with unit_test_dependencies
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.
PR made from a now-removed fork. Replaced by #1294. |
Closes #1185.
Opened PR as a draft for discussion.