Merge lp:~gz/brz/py3_static_tuple_mismatched_types into lp:brz

Proposed by Martin Packman on 2017-06-27
Status: Merged
Approved by: Martin Packman on 2017-06-27
Approved revision: 6724
Merge reported by: The Breezy Bot
Merged at revision: not available
Proposed branch: lp:~gz/brz/py3_static_tuple_mismatched_types
Merge into: lp:brz
Prerequisite: lp:~gz/brz/py3_static_tuple_import
Diff against target: 163 lines (+47/-28)
2 files modified
breezy/_static_tuple_c.c (+15/-7)
breezy/tests/test__static_tuple.py (+32/-21)
To merge this branch: bzr merge lp:~gz/brz/py3_static_tuple_mismatched_types
Reviewer Review Type Date Requested Status
Jelmer Vernooij 2017-06-27 Approve on 2017-06-27
Review via email: mp+326330@code.launchpad.net

Commit message

Make StaticTuple tests pass on Python 3

Description of the change

Fix remaining StaticTuple test failures on Python 3.

All related to the change in semantics of lt/gt comparisons with mismatched types. I'm hoping we don't actually have core logic trying to sort tuples with different contents anywhere.

To post a comment you must log in.
Jelmer Vernooij (jelmer) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'breezy/_static_tuple_c.c'
2--- breezy/_static_tuple_c.c 2017-06-25 23:37:57 +0000
3+++ breezy/_static_tuple_c.c 2017-06-27 01:23:29 +0000
4@@ -428,14 +428,22 @@
5 } else if (w == Py_None) {
6 // None is always less than the object
7 switch (op) {
8- case Py_NE:case Py_GT:case Py_GE:
9+ case Py_NE:
10+#if PY_MAJOR_VERSION >= 3
11+#else
12+ case Py_GT:case Py_GE:
13+#endif
14 Py_INCREF(Py_True);
15 return Py_True;
16- case Py_EQ:case Py_LT:case Py_LE:
17+ case Py_EQ:
18+#if PY_MAJOR_VERSION >= 3
19+#else
20+ case Py_LT:case Py_LE:
21+#endif
22 Py_INCREF(Py_False);
23 return Py_False;
24- default: // Should never happen
25- return Py_NotImplemented;
26+ default: // Should only happen on Python 3
27+ return Py_NotImplemented;
28 }
29 } else {
30 /* We don't special case this comparison, we just let python handle
31@@ -725,10 +733,10 @@
32 static PyObject *
33 StaticTuple_sizeof(StaticTuple *self)
34 {
35- Py_ssize_t res;
36+ Py_ssize_t res;
37
38- res = _PyObject_SIZE(&StaticTuple_Type) + (int)self->size * sizeof(void*);
39- return PyInt_FromSsize_t(res);
40+ res = _PyObject_SIZE(&StaticTuple_Type) + (int)self->size * sizeof(void*);
41+ return PyInt_FromSsize_t(res);
42 }
43
44
45
46=== modified file 'breezy/tests/test__static_tuple.py'
47--- breezy/tests/test__static_tuple.py 2017-06-22 00:41:56 +0000
48+++ breezy/tests/test__static_tuple.py 2017-06-27 01:23:29 +0000
49@@ -20,6 +20,7 @@
50 import cPickle as pickle
51 except ImportError:
52 import pickle
53+import operator
54 import sys
55
56 from breezy import (
57@@ -287,15 +288,24 @@
58 k6 = self.module.StaticTuple(k3, k4)
59 self.assertCompareEqual(k5, k6)
60
61- def assertCompareDifferent(self, k_small, k_big):
62+ def check_strict_compare(self, k1, k2, mismatched_types):
63+ """True if on Python 3 and stricter comparison semantics are used."""
64+ if PY3 and mismatched_types:
65+ for op in ("ge", "gt", "le", "lt"):
66+ self.assertRaises(TypeError, getattr(operator, op), k1, k2)
67+ return True
68+ return False
69+
70+ def assertCompareDifferent(self, k_small, k_big, mismatched_types=False):
71 self.assertFalse(k_small == k_big)
72- self.assertFalse(k_small >= k_big)
73- self.assertFalse(k_small > k_big)
74 self.assertTrue(k_small != k_big)
75- self.assertTrue(k_small <= k_big)
76- self.assertTrue(k_small < k_big)
77+ if not self.check_strict_compare(k_small, k_big, mismatched_types):
78+ self.assertFalse(k_small >= k_big)
79+ self.assertFalse(k_small > k_big)
80+ self.assertTrue(k_small <= k_big)
81+ self.assertTrue(k_small < k_big)
82
83- def assertCompareNoRelation(self, k1, k2):
84+ def assertCompareNoRelation(self, k1, k2, mismatched_types=False):
85 """Run the comparison operators, make sure they do something.
86
87 However, we don't actually care what comes first or second. This is
88@@ -304,20 +314,21 @@
89 """
90 self.assertFalse(k1 == k2)
91 self.assertTrue(k1 != k2)
92- # Do the comparison, but we don't care about the result
93- k1 >= k2
94- k1 > k2
95- k1 <= k2
96- k1 < k2
97+ if not self.check_strict_compare(k1, k2, mismatched_types):
98+ # Do the comparison, but we don't care about the result
99+ k1 >= k2
100+ k1 > k2
101+ k1 <= k2
102+ k1 < k2
103
104 def test_compare_vs_none(self):
105 k1 = self.module.StaticTuple('baz', 'bing')
106- self.assertCompareDifferent(None, k1)
107+ self.assertCompareDifferent(None, k1, mismatched_types=True)
108
109 def test_compare_cross_class(self):
110 k1 = self.module.StaticTuple('baz', 'bing')
111- self.assertCompareNoRelation(10, k1)
112- self.assertCompareNoRelation('baz', k1)
113+ self.assertCompareNoRelation(10, k1, mismatched_types=True)
114+ self.assertCompareNoRelation('baz', k1, mismatched_types=True)
115
116 def test_compare_all_different_same_width(self):
117 k1 = self.module.StaticTuple('baz', 'bing')
118@@ -344,8 +355,8 @@
119 k4 = self.module.StaticTuple(k1, k2)
120 self.assertCompareDifferent(k3, k4)
121 k5 = self.module.StaticTuple('foo', None)
122- self.assertCompareDifferent(k5, k1)
123- self.assertCompareDifferent(k5, k2)
124+ self.assertCompareDifferent(k5, k1, mismatched_types=True)
125+ self.assertCompareDifferent(k5, k2, mismatched_types=True)
126
127 def test_compare_diff_width(self):
128 k1 = self.module.StaticTuple('foo')
129@@ -359,13 +370,13 @@
130 k1 = self.module.StaticTuple('foo', 'bar')
131 k2 = self.module.StaticTuple('foo', 1, None, u'\xb5', 1.2, 2**65, True,
132 k1)
133- self.assertCompareNoRelation(k1, k2)
134+ self.assertCompareNoRelation(k1, k2, mismatched_types=True)
135 k3 = self.module.StaticTuple('foo')
136 self.assertCompareDifferent(k3, k1)
137 k4 = self.module.StaticTuple(None)
138- self.assertCompareDifferent(k4, k1)
139+ self.assertCompareDifferent(k4, k1, mismatched_types=True)
140 k5 = self.module.StaticTuple(1)
141- self.assertCompareNoRelation(k1, k5)
142+ self.assertCompareNoRelation(k1, k5, mismatched_types=True)
143
144 def test_compare_to_tuples(self):
145 k1 = self.module.StaticTuple('foo')
146@@ -381,7 +392,7 @@
147 self.assertCompareDifferent(('foo',), k2)
148 self.assertCompareDifferent(('foo', 'aaa'), k2)
149 self.assertCompareDifferent(('baz', 'bing'), k2)
150- self.assertCompareDifferent(('foo', 10), k2)
151+ self.assertCompareDifferent(('foo', 10), k2, mismatched_types=True)
152
153 k3 = self.module.StaticTuple(k1, k2)
154 self.assertCompareEqual(k3, (('foo',), ('foo', 'bar')))
155@@ -397,7 +408,7 @@
156 # This requires comparing a StaticTuple to a 'string', and then
157 # interpreting that value in the next higher StaticTuple. This used to
158 # generate a PyErr_BadIternalCall. We now fall back to *something*.
159- self.assertCompareNoRelation(k1, k2)
160+ self.assertCompareNoRelation(k1, k2, mismatched_types=True)
161
162 def test_hash(self):
163 k = self.module.StaticTuple('foo')

Subscribers

People subscribed via source and target branches