Skip to content

Commit

Permalink
pr feedback: remove unneeded non-root case and fix tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@reesehyde
reesehyde committed Nov 13, 2024
1 parent fd1c1dc commit 4734c0f
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 148 deletions.
14 changes: 7 additions & 7 deletions src/poetry/puzzle/provider.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -555,13 +555,13 @@ def complete_package(
if dep.name in self.UNSAFE_PACKAGES:
continue

active_extras = (
self._active_root_extras if package.is_root() else dependency.extras
)
if self._env and not dep.marker.validate(
self._marker_values(active_extras)
):
continue
if self._env:
marker_values = (
self._marker_values(self._active_root_extras) if package.is_root()
else self._env.marker_env
)
if not dep.marker.validate(marker_values):
continue

if not package.is_root() and (
(dep.is_optional() and dep.name not in optional_dependencies)
Expand Down
30 changes: 18 additions & 12 deletions tests/installation/fixtures/with-dependencies-differing-extras.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,37 +4,43 @@ version = "1.0.0"
description = ""
optional = true
python-versions = "*"
files = []
files = [ ]

[package.extras]
demo-extra-one = ["transitive-dep-one"]
demo-extra-two = ["transitive-dep-two"]
[package.dependencies.transitive-dep-one]
version = "1.1.0"
optional = true
markers = 'extra == "demo-extra-one" and extra != "demo-extra-two"'

[package.dependencies]
transitive-dep-one = {version = "1.1.0", optional = true}
transitive-dep-two = {version = "1.2.0", optional = true}
[package.dependencies.transitive-dep-two]
version = "1.2.0"
optional = true
markers = 'extra != "demo-extra-one" and extra == "demo-extra-two"'

[package.extras]
demo-extra-one = [ "transitive-dep-one", "transitive-dep-two" ]
demo-extra-two = [ "transitive-dep-one", "transitive-dep-two" ]

[[package]]
name = "transitive-dep-one"
version = "1.1.0"
description = ""
optional = true
python-versions = "*"
files = []
files = [ ]

[[package]]
name = "transitive-dep-two"
version = "1.2.0"
description = ""
optional = true
python-versions = "*"
files = []
files = [ ]

[extras]
extra-one = ["demo", "demo"]
extra-two = ["demo", "demo"]
extra-one = [ "demo", "demo" ]
extra-two = [ "demo", "demo" ]

[metadata]
python-versions = "*"
lock-version = "2.0"
python-versions = "*"
content-hash = "123456789"
162 changes: 37 additions & 125 deletions tests/installation/test_installer.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1027,110 +1027,6 @@ def test_run_with_dependencies_nested_extras(
assert locker.written_data == expected


@pytest.mark.parametrize("locked", [False]) # TODO: lock data
@pytest.mark.parametrize("extra", [None, "extra-one", "extra-two"])
def test_run_with_conflicting_dependency_extras(
installer: Installer,
pool: RepositoryPool,
locker: Locker,
installed: CustomInstalledRepository,
repo: Repository,
config: Config,
package: ProjectPackage,
extra: str | None,
locked: bool,
) -> None:
"""https://github.com/python-poetry/poetry/issues/834
Tests resolution of extras in both root ('extra-one', 'extra-two') and transitive
('demo-extra-one', 'demo-extra-two') dependencies
"""
# Demo package with two optional transitive dependencies, one for each extra
demo_pkg = get_package("demo", "1.0.0")
transitive_dep_one = get_package("transitive-dep", "1.1.0")
transitive_dep_two = get_package("transitive-dep", "1.2.0")
demo_pkg.extras = {
canonicalize_name("demo-extra-one"): [
get_dependency("transitive-dep", constraint={"version": "1.1.0", "optional": True})
],
canonicalize_name("demo-extra-two"): [
get_dependency("transitive-dep", constraint={"version": "1.2.0", "optional": True})
],
}
demo_pkg.add_dependency(
Factory.create_dependency(
"transitive-dep",
{
"version": "1.1.0",
"markers": "extra == 'demo-extra-one' and extra != 'demo-extra-two'",
"optional": True,
}
)
)
demo_pkg.add_dependency(
Factory.create_dependency(
"transitive-dep",
{
"version": "1.2.0",
"markers": "extra != 'demo-extra-one' and extra == 'demo-extra-two'",
"optional": True,
}
)
)
repo.add_package(demo_pkg)
repo.add_package(transitive_dep_one)
repo.add_package(transitive_dep_two)

# 'demo' with extra 'demo-extra-one' when package has 'extra-one' extra
# and with extra 'demo-extra-two' when 'extra-two'
extra_one_dep = Factory.create_dependency(
"demo",
{
"version": "1.0.0",
"markers": "extra == 'extra-one' and extra != 'extra-two'",
"extras": ["demo-extra-one"],
"optional": True,
},
)
extra_two_dep = Factory.create_dependency(
"demo",
{
"version": "1.0.0",
"markers": "extra != 'extra-one' and extra == 'extra-two'",
"extras": ["demo-extra-two"],
"optional": True,
},
)
package.add_dependency(extra_one_dep)
package.add_dependency(extra_two_dep)
package.extras = {
canonicalize_name("extra-one"): [extra_one_dep],
canonicalize_name("extra-two"): [extra_two_dep],
}

locker.locked(locked)
if locked:
raise ValueError("no lock data for this test yet")
locker.mock_lock_data(dict(fixture("TODO")))

if extra is not None:
installer.extras([extra])
result = installer.run()
assert result == 0

if not locked:
raise ValueError("no lock data for this test yet")
expected = fixture("TODO")
assert locker.written_data == expected

# Results of installation are consistent with the 'extra' input
assert isinstance(installer.executor, Executor)
if extra is None:
assert len(installer.executor.installations) == 0
else:
assert len(installer.executor.installations) == 2


@pytest.mark.parametrize("locked", [True, False])
@pytest.mark.parametrize("extra", [None, "cpu", "cuda"])
def test_run_with_exclusive_extras_different_sources(
Expand Down Expand Up @@ -1331,30 +1227,42 @@ def test_run_with_different_dependency_extras(
extra: str | None,
locked: bool,
) -> None:
"""https://github.com/python-poetry/poetry/issues/834"""
# Demo package with two optional transitive dependencies, one for each extra
"""https://github.com/python-poetry/poetry/issues/834
This tests different sets of extras in a dependency of the root project. These different dependency extras are
themselves conditioned on extras in the root project.
"""
# Three packages in additon to root: demo (direct dependency) and two transitive dep packages
demo_pkg = get_package("demo", "1.0.0")
transitive_dep_one = get_package("transitive-dep-one", "1.1.0")
transitive_dep_two = get_package("transitive-dep-two", "1.2.0")
demo_pkg.extras = {
canonicalize_name("demo-extra-one"): [get_dependency("transitive-dep-one")],
canonicalize_name("demo-extra-two"): [get_dependency("transitive-dep-two")],
}
demo_pkg.add_dependency(
Factory.create_dependency(
"transitive-dep-one", {"version": "1.1.0", "optional": True}
)
transitive_one_pkg = get_package("transitive-dep-one", "1.1.0")
transitive_two_pkg = get_package("transitive-dep-two", "1.2.0")

# Switch each transitive dependency based on extra markers in the 'demo' package
transitive_dep_one = Factory.create_dependency(
"transitive-dep-one",
{
"version": "1.1.0",
"markers": "extra == 'demo-extra-one' and extra != 'demo-extra-two'",
"optional": True,
}
)
demo_pkg.add_dependency(
Factory.create_dependency(
"transitive-dep-two", {"version": "1.2.0", "optional": True}
)
transitive_dep_two = Factory.create_dependency(
"transitive-dep-two",
{
"version": "1.2.0",
"markers": "extra != 'demo-extra-one' and extra == 'demo-extra-two'",
"optional": True,
}
)
repo.add_package(demo_pkg)
repo.add_package(transitive_dep_one)
repo.add_package(transitive_dep_two)
# Include both packages in both demo extras, to validate that they're filtered out based on extra markers alone
demo_pkg.extras = {
canonicalize_name("demo-extra-one"): [get_dependency("transitive-dep-one"), get_dependency("transitive-dep-two")],
canonicalize_name("demo-extra-two"): [get_dependency("transitive-dep-one"), get_dependency("transitive-dep-two")],
}
demo_pkg.add_dependency(transitive_dep_one)
demo_pkg.add_dependency(transitive_dep_two)

# 'demo' with extra 'one' when package has 'extra-one' extra and with extra 'two' when 'extra-two'
# Now define the demo dependency, similarly switched on extra markers in the root package
extra_one_dep = Factory.create_dependency(
"demo",
{
Expand All @@ -1373,12 +1281,16 @@ def test_run_with_different_dependency_extras(
)
package.add_dependency(extra_one_dep)
package.add_dependency(extra_two_dep)
# We don't want to cheat by only including the correct dependency in the 'extra' mapping
# Again we don't want to cheat by only including the correct dependency in the 'extra' mapping
package.extras = {
canonicalize_name("extra-one"): [extra_one_dep, extra_two_dep],
canonicalize_name("extra-two"): [extra_one_dep, extra_two_dep],
}

repo.add_package(demo_pkg)
repo.add_package(transitive_one_pkg)
repo.add_package(transitive_two_pkg)

locker.locked(locked)
if locked:
locker.mock_lock_data(dict(fixture("with-dependencies-differing-extras")))
Expand Down
7 changes: 3 additions & 4 deletions tests/puzzle/test_solver.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4718,9 +4718,8 @@ def test_solver_resolves_duplicate_dependency_in_root_extra(
"""
Without extras, a newer version of A can be chosen than with root extras.
"""
constraint: dict[str, Any] = {"version": "*"}
if with_extra:
constraint["extras"] = ["foo"]
extra = ["foo"] if with_extra else []

package_a1 = get_package("A", "1.0")
package_a2 = get_package("A", "2.0")

Expand All @@ -4735,7 +4734,7 @@ def test_solver_resolves_duplicate_dependency_in_root_extra(
repo.add_package(package_a1)
repo.add_package(package_a2)

solver = Solver(package, pool, [], [], io)
solver = Solver(package, pool, [], [], io, active_root_extras=extra)
transaction = solver.solve()

check_solver_result(
Expand Down

0 comments on commit 4734c0f

Please sign in to comment.