Re: [PATCH v2 1/2] Add test to describe expectation of blame --reverse with branched history

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH v2 1/2] Add test to describe expectation of blame --reverse with branched history

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:07:01

Max Kirillov [off-list ref] writes:
quoted hunk
If history contains merges from feature branches, `blame --reverse`
reports not the commit when the line was actually edited, but head of
the last merged branch which was created before the edit.

As a workaround, `blame --reverse --first-parent` could be used to find
the merge of branch containing the edit, but it was disabled in
95a4fb0eac, because incorrectly specified range could produce in
unexpected and meaningless result.

Add tests which describe ideal functionality with and without
`--first-parent`.

Signed-off-by: Max Kirillov <redacted>
---
 t/t8009-blame-reverse.sh | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100755 t/t8009-blame-reverse.sh
diff --git a/t/t8009-blame-reverse.sh b/t/t8009-blame-reverse.sh
new file mode 100755
index 0000000..9f40613
--- /dev/null
+++ b/t/t8009-blame-reverse.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+
+test_description='git blame reverse'
+. ./test-lib.sh
+
If you are going to dedicate the whole test script for this test,
could you draw the topology of the commit DAG here, so that readers
can more easily follow what is going on?
+test_expect_success setup '
+	test_commit A0 file.t line0 &&
+	test_commit A1 &&
+	git reset --hard A0 &&
+	test_commit B1 &&
+	test_commit B2 file.t line0changed &&
+	git reset --hard A1 &&
+	test_merge A2 B2 &&
+	git reset --hard A1 &&
+	test_commit C1 &&
+	git reset --hard A2 &&
+	test_merge A3 C1
+	'
Let me try.

 (1) for merges, an edge with '*' denotes the one to the first
     parent.
 (2) a commit that touches file.t are in capital

           c1---a3
          /    *
         /    /
   A0---a1--*a2
     \      /
      b1---B2

Did I draw it correctly?

+test_expect_failure 'blame --reverse finds B1, not C1' '
+	git blame --porcelain --reverse A0..A3 -- file.t >actual_full &&
+	head -1 <actual_full | sed -e "sX .*XX" >actual &&
"head -n 1" is more POSIXy way to spell this.

Don't get cute with sXXX in sed script when the usual s/// suffices;
the only effect of the cuteness is to waste readers' time, who have
to wonder if there is something interesting going on.
+	git rev-parse B1 >expect &&
+	test_cmp expect actual
+	'
+
+test_expect_failure 'blame --reverse --first-parent finds A1' '
+	git blame --porcelain --reverse --first-parent A0..A3 -- file.t >actual_full &&
+	head -1 <actual_full | sed -e "sX .*XX" >actual &&
+	git rev-parse A1 >expect &&
+	test_cmp expect actual
+	'
+
+test_done

Re: [PATCH v2 1/2] Add test to describe expectation of blame --reverse with branched history

From: Max Kirillov <hidden>
Date: 2016-06-15 23:07:01

On Wed, Oct 21, 2015 at 09:19:45PM -0700, Junio C Hamano wrote:
Let me try.

 (1) for merges, an edge with '*' denotes the one to the first
     parent.
 (2) a commit that touches file.t are in capital

           c1---a3
          /    *
         /    /
   A0---a1--*a2
     \      /
      b1---B2

Did I draw it correctly?
Yes. Though I would
quoted
+test_expect_failure 'blame --reverse finds B1, not C1' '
+	git blame --porcelain --reverse A0..A3 -- file.t >actual_full &&
+	head -1 <actual_full | sed -e "sX .*XX" >actual &&
"head -n 1" is more POSIXy way to spell this.

Don't get cute with sXXX in sed script when the usual s/// suffices;
the only effect of the cuteness is to waste readers' time, who have
to wonder if there is something interesting going on.
Ok. Will fix it and add the graph.

Re: [PATCH v2 1/2] Add test to describe expectation of blame --reverse with branched history

From: Max Kirillov <hidden>
Date: 2016-06-15 23:07:01

On Thu, Oct 22, 2015 at 05:37:21PM +0300, Max Kirillov wrote:
Yes. Though I would
... make the line of As straight.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help