Skip to content

Commit e2f14b7

Browse files
radoeringradoering
committed
constraints: fix allows, allows_any and allows_all for generic constraints and increase test coverage
1 parent 1459c57 commit e2f14b7

File tree

6 files changed

+352
-156
lines changed

6 files changed

+352
-156
lines changed

‎src/poetry/core/constraints/generic/constraint.py‎

Lines changed: 47 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -43,44 +43,72 @@ def operator(self) -> str:
4343
return self._operator
4444

4545
def allows(self, other: BaseConstraint) -> bool:
46-
if not isinstance(other, Constraint):
47-
raise ValueError("Unimplemented comparison of constraints")
46+
if not isinstance(other, Constraint) or other.operator != "==":
47+
raise ValueError(
48+
f"Invalid argument for allows"
49+
f' ("other" must be a constraint with operator "=="): {other}'
50+
)
4851

4952
is_equal_op = self._operator == "=="
5053
is_non_equal_op = self._operator == "!="
51-
is_other_equal_op = other.operator == "=="
52-
is_other_non_equal_op = other.operator == "!="
5354

54-
if is_equal_op and is_other_equal_op:
55+
if is_equal_op:
5556
return self._value == other.value
5657

57-
if (
58-
is_equal_op
59-
and is_other_non_equal_op
60-
or is_non_equal_op
61-
and is_other_equal_op
62-
or is_non_equal_op
63-
and is_other_non_equal_op
64-
):
58+
if is_non_equal_op:
6559
return self._value != other.value
6660

6761
return False
6862

6963
def allows_all(self, other: BaseConstraint) -> bool:
70-
if not isinstance(other, Constraint):
71-
return other.is_empty()
64+
from poetry.core.constraints.generic import MultiConstraint
65+
from poetry.core.constraints.generic import UnionConstraint
66+
67+
if isinstance(other, Constraint):
68+
if other.operator == "==":
69+
return self.allows(other)
70+
71+
return self == other
7272

73-
return other == self
73+
if isinstance(other, MultiConstraint):
74+
return any(self.allows_all(c) for c in other.constraints)
75+
76+
if isinstance(other, UnionConstraint):
77+
return all(self.allows_all(c) for c in other.constraints)
78+
79+
return other.is_empty()
7480

7581
def allows_any(self, other: BaseConstraint) -> bool:
82+
from poetry.core.constraints.generic import MultiConstraint
83+
from poetry.core.constraints.generic import UnionConstraint
84+
85+
is_equal_op = self._operator == "=="
86+
is_non_equal_op = self._operator == "!="
87+
88+
if is_equal_op:
89+
return other.allows(self)
90+
7691
if isinstance(other, Constraint):
77-
is_non_equal_op = self._operator == "!="
92+
is_other_equal_op = other.operator == "=="
7893
is_other_non_equal_op = other.operator == "!="
7994

80-
if is_non_equal_op and is_other_non_equal_op:
95+
if is_other_equal_op:
96+
return self.allows(other)
97+
98+
if is_equal_op and is_other_non_equal_op:
8199
return self._value != other.value
82100

83-
return other.allows(self)
101+
return is_non_equal_op and is_other_non_equal_op
102+
103+
elif isinstance(other, MultiConstraint):
104+
return is_non_equal_op
105+
106+
elif isinstance(other, UnionConstraint):
107+
return is_non_equal_op and any(
108+
self.allows_any(c) for c in other.constraints
109+
)
110+
111+
return other.is_any()
84112

85113
def invert(self) -> Constraint:
86114
return Constraint(self._value, "!=" if self._operator == "==" else "==")

‎src/poetry/core/constraints/generic/multi_constraint.py‎

Lines changed: 12 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -29,44 +29,27 @@ def allows(self, other: BaseConstraint) -> bool:
2929
return all(constraint.allows(other) for constraint in self._constraints)
3030

3131
def allows_all(self, other: BaseConstraint) -> bool:
32-
if other.is_any():
33-
return False
34-
35-
if other.is_empty():
36-
return True
37-
38-
if not isinstance(other, MultiConstraint):
39-
return self.allows(other)
40-
41-
our_constraints = iter(self._constraints)
42-
their_constraints = iter(other.constraints)
43-
our_constraint = next(our_constraints, None)
44-
their_constraint = next(their_constraints, None)
45-
46-
while our_constraint and their_constraint:
47-
if our_constraint.allows_all(their_constraint):
48-
their_constraint = next(their_constraints, None)
49-
else:
50-
our_constraint = next(our_constraints, None)
32+
if isinstance(other, MultiConstraint):
33+
return all(c in other.constraints for c in self._constraints)
5134

52-
return their_constraint is None
35+
return all(c.allows_all(other) for c in self._constraints)
5336

5437
def allows_any(self, other: BaseConstraint) -> bool:
55-
if other.is_any():
56-
return True
57-
58-
if other.is_empty():
59-
return True
38+
from poetry.core.constraints.generic import UnionConstraint
6039

6140
if isinstance(other, Constraint):
62-
return self.allows(other)
41+
if other.operator == "==":
42+
return self.allows(other)
6343

64-
if isinstance(other, MultiConstraint):
44+
return other.operator == "!="
45+
46+
if isinstance(other, UnionConstraint):
6547
return any(
66-
c1.allows(c2) for c1 in self.constraints for c2 in other.constraints
48+
all(c1.allows_any(c2) for c1 in self.constraints)
49+
for c2 in other.constraints
6750
)
6851

69-
return False
52+
return isinstance(other, MultiConstraint) or other.is_any()
7053

7154
def invert(self) -> UnionConstraint:
7255
from poetry.core.constraints.generic import UnionConstraint

‎src/poetry/core/constraints/generic/union_constraint.py‎

Lines changed: 13 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -24,47 +24,23 @@ def allows(
2424
return any(constraint.allows(other) for constraint in self._constraints)
2525

2626
def allows_any(self, other: BaseConstraint) -> bool:
27-
if other.is_empty():
28-
return False
29-
30-
if other.is_any():
31-
return True
32-
33-
if isinstance(other, (UnionConstraint, MultiConstraint)):
34-
constraints = other.constraints
35-
else:
36-
constraints = (other,)
27+
if isinstance(other, UnionConstraint):
28+
return any(
29+
c1.allows_any(c2)
30+
for c1 in self._constraints
31+
for c2 in other.constraints
32+
)
3733

38-
return any(
39-
our_constraint.allows_any(their_constraint)
40-
for our_constraint in self._constraints
41-
for their_constraint in constraints
42-
)
34+
return any(c.allows_any(other) for c in self._constraints)
4335

4436
def allows_all(self, other: BaseConstraint) -> bool:
45-
if other.is_any():
46-
return False
47-
48-
if other.is_empty():
49-
return True
50-
51-
if isinstance(other, (UnionConstraint, MultiConstraint)):
52-
constraints = other.constraints
53-
else:
54-
constraints = (other,)
55-
56-
our_constraints = iter(self._constraints)
57-
their_constraints = iter(constraints)
58-
our_constraint = next(our_constraints, None)
59-
their_constraint = next(their_constraints, None)
60-
61-
while our_constraint and their_constraint:
62-
if our_constraint.allows_all(their_constraint):
63-
their_constraint = next(their_constraints, None)
64-
else:
65-
our_constraint = next(our_constraints, None)
37+
if isinstance(other, UnionConstraint):
38+
return all(
39+
any(c1.allows_all(c2) for c1 in self._constraints)
40+
for c2 in other.constraints
41+
)
6642

67-
return their_constraint is None
43+
return any(c.allows_all(other) for c in self._constraints)
6844

6945
def invert(self) -> MultiConstraint:
7046
inverted_constraints = [c.invert() for c in self._constraints]

‎tests/constraints/generic/test_constraint.py‎

Lines changed: 110 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -15,41 +15,118 @@
1515
from poetry.core.constraints.generic import BaseConstraint
1616

1717

18-
def test_allows() -> None:
19-
c = Constraint("win32")
20-
21-
assert c.allows(Constraint("win32"))
22-
assert not c.allows(Constraint("linux"))
23-
24-
c = Constraint("win32", "!=")
25-
26-
assert not c.allows(Constraint("win32"))
27-
assert c.allows(Constraint("linux"))
28-
29-
30-
def test_allows_any() -> None:
31-
c = Constraint("win32")
32-
33-
assert c.allows_any(Constraint("win32"))
34-
assert not c.allows_any(Constraint("linux"))
35-
assert c.allows_any(UnionConstraint(Constraint("win32"), Constraint("linux")))
36-
assert c.allows_any(Constraint("linux", "!="))
37-
38-
c = Constraint("win32", "!=")
39-
40-
assert not c.allows_any(Constraint("win32"))
41-
assert c.allows_any(Constraint("linux"))
42-
assert c.allows_any(UnionConstraint(Constraint("win32"), Constraint("linux")))
43-
assert c.allows_any(Constraint("linux", "!="))
18+
@pytest.mark.parametrize(
19+
("constraint1", "constraint2", "expected"),
20+
[
21+
(Constraint("win32"), Constraint("win32"), True),
22+
(Constraint("win32"), Constraint("linux"), False),
23+
(Constraint("win32", "!="), Constraint("win32"), False),
24+
(Constraint("win32", "!="), Constraint("linux"), True),
25+
],
26+
)
27+
def test_allows(
28+
constraint1: Constraint, constraint2: Constraint, expected: bool
29+
) -> None:
30+
assert constraint1.allows(constraint2) is expected
4431

4532

46-
def test_allows_all() -> None:
47-
c = Constraint("win32")
48-
49-
assert c.allows_all(Constraint("win32"))
50-
assert not c.allows_all(Constraint("linux"))
51-
assert not c.allows_all(Constraint("linux", "!="))
52-
assert not c.allows_all(UnionConstraint(Constraint("win32"), Constraint("linux")))
33+
@pytest.mark.parametrize(
34+
("constraint1", "constraint2", "expected_any", "expected_all"),
35+
[
36+
(Constraint("win32"), EmptyConstraint(), False, True),
37+
(Constraint("win32"), AnyConstraint(), True, False),
38+
(Constraint("win32"), Constraint("win32"), True, True),
39+
(Constraint("win32"), Constraint("linux"), False, False),
40+
(Constraint("win32"), Constraint("win32", "!="), False, False),
41+
(Constraint("win32"), Constraint("linux", "!="), True, False),
42+
(
43+
Constraint("win32"),
44+
UnionConstraint(Constraint("win32"), Constraint("linux")),
45+
True,
46+
False,
47+
),
48+
(
49+
Constraint("win32"),
50+
UnionConstraint(Constraint("darwin"), Constraint("linux")),
51+
False,
52+
False,
53+
),
54+
(
55+
Constraint("win32"),
56+
UnionConstraint(Constraint("win32", "!="), Constraint("linux", "!=")),
57+
True,
58+
False,
59+
),
60+
(
61+
Constraint("win32"),
62+
UnionConstraint(Constraint("darwin", "!="), Constraint("linux", "!=")),
63+
True,
64+
False,
65+
),
66+
(
67+
Constraint("win32"),
68+
MultiConstraint(Constraint("win32", "!="), Constraint("linux", "!=")),
69+
False,
70+
False,
71+
),
72+
(
73+
Constraint("win32"),
74+
MultiConstraint(Constraint("darwin", "!="), Constraint("linux", "!=")),
75+
True,
76+
False,
77+
),
78+
(Constraint("win32", "!="), EmptyConstraint(), False, True),
79+
(Constraint("win32", "!="), AnyConstraint(), True, False),
80+
(Constraint("win32", "!="), Constraint("win32"), False, False),
81+
(Constraint("win32", "!="), Constraint("linux"), True, True),
82+
(Constraint("win32", "!="), Constraint("win32", "!="), True, True),
83+
(Constraint("win32", "!="), Constraint("linux", "!="), True, False),
84+
(
85+
Constraint("win32", "!="),
86+
UnionConstraint(Constraint("win32"), Constraint("linux")),
87+
True,
88+
False,
89+
),
90+
(
91+
Constraint("win32", "!="),
92+
UnionConstraint(Constraint("darwin"), Constraint("linux")),
93+
True,
94+
True,
95+
),
96+
(
97+
Constraint("win32", "!="),
98+
UnionConstraint(Constraint("win32", "!="), Constraint("linux", "!=")),
99+
True,
100+
False,
101+
),
102+
(
103+
Constraint("win32", "!="),
104+
UnionConstraint(Constraint("darwin", "!="), Constraint("linux", "!=")),
105+
True,
106+
False,
107+
),
108+
(
109+
Constraint("win32", "!="),
110+
MultiConstraint(Constraint("win32", "!="), Constraint("linux", "!=")),
111+
True,
112+
True,
113+
),
114+
(
115+
Constraint("win32", "!="),
116+
MultiConstraint(Constraint("darwin", "!="), Constraint("linux", "!=")),
117+
True,
118+
False,
119+
),
120+
],
121+
)
122+
def test_allows_any_and_allows_all(
123+
constraint1: Constraint,
124+
constraint2: BaseConstraint,
125+
expected_any: bool,
126+
expected_all: bool,
127+
) -> None:
128+
assert constraint1.allows_any(constraint2) is expected_any
129+
assert constraint1.allows_all(constraint2) is expected_all
53130

54131

55132
@pytest.mark.parametrize(

0 commit comments

Comments
 (0)