Bug 1379298 - Relax __eq__ for empty OptionValue; r=nalexander,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

--HG--
extra : rebase_source : 2c41428d1be667edecdab0947d006a1a6a533569
This commit is contained in:
Gregory Szorc 2017-07-10 11:21:37 -07:00
parent 3dd4e99bb1
commit e0c0104c60
2 changed files with 22 additions and 5 deletions

View File

@ -56,11 +56,14 @@ class OptionValue(tuple):
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.

View File

@ -297,6 +297,20 @@ class TestOption(unittest.TestCase):
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'))