owned this note
owned this note
Published
Linked with GitHub
# Failing pip tests for the new resolver
## General Notes
Note that the new resolver calls the finder via `find_best_candidate`, whereas the old resolver calls it via `find_requirement`. So we don't do any of the post-processing in `find_requirement`. It looks like it's only messaging, though, so probably not something we need to worry too much about. Or at least we'll handle those issues a different way.
## Different messages
### `test_upgrade_with_newest_already_installed`
### `test_only_if_needed_does_not_upgrade_deps_when_satisfied`
Does not print the expected log from the legacy resolver.
```
Requirement already satisfied, skipping upgrade: simple
```
Paul: Not sure whether the new resolver can even distinguish this case.
### `test_install_pip_does_not_modify_pip_when_satisfied[upgrade=True]`
The legacy resolver prints
```
Requirement already up-to-date: pip in
```
but the new resolver prints
```
Requirement already satisfied: pip in
```
I think the difference between these two are simply whether we are in `upgrade` mode or not?
### `test_install_distribution_union_conflicting_extras`
This test was actually written with to ensure the new resolver can detect the conflict introduced by the extra. We are doing that alright, but not using the work "Conflict" as anticipated by the people before us. So this is a messaging difference 😏
### `test_hashed_install_failure_later_flag`
```
ERROR: Hashes are required in --require-hashes mode, but they are missing
from some requirements. Here is a list of those requirements along with the
hashes their downloaded archives actually had. Add lines like these to your
requirements files to prevent tampering. (If you did not enable
--require-hashes manually, note that it turns on automatically when any
package has a hash.)
unknown package --hash=sha256:9ee5702ccd5d16d54ac9dcfce4a7c8ac90020cfe4e6b494ca44c68492add364c
```
Two problems here:
1. The new resolver does not populate `HashError.req`, which the legacy resolver does [here](https://github.com/pypa/pip/blob/5cac4dc6bd4c2150380d5368f2541357364b5d4b/src/pip/_internal/resolution/legacy/resolver.py#L182)
2. The new resolver fails at the first hash error, instead of collecting all hash errors before displaying all of them (with a `HashErrors` exception)
The first problem is easy to resolve; we can simply catch `HashError` [here](https://github.com/pypa/pip/blob/5cac4dc6bd4c2150380d5368f2541357364b5d4b/src/pip/_internal/resolution/resolvelib/candidates.py#L163) and set the attribute.
I don’t see how the second problem can be dealt with, however. The new resolver resolves one thing at a time, and one a hash error it does not actually make sense to continue resolution. I guess it is theoratically possible to continue resolving and stop at a later time, but TBH I don’t think the extra complexity is worth it.
This is probably one of the situations we should just let the new resolver have a different error message.
## Yanked
Does not print warning for yanked candidates yet.
### `test_install_yanked_file_and_print_warning`
**Fixed by**: [`new-resolver-yanked-warning`](https://github.com/pypa/pip/pull/8340) (TP)
## Excluding incompatible wheels specified by direct path
### `test_install_nonlocal_compatible_wheel_path`
### `test_install_unsupported_wheel_file`
We are not rejecting incompatibile wheels if they are specified via direct URL.
**Fixed by**: [`new-resolver-reject-unsupported-wheel`](https://github.com/pypa/pip/pull/8349) (TP)
## Uncategorised
### `test_install_with_extras_from_install`
Test the extra selected by the `install` line can be correctly merged with the constraint. This test used to depend on the contrain-to-URL feature, but does not need to. There are other tests making sure URL contraints work, this doesn’t need to duplicate the check.
**Fixed by:** [`constraint-tests-rewrite`](https://github.com/pypa/pip/pull/8382) (TP)
### `test_upgrade_to_same_version_from_url`
The legacy resolver does not install a direct URL req if there is a “matching” (same version) dist already installed. I’m not quite sure this is the correct behaviour. Discussions needed.
https://github.com/pypa/pip/commit/e51fdce95d8d03abe38c89d94e713e0369f15883
> Resolution: It’s “no need” to do this. We should still do this anyway, but don’t need to panic for the fact we don’t.
> Paul: There is a genuine bug here, but I don’t know how to fix it yet.
> TP: I’ll leave this to you then 😛
Work in progress: [`nr_upgrade_to_same_version_from_url`](https://github.com/pypa/pip/pull/8359) (Paul)
----
## We want new behaviour
### `test_install_distribution_union_with_versions`
```python
def test_install_distribution_union_with_versions(script, data):
to_install_001 = data.packages.joinpath("LocalExtras")
to_install_002 = data.packages.joinpath("LocalExtras-0.0.2")
result = script.pip_install_local(
to_install_001 + "[bar]", to_install_002 + "[baz]")
assert ("Successfully installed LocalExtras-0.0.1 simple-3.0 " +
"singlemodule-0.0.1" in result.stdout)
```
I don’t think this is *supposed* to work like this?
### `test_yaml_based[install/conflicting_triangle]`
The is basically testing whether an already-installed package would reject an incoming dependency. The discussion in [#7744](https://github.com/pypa/pip/issues/7744) seems to favour delaying this until we can provide some sort of “dependency override” functionalities so workflows won’t be blocked by the significantly stricter approach.
### `test_constraints_constrain_to_local`
### `test_constraints_constrain_to_local_editable`
### `test_constrained_to_url_install_same_url`
### `test_install_with_extras_editable_joined`
Contrains a package to a URL/editable. The resolution to this feature is delayed until we can get the resolver out of beta and collect feedback on how useful the feature would be. https://github.com/pypa/pip/issues/8253
### `test_install_with_extras_from_constraints`
### `test_install_with_extras_joined`
### `test_install_distribution_union_with_constraints`
Constraints select extras. We're splitting this out of the constraints feature, and *maybe* bring this back under something else. https://github.com/pypa/pip/issues/8211
----
## FIXED ✨✨✨
### `test_nonexistent_extra_warns_user_no_wheel`
### `test_nonexistent_extra_warns_user_with_wheel`
### `test_nonexistent_options_listed_in_order`
Legacy: `simple 3.0 does not provide the extra 'nonexistent'`
New: `WARNING: Invalid extras specified in simple[nonexistent]: nonexistent`
**Fixed by:** [`message_fixes`](https://github.com/pypa/pip/pull/8243)
### `test_install_pip_does_not_modify_pip_when_satisfied[upgrade=False]`
We implement the “Requirement already satisfied” message differently. The legacy resolver outputs:
```
Requirement already satisfied: pip in <site-packages>
```
The new resolver outputs:
```
Requirement already satisfied: pip==<version> in <site-packages>
```
**Fixed by:** [`message_fixes`](https://github.com/pypa/pip/pull/8243)
### `test_constraints_not_installed_by_default`
I think empty specifier should just be allowed. It’s useless, but why not.
**Fixed by:** [`constraint_fixes`](https://github.com/pypa/pip/pull/8239)
### `test_pep518_with_extra_and_markers`
We didn’t implemented any marker support.
**Fixed by:** [`markers-in-deps`](https://github.com/pypa/pip/pull/8230)
### `test_options_from_env_vars`
### `test_options_from_venv_config`
### `test_env_vars_override_config_file`
These three are failing at the same check.
Legacy: `DistributionNotFound: No matching distribution found for INITools`
New: `InstallationError: No matching distribution found for initools`
**Fixed by:** [`different-messages`](https://github.com/pypa/pip/pull/8298)
### `test_install_extra_merging`
### `test_install_local_editable_with_extras`
### `test_new_resolver_extra_merge_in_package[_local_with_setup]`
```python
def _local_with_setup(script, name, version, requires, extras):
"""Create the package as a local source directory to install from path.
"""
return create_test_package_with_setup(
script,
name=name,
version=version,
install_requires=requires,
extras_require=extras,
)
```
This is failing because we are not able to find `pkg==<version>` if `pkg` is provided by a direct URL (including a local directory). This will probably be fixed with the `find_matches()` rewrite.
```
ERROR: Could not find a version that satisfies the requirement pkg==1.0.0 (from pkg[dev])
ERROR: No matching distribution found for pkg
```
where `pkg[dev]` is actually provided by `path/to/pkg[dev]`
**Fixed by:** [`direct-url-with-extras`](https://github.com/pypa/pip/pull/8291)
### `test_pip_wheel_build_cache`
### `test_install_git_sha_cached`
**Fixed by:** [`new-resolver-wheel-cache`](https://github.com/pypa/pip/pull/8066)
### `test_install_nonlocal_compatible_wheel`
It seems like the finder is returning wheels not actually compatible with the host environment. Someone needs to filter them out and not returning them in `find_matches()`.
**Fixed by:** [`new-resolver-candidate-order`](https://github.com/pypa/pip/pull/8267)
### `test_double_install_fail`
Tests `pip install pip==* pip==7.1.2` fails. I think the new resolver should just skip this test.
**Fixed by:** [`nr_double_install`](https://github.com/pypa/pip/pull/8306)
### `test_download_prefer_binary_when_tarball_higher_than_wheel`
### `test_ignore_yanked_file`
Failing since [#8234](https://github.com/pypa/pip/pull/8234) (Split find_matches into generation and sorting)
**Fixed by:** [`fix-prefer-binary-reqfile-new-resolver`](https://github.com/pypa/pip/pull/8319)
### `test_install_editable_with_wrong_egg_name`
We are not going to allow this anymore. The new resolver throws a hard error if the provided name does not match the built name.
**Fixed by:** [`nr_wrong_egg_name`](https://github.com/pypa/pip/pull/8390)
### `test_install_unsupported_wheel_link_with_marker`
We are not emitting a warning on non-matching marker?
```
assert 'Ignoring asdf: markers \'sys_platform == "xyz"\' don\'t match your environment' in ''
```
**Fixed by**: [`unsupported-link-with-marker`](https://github.com/pypa/pip/pull/8374)
### `test_install_vcs_non_editable_direct_url`
`direct_url.json` is not generated correctly here, because we are not passing `original_url` on correctly.
**Fixed by:** [`direct-url-fix`](https://github.com/pypa/pip/pull/8378)