Patchwork [7,of,7] tests: add a test case verifying that mq respects --no-git option

login
register
mail settings
Submitter Alexander Fomin
Date March 21, 2017, 5:08 p.m.
Message ID <9a11a79f6bcdd1134484.1490116120@devvm2125.lla2.facebook.com>
Download mbox | patch
Permalink /patch/19534/
State Changes Requested
Headers show

Comments

Alexander Fomin - March 21, 2017, 5:08 p.m.
# HG changeset patch
# User Alexander Fomin <afomin@fb.com>
# Date 1490113938 25200
#      Tue Mar 21 09:32:18 2017 -0700
# Node ID 9a11a79f6bcdd1134484ddd8eace997b55e7073a
# Parent  e9044ade1523e847877f4eee1d4e06734e2aa4cd
tests: add a test case verifying that mq respects --no-git option

This patch adds a test case to verify that --no-git option still works in mq
after making it explicitly request binary diff even in Git mode (issue5510).
Ryan McElroy - March 21, 2017, 8:30 p.m.
Overall this series looks good to me, except for this last patch. See 
inline comments. For now, I'd take the rest of this series if we're okay 
with the BC break, and just drop this patch while we figure out the mq 
stuff.

On 3/21/17 5:08 PM, Alexander Fomin wrote:
> # HG changeset patch
> # User Alexander Fomin<afomin@fb.com>
> # Date 1490113938 25200
> #      Tue Mar 21 09:32:18 2017 -0700
> # Node ID 9a11a79f6bcdd1134484ddd8eace997b55e7073a
> # Parent  e9044ade1523e847877f4eee1d4e06734e2aa4cd
> tests: add a test case verifying that mq respects --no-git option
>
> This patch adds a test case to verify that --no-git option still works in mq
> after making it explicitly request binary diff even in Git mode (issue5510).
>
> diff --git a/tests/test-mq.t b/tests/test-mq.t
> --- a/tests/test-mq.t
> +++ b/tests/test-mq.t
> @@ -1162,6 +1162,21 @@ check binary patches can be popped and p
>     8ba2a2f3e77b55d03051ff9c24ad65e7  bucephalus
>   
>   
> +check binary patches respect --no-git
> +
> +  $ cat > writebin.py <<EOF
> +  > import sys
> +  > path = sys.argv[1]
> +  > open(path, 'wb').write('BIN\x42RY')

Hex 42 is the character 'B', isn't it? So this isn't binary at all.

Also, binary detection just searches for \x00 I think. So that would be 
more appropriate to use here.

> +  > EOF
> +  $ python writebin.py answer

Rather than creating a little python program, I think you could just use 
printf here:

   $ printf 'BIN\x00RY' > answer

> +
> +  $ python "$TESTDIR/md5sum.py" answer
> +  ce0b4fda508e3d9f9ece98f8e823b6f7  answer

What is the reason for the md5sum here? Did you want to check 
round-tripping? (but I don't see that here)

> +  $ hg add answer
> +  $ hg qnew -f --no-git addanswer

What does --no-git do before this patch series? I don't see any 
differences in patch files with or without --no-git today, so I'm not 
sure it's actually respected today.

> +  $ grep diff .hg/patches/addanswer
> +  diff -r [a-f0-9]* -r [a-f0-9]* answer (re)
>   
>   strip again
>   
>
>    
>
Ryan McElroy - March 22, 2017, 10:28 a.m.
FYI, there's a discussion going on about the "correct" behavior on the 
issue tracker: https://bz.mercurial-scm.org/show_bug.cgi?id=5510


On 3/21/17 8:30 PM, Ryan McElroy wrote:
> Overall this series looks good to me, except for this last patch. See 
> inline comments. For now, I'd take the rest of this series if we're 
> okay with the BC break, and just drop this patch while we figure out 
> the mq stuff.
>
> On 3/21/17 5:08 PM, Alexander Fomin wrote:
>> # HG changeset patch
>> # User Alexander Fomin<afomin@fb.com>
>> # Date 1490113938 25200
>> #      Tue Mar 21 09:32:18 2017 -0700
>> # Node ID 9a11a79f6bcdd1134484ddd8eace997b55e7073a
>> # Parent  e9044ade1523e847877f4eee1d4e06734e2aa4cd
>> tests: add a test case verifying that mq respects --no-git option
>>
>> This patch adds a test case to verify that --no-git option still works in mq
>> after making it explicitly request binary diff even in Git mode (issue5510).
>>
>> diff --git a/tests/test-mq.t b/tests/test-mq.t
>> --- a/tests/test-mq.t
>> +++ b/tests/test-mq.t
>> @@ -1162,6 +1162,21 @@ check binary patches can be popped and p
>>     8ba2a2f3e77b55d03051ff9c24ad65e7  bucephalus
>>   
>>   
>> +check binary patches respect --no-git
>> +
>> +  $ cat > writebin.py <<EOF
>> +  > import sys
>> +  > path = sys.argv[1]
>> +  > open(path, 'wb').write('BIN\x42RY')
>
> Hex 42 is the character 'B', isn't it? So this isn't binary at all.
>
> Also, binary detection just searches for \x00 I think. So that would 
> be more appropriate to use here.
>
>> +  > EOF
>> +  $ python writebin.py answer
>
> Rather than creating a little python program, I think you could just 
> use printf here:
>
>   $ printf 'BIN\x00RY' > answer
>
>> +
>> +  $ python "$TESTDIR/md5sum.py" answer
>> +  ce0b4fda508e3d9f9ece98f8e823b6f7  answer
>
> What is the reason for the md5sum here? Did you want to check 
> round-tripping? (but I don't see that here)
>
>> +  $ hg add answer
>> +  $ hg qnew -f --no-git addanswer
>
> What does --no-git do before this patch series? I don't see any 
> differences in patch files with or without --no-git today, so I'm not 
> sure it's actually respected today.
>
>> +  $ grep diff .hg/patches/addanswer
>> +  diff -r [a-f0-9]* -r [a-f0-9]* answer (re)
>>   
>>   strip again
>>   
>>
>>    
>>
>
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Jw8rundaE7TbmqBYd1txIQ&m=2gvSPeBA6Q2eteGH5pR0M0qoiyQsjRbkUa0BkXzQ2C4&s=eYuPnseRo71oYyYXfcJgzwAbECl69gbMV2QU6VUmAVw&e=

Patch

diff --git a/tests/test-mq.t b/tests/test-mq.t
--- a/tests/test-mq.t
+++ b/tests/test-mq.t
@@ -1162,6 +1162,21 @@  check binary patches can be popped and p
   8ba2a2f3e77b55d03051ff9c24ad65e7  bucephalus
 
 
+check binary patches respect --no-git
+
+  $ cat > writebin.py <<EOF
+  > import sys
+  > path = sys.argv[1]
+  > open(path, 'wb').write('BIN\x42RY')
+  > EOF
+  $ python writebin.py answer
+
+  $ python "$TESTDIR/md5sum.py" answer
+  ce0b4fda508e3d9f9ece98f8e823b6f7  answer
+  $ hg add answer
+  $ hg qnew -f --no-git addanswer
+  $ grep diff .hg/patches/addanswer
+  diff -r [a-f0-9]* -r [a-f0-9]* answer (re)
 
 strip again