Patchwork [3,of,4] test-manifest.py: separate out test for byte 22 of nodeid

login
register
mail settings
Submitter Martin von Zweigbergk
Date March 26, 2015, 5:31 p.m.
Message ID <4dd5ebf2778b1ec0f967.1427391060@martinvonz.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/8295/
State Superseded
Headers show

Comments

Martin von Zweigbergk - March 26, 2015, 5:31 p.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@google.com>
# Date 1427388141 25200
#      Thu Mar 26 09:42:21 2015 -0700
# Node ID 4dd5ebf2778b1ec0f967d28e1f752906360602ae
# Parent  c99862a283585e40e907b9116a7736de6a3f4369
test-manifest.py: separate out test for byte 22 of nodeid

When assigning a 22-byte hash to a nodeid in a manifest, manifestdict
drops the 22nd byte, while treemanifest keeps it. Let's keep this test
separate and make it pass whether the 22nd byte gets stored or
dropped.
Augie Fackler - March 26, 2015, 5:34 p.m.
On Thu, Mar 26, 2015 at 1:31 PM, Martin von Zweigbergk <
martinvonz@google.com> wrote:

> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1427388141 25200
> #      Thu Mar 26 09:42:21 2015 -0700
> # Node ID 4dd5ebf2778b1ec0f967d28e1f752906360602ae
> # Parent  c99862a283585e40e907b9116a7736de6a3f4369
> test-manifest.py: separate out test for byte 22 of nodeid
>

How much of a perf lose is it for treemanifest to trim the nodeid to no
more than 21 bytes? I think I'd rather see the behavior be consistent...


>
> When assigning a 22-byte hash to a nodeid in a manifest, manifestdict
> drops the 22nd byte, while treemanifest keeps it. Let's keep this test
> separate and make it pass whether the 22nd byte gets stored or
> dropped.
>
> diff -r c99862a28358 -r 4dd5ebf2778b tests/test-manifest.py
> --- a/tests/test-manifest.py    Wed Mar 25 14:13:46 2015 -0700
> +++ b/tests/test-manifest.py    Thu Mar 26 09:42:21 2015 -0700
> @@ -127,10 +127,6 @@
>          self.assertEqual([('bar/baz/qux.py', binascii.unhexlify(HASH_2)),
>                            ('foo', binascii.unhexlify(HASH_1) + 'a')],
>                           list(m.iteritems()))
> -        # Sometimes it even tries a 22-byte fake hash, but we can
> -        # return 21 and it'll work out
> -        m['foo'] = want + '+'
> -        self.assertEqual(want, m['foo'])
>          # make sure the suffix survives a copy
>          match = matchmod.match('', '', ['re:foo'])
>          m2 = m.matches(match)
> @@ -148,6 +144,14 @@
>          self.assertEqual({'foo': ((want, f), (h, ''))}, m.diff(clean))
>          self.assertEqual({'foo': ((h, ''), (want, f))}, clean.diff(m))
>
> +    def test22ByteNode(self):
> +        m = parsemanifest(A_SHORT_MANIFEST)
> +        h = binascii.unhexlify(HASH_3)
> +        # Sometimes it even tries a 22-byte fake hash, but we can
> +        # keep 21 and it'll work out
> +        m['foo'] = h + 'a+'
> +        self.assertIn(m['foo'], [h + 'a', h + 'a+'])
> +
>      def testMatchException(self):
>          m = parsemanifest(A_SHORT_MANIFEST)
>          match = matchmod.match('', '', ['re:.*'])
>
Martin von Zweigbergk - March 26, 2015, 5:37 p.m.
On Thu, Mar 26, 2015 at 10:34 AM Augie Fackler <augie@google.com> wrote:

> On Thu, Mar 26, 2015 at 1:31 PM, Martin von Zweigbergk <
> martinvonz@google.com> wrote:
>
>> # HG changeset patch
>> # User Martin von Zweigbergk <martinvonz@google.com>
>> # Date 1427388141 25200
>> #      Thu Mar 26 09:42:21 2015 -0700
>> # Node ID 4dd5ebf2778b1ec0f967d28e1f752906360602ae
>> # Parent  c99862a283585e40e907b9116a7736de6a3f4369
>> test-manifest.py: separate out test for byte 22 of nodeid
>>
>
> How much of a perf lose is it for treemanifest to trim the nodeid to no
> more than 21 bytes? I think I'd rather see the behavior be consistent...
>

Probably not even noticeably much. I'll send a V2.


>
>
>>
>> When assigning a 22-byte hash to a nodeid in a manifest, manifestdict
>> drops the 22nd byte, while treemanifest keeps it. Let's keep this test
>> separate and make it pass whether the 22nd byte gets stored or
>> dropped.
>>
>> diff -r c99862a28358 -r 4dd5ebf2778b tests/test-manifest.py
>> --- a/tests/test-manifest.py    Wed Mar 25 14:13:46 2015 -0700
>> +++ b/tests/test-manifest.py    Thu Mar 26 09:42:21 2015 -0700
>> @@ -127,10 +127,6 @@
>>          self.assertEqual([('bar/baz/qux.py', binascii.unhexlify(HASH_2)),
>>                            ('foo', binascii.unhexlify(HASH_1) + 'a')],
>>                           list(m.iteritems()))
>> -        # Sometimes it even tries a 22-byte fake hash, but we can
>> -        # return 21 and it'll work out
>> -        m['foo'] = want + '+'
>> -        self.assertEqual(want, m['foo'])
>>          # make sure the suffix survives a copy
>>          match = matchmod.match('', '', ['re:foo'])
>>          m2 = m.matches(match)
>> @@ -148,6 +144,14 @@
>>          self.assertEqual({'foo': ((want, f), (h, ''))}, m.diff(clean))
>>          self.assertEqual({'foo': ((h, ''), (want, f))}, clean.diff(m))
>>
>> +    def test22ByteNode(self):
>> +        m = parsemanifest(A_SHORT_MANIFEST)
>> +        h = binascii.unhexlify(HASH_3)
>> +        # Sometimes it even tries a 22-byte fake hash, but we can
>> +        # keep 21 and it'll work out
>> +        m['foo'] = h + 'a+'
>> +        self.assertIn(m['foo'], [h + 'a', h + 'a+'])
>> +
>>      def testMatchException(self):
>>          m = parsemanifest(A_SHORT_MANIFEST)
>>          match = matchmod.match('', '', ['re:.*'])
>>
>

Patch

diff -r c99862a28358 -r 4dd5ebf2778b tests/test-manifest.py
--- a/tests/test-manifest.py	Wed Mar 25 14:13:46 2015 -0700
+++ b/tests/test-manifest.py	Thu Mar 26 09:42:21 2015 -0700
@@ -127,10 +127,6 @@ 
         self.assertEqual([('bar/baz/qux.py', binascii.unhexlify(HASH_2)),
                           ('foo', binascii.unhexlify(HASH_1) + 'a')],
                          list(m.iteritems()))
-        # Sometimes it even tries a 22-byte fake hash, but we can
-        # return 21 and it'll work out
-        m['foo'] = want + '+'
-        self.assertEqual(want, m['foo'])
         # make sure the suffix survives a copy
         match = matchmod.match('', '', ['re:foo'])
         m2 = m.matches(match)
@@ -148,6 +144,14 @@ 
         self.assertEqual({'foo': ((want, f), (h, ''))}, m.diff(clean))
         self.assertEqual({'foo': ((h, ''), (want, f))}, clean.diff(m))
 
+    def test22ByteNode(self):
+        m = parsemanifest(A_SHORT_MANIFEST)
+        h = binascii.unhexlify(HASH_3)
+        # Sometimes it even tries a 22-byte fake hash, but we can
+        # keep 21 and it'll work out
+        m['foo'] = h + 'a+'
+        self.assertIn(m['foo'], [h + 'a', h + 'a+'])
+
     def testMatchException(self):
         m = parsemanifest(A_SHORT_MANIFEST)
         match = matchmod.match('', '', ['re:.*'])