Patchwork [3,of,4] tests: add tests for --binary option in Git mode

login
register
mail settings
Submitter Alexander Fomin
Date April 4, 2017, 10:21 p.m.
Message ID <373acf7da1a621607a1c.1491344482@devvm2125.lla2.facebook.com>
Download mbox | patch
Permalink /patch/19962/
State Changes Requested
Headers show

Comments

Alexander Fomin - April 4, 2017, 10:21 p.m.
# HG changeset patch
# User Alexander Fomin <afomin@fb.com>
# Date 1491335167 25200
#      Tue Apr 04 12:46:07 2017 -0700
# Node ID 373acf7da1a621607a1cc062b96ad67a42a7e016
# Parent  27515c7e38db9d93e18b7df13149da7c0d88eeb2
tests: add tests for --binary option in Git mode

This patch adds some tests to verify correct --binary option behaviour in Git
mode (issue5510).
Ryan McElroy - April 5, 2017, 2:37 p.m.
On 4/4/17 11:21 PM, Alexander Fomin wrote:
> # HG changeset patch
> # User Alexander Fomin <afomin@fb.com>
> # Date 1491335167 25200
> #      Tue Apr 04 12:46:07 2017 -0700
> # Node ID 373acf7da1a621607a1cc062b96ad67a42a7e016
> # Parent  27515c7e38db9d93e18b7df13149da7c0d88eeb2
> tests: add tests for --binary option in Git mode

I think we should keep git uncapitalized here.

The code in patch 2 in this series looks good to me. This new test is 
short enough that I'd fold it into the previous patch.

> This patch adds some tests to verify correct --binary option behaviour in Git
> mode (issue5510).
>
> diff --git a/tests/test-diff-binary-file.t b/tests/test-diff-binary-file.t
> --- a/tests/test-diff-binary-file.t
> +++ b/tests/test-diff-binary-file.t
> @@ -107,4 +107,28 @@
>     \ No newline at end of file
>     +\x00\x01\x02\x03 (esc)

Remember to add a comment (non-indented line here) with a quick summary 
of the test.

The test itself looks good.

>   
> +  $ hg diff --no-binary -r 0 -r 1
> +  diff -r fb45f71337ad -r 9ca112d1a3c1 binfile.bin
> +  Binary file binfile.bin has changed
> +
> +  $ hg diff --git --no-binary -r 0 -r 1
> +  diff --git a/binfile.bin b/binfile.bin
> +  Binary file binfile.bin has changed
> +
> +  $ hg diff --git --binary -r 0 -r 1
> +  diff --git a/binfile.bin b/binfile.bin
> +  index eaf36c1daccfdf325514461cd1a2ffbc139b5464..ba71a782e93f3fb63a428383706065e3ec2828e9
> +  GIT binary patch
> +  literal 5
> +  Mc${NkWMbw50018V5dZ)H
> +
> +
> +  $ hg diff --git --binary --config diff.nobinary=True -r 0 -r 1
> +  diff --git a/binfile.bin b/binfile.bin
> +  index eaf36c1daccfdf325514461cd1a2ffbc139b5464..ba71a782e93f3fb63a428383706065e3ec2828e9
> +  GIT binary patch
> +  literal 5
> +  Mc${NkWMbw50018V5dZ)H
> +
> +
>     $ cd ..
>
Yuya Nishihara - April 5, 2017, 2:54 p.m.
On Wed, 5 Apr 2017 15:37:46 +0100, Ryan McElroy wrote:
> On 4/4/17 11:21 PM, Alexander Fomin wrote:
> > # HG changeset patch
> > # User Alexander Fomin <afomin@fb.com>
> > # Date 1491335167 25200
> > #      Tue Apr 04 12:46:07 2017 -0700
> > # Node ID 373acf7da1a621607a1cc062b96ad67a42a7e016
> > # Parent  27515c7e38db9d93e18b7df13149da7c0d88eeb2
> > tests: add tests for --binary option in Git mode
> 
> I think we should keep git uncapitalized here.
> 
> The code in patch 2 in this series looks good to me. This new test is
> short enough that I'd fold it into the previous patch.

Yeah, I was about to do that.

Can you resend the patches up to here?

> > +  $ hg diff --no-binary -r 0 -r 1
> > +  diff -r fb45f71337ad -r 9ca112d1a3c1 binfile.bin
> > +  Binary file binfile.bin has changed
> > +
> > +  $ hg diff --git --no-binary -r 0 -r 1
> > +  diff --git a/binfile.bin b/binfile.bin
> > +  Binary file binfile.bin has changed
> > +
> > +  $ hg diff --git --binary -r 0 -r 1
> > +  diff --git a/binfile.bin b/binfile.bin
> > +  index eaf36c1daccfdf325514461cd1a2ffbc139b5464..ba71a782e93f3fb63a428383706065e3ec2828e9
> > +  GIT binary patch
> > +  literal 5
> > +  Mc${NkWMbw50018V5dZ)H
> > +
> > +
> > +  $ hg diff --git --binary --config diff.nobinary=True -r 0 -r 1
> > +  diff --git a/binfile.bin b/binfile.bin
> > +  index eaf36c1daccfdf325514461cd1a2ffbc139b5464..ba71a782e93f3fb63a428383706065e3ec2828e9
> > +  GIT binary patch
> > +  literal 5
> > +  Mc${NkWMbw50018V5dZ)H

If both --binary and --text are explicitly set, which one will win? Can
you add a test about it? I guess text diff will be displayed, which seems
correct.
Alexander Fomin - April 5, 2017, 9:49 p.m.
On 05/04/2017, 07:54, "Yuya Nishihara" <youjah@gmail.com on behalf of yuya@tcha.org> wrote:
> 

> On Wed, 5 Apr 2017 15:37:46 +0100, Ryan McElroy wrote:

> > On 4/4/17 11:21 PM, Alexander Fomin wrote:

> > > # HG changeset patch

> > > # User Alexander Fomin <afomin@fb.com>

> > > # Date 1491335167 25200

> > > #      Tue Apr 04 12:46:07 2017 -0700

> > > # Node ID 373acf7da1a621607a1cc062b96ad67a42a7e016

> > > # Parent  27515c7e38db9d93e18b7df13149da7c0d88eeb2

> > > tests: add tests for --binary option in Git mode

> >

> > I think we should keep git uncapitalized here.

> >

> > The code in patch 2 in this series looks good to me. This new test is

> > short enough that I'd fold it into the previous patch.

> 

> Yeah, I was about to do that.

> 

> Can you resend the patches up to here?

> 

Yes, I’ll merge the second and third patches into one.
> > > +  $ hg diff --no-binary -r 0 -r 1

> > > +  diff -r fb45f71337ad -r 9ca112d1a3c1 binfile.bin

> > > +  Binary file binfile.bin has changed

> > > +

> > > +  $ hg diff --git --no-binary -r 0 -r 1

> > > +  diff --git a/binfile.bin b/binfile.bin

> > > +  Binary file binfile.bin has changed

> > > +

> > > +  $ hg diff --git --binary -r 0 -r 1

> > > +  diff --git a/binfile.bin b/binfile.bin

> > > +  index eaf36c1daccfdf325514461cd1a2ffbc139b5464..ba71a782e93f3fb63a428383706065e3ec2828e9

> > > +  GIT binary patch

> > > +  literal 5

> > > +  Mc${NkWMbw50018V5dZ)H

> > > +

> > > +

> > > +  $ hg diff --git --binary --config diff.nobinary=True -r 0 -r 1

> > > +  diff --git a/binfile.bin b/binfile.bin

> > > +  index eaf36c1daccfdf325514461cd1a2ffbc139b5464..ba71a782e93f3fb63a428383706065e3ec2828e9

> > > +  GIT binary patch

> > > +  literal 5

> > > +  Mc${NkWMbw50018V5dZ)H

> 

> If both --binary and --text are explicitly set, which one will win? Can

> you add a test about it? I guess text diff will be displayed, which seems

> correct.

OK, I’ll add a testcase.

Patch

diff --git a/tests/test-diff-binary-file.t b/tests/test-diff-binary-file.t
--- a/tests/test-diff-binary-file.t
+++ b/tests/test-diff-binary-file.t
@@ -107,4 +107,28 @@ 
   \ No newline at end of file
   +\x00\x01\x02\x03 (esc)
 
+  $ hg diff --no-binary -r 0 -r 1
+  diff -r fb45f71337ad -r 9ca112d1a3c1 binfile.bin
+  Binary file binfile.bin has changed
+
+  $ hg diff --git --no-binary -r 0 -r 1
+  diff --git a/binfile.bin b/binfile.bin
+  Binary file binfile.bin has changed
+
+  $ hg diff --git --binary -r 0 -r 1
+  diff --git a/binfile.bin b/binfile.bin
+  index eaf36c1daccfdf325514461cd1a2ffbc139b5464..ba71a782e93f3fb63a428383706065e3ec2828e9
+  GIT binary patch
+  literal 5
+  Mc${NkWMbw50018V5dZ)H
+  
+
+  $ hg diff --git --binary --config diff.nobinary=True -r 0 -r 1
+  diff --git a/binfile.bin b/binfile.bin
+  index eaf36c1daccfdf325514461cd1a2ffbc139b5464..ba71a782e93f3fb63a428383706065e3ec2828e9
+  GIT binary patch
+  literal 5
+  Mc${NkWMbw50018V5dZ)H
+  
+
   $ cd ..