Bug 1379298 - Relax __eq__ for empty OptionValue; r?rillian
The rigid type comparison added in 51a92a22d6d1 (
bug 1375231) was
too rigid. This broke at least one consumer that was comparing an
empty PositiveOptionValue/NegativeOptionValue against a string.
Since comparisons against empty OptionValue are convenient and
don't violate the spirit of the type checking previously added,
this commit relaxes the type equivalence check in cases where the
OptionValue is empty.
MozReview-Commit-ID: UllTrzCjj
--- a/python/mozbuild/mozbuild/configure/options.py
+++ b/python/mozbuild/mozbuild/configure/options.py
@@ -51,21 +51,24 @@ class OptionValue(tuple):
return option
elif self and not len(self):
return '%s=1' % option
return '%s=%s' % (option, ','.join(self))
def __eq__(self, other):
# This is to catch naive comparisons against strings and other
# types in moz.configure files, as it is really easy to write
- # value == 'foo'.
- if not isinstance(other, tuple):
- raise TypeError('cannot compare a %s against an %s; OptionValue '
- 'instances are tuples - did you mean to compare '
- 'against member elements using [x]?' % (
+ # value == 'foo'. We only raise a TypeError for instances that
+ # have content, because value-less instances (like PositiveOptionValue
+ # and NegativeOptionValue) are common and it is trivial to
+ # compare these.
+ if not isinstance(other, tuple) and len(self):
+ raise TypeError('cannot compare a populated %s against an %s; '
+ 'OptionValue instances are tuples - did you mean to '
+ 'compare against member elements using [x]?' % (
type(other).__name__, type(self).__name__))
# Allow explicit tuples to be compared.
if type(other) == tuple:
return tuple.__eq__(self, other)
# Else we're likely an OptionValue class.
elif type(other) != type(self):
return False
--- a/python/mozbuild/mozbuild/test/configure/test_options.py
+++ b/python/mozbuild/mozbuild/test/configure/test_options.py
@@ -292,16 +292,30 @@ class TestOption(unittest.TestCase):
# Different OptionValue types are never equal.
self.assertNotEqual(val, OptionValue(('foo',)))
# For usability reasons, we raise TypeError when attempting to compare
# against a non-tuple.
with self.assertRaisesRegexp(TypeError, 'cannot compare a'):
val == 'foo'
+ # But we allow empty option values to compare otherwise we can't
+ # easily compare value-less types like PositiveOptionValue and
+ # NegativeOptionValue.
+ empty_positive = PositiveOptionValue()
+ empty_negative = NegativeOptionValue()
+ self.assertEqual(empty_positive, ())
+ self.assertEqual(empty_positive, PositiveOptionValue())
+ self.assertEqual(empty_negative, ())
+ self.assertEqual(empty_negative, NegativeOptionValue())
+ self.assertNotEqual(empty_positive, 'foo')
+ self.assertNotEqual(empty_positive, ('foo',))
+ self.assertNotEqual(empty_negative, 'foo')
+ self.assertNotEqual(empty_negative, ('foo',))
+
def test_option_value_format(self):
val = PositiveOptionValue()
self.assertEquals('--with-value', val.format('--with-value'))
self.assertEquals('--with-value', val.format('--without-value'))
self.assertEquals('--enable-value', val.format('--enable-value'))
self.assertEquals('--enable-value', val.format('--disable-value'))
self.assertEquals('--value', val.format('--value'))
self.assertEquals('VALUE=1', val.format('VALUE'))