Simplify and improve test_packages.py #2219
Conversation
| @@ -95,30 +95,21 @@ def package_helper(container: TrackedContainer) -> CondaPackageHelper: | |||
|
|
|||
|
|
|||
| @pytest.fixture(scope="function") | |||
| def packages(package_helper: CondaPackageHelper) -> dict[str, set[str]]: | |||
| def requested_packages(package_helper: CondaPackageHelper) -> dict[str, set[str]]: | |||
There was a problem hiding this comment.
This better shows an intention
|
|
||
| def python_package_predicate(package: str) -> bool: |
There was a problem hiding this comment.
Having so many predicates makes code difficult to read
| assert ( | ||
| exec_result.exit_code == 0 | ||
| ), f"Import package failed, output: {exec_result.output}" | ||
| assert exec_result.exit_code == 0, exec_result.output.decode("utf-8") |
There was a problem hiding this comment.
This .decode("utf-8") is the most important part of this PR - currently, logs are difficult to read, because they are printed as binary strings.
I've checked after change and logs are much better if some import fails.
| if len(failures) > 0: | ||
| raise AssertionError(failures) | ||
| failed_imports.append(package) | ||
| LOGGER.error(f"Failed to import package: {package}, output:\n {err}") |
There was a problem hiding this comment.
Nesting error message in a json makes it much worse when printing, simple LOGGER.error looks much better.
The downside is that there are several messages instead of one, but I actually prefer reading several beautiful messages than one unreadable json.
|
This is how the failure looked before this PR: |
Describe your changes
Issue ticket if applicable
Checklist (especially for first-time contributors)