[1/3] t8003: check output of coalesced blame
diff mbox series

Message ID 20200813052305.GA2514880@coredump.intra.peff.net
State New
Headers show
Series
  • blame: fix bug in coalescing non-adjacent "-L" chunks
Related show

Commit Message

Jeff King Aug. 13, 2020, 5:23 a.m. UTC
Commit f0cbe742f4 (blame: add a test to cover blame_coalesce(),
2019-06-20) added a test case where blame can usefully coalesce two
groups of lines. But since it relies on the normal blame output, it only
exercises the code and can't tell whether the lines were actually
joined into a single group.

However, by using --porcelain output, we can see how git-blame considers
the groupings (and likewise how the coalescing might have a real
user-visible impact for a tool that uses the porcelain-output
groupings). This lets us confirm that we are indeed coalescing correctly
(and the fact that this test case requires coalescing can be verified by
dropping the call to blame_coalesce(), causing the test to fail).

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t8003-blame-corner-cases.sh | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

Comments

Junio C Hamano Aug. 13, 2020, 5:04 p.m. UTC | #1
Jeff King <peff@peff.net> writes:

> Commit f0cbe742f4 (blame: add a test to cover blame_coalesce(),
> 2019-06-20) added a test case where blame can usefully coalesce two
> groups of lines. But since it relies on the normal blame output, it only
> exercises the code and can't tell whether the lines were actually
> joined into a single group.
>
> However, by using --porcelain output, we can see how git-blame considers
> the groupings (and likewise how the coalescing might have a real
> user-visible impact for a tool that uses the porcelain-output
> groupings). This lets us confirm that we are indeed coalescing correctly
> (and the fact that this test case requires coalescing can be verified by
> dropping the call to blame_coalesce(), causing the test to fail).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  t/t8003-blame-corner-cases.sh | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh
> index b871dd4f86..1e89494ef6 100755
> --- a/t/t8003-blame-corner-cases.sh
> +++ b/t/t8003-blame-corner-cases.sh
> @@ -273,10 +273,6 @@ test_expect_success 'blame file with CRLF core.autocrlf=true' '
>  	grep "A U Thor" actual
>  '
>  
> -# Tests the splitting and merging of blame entries in blame_coalesce().
> -# The output of blame is the same, regardless of whether blame_coalesce() runs
> -# or not, so we'd likely only notice a problem if blame crashes or assigned
> -# blame to the "splitting" commit ('SPLIT' below).
>  test_expect_success 'blame coalesce' '
>  	cat >giraffe <<-\EOF &&
>  	ABC
> @@ -302,10 +298,11 @@ test_expect_success 'blame coalesce' '
>  	git commit -m "same contents as original" &&
>  
>  	cat >expect <<-EOF &&
> -	$oid 1) ABC
> -	$oid 2) DEF
> +	$oid 1 1 2
> +	$oid 2 2
>  	EOF

It has become a bit harder to grok, but for the purpose of the later
steps to see where things exactly came from (including their line
numbers), it is easier to see what is going on with the new format.

> -	git -c core.abbrev=$(test_oid hexsz) blame -s giraffe >actual &&
> +	git blame --porcelain giraffe >actual.raw &&

The original forced the abbrev length; by switching to the format
for porcelain-writers, we know we will get the full object name.  OK.

> +	grep "^$oid" actual.raw >actual &&
>  	test_cmp expect actual
>  '
Jeff King Aug. 13, 2020, 5:22 p.m. UTC | #2
On Thu, Aug 13, 2020 at 10:04:51AM -0700, Junio C Hamano wrote:

> >  	cat >expect <<-EOF &&
> > -	$oid 1) ABC
> > -	$oid 2) DEF
> > +	$oid 1 1 2
> > +	$oid 2 2
> >  	EOF
> 
> It has become a bit harder to grok, but for the purpose of the later
> steps to see where things exactly came from (including their line
> numbers), it is easier to see what is going on with the new format.

Yeah, I agree the numbers are a bit more inscrutable. We could do
something like:

diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh
index ba8013b002..9486888e5a 100755
--- a/t/t8003-blame-corner-cases.sh
+++ b/t/t8003-blame-corner-cases.sh
@@ -301,12 +301,14 @@ test_expect_success 'setup coalesce tests' '
 '
 
 test_expect_success 'blame coalesce' '
-	cat >expect <<-EOF &&
+	q_to_tab >expect <<-EOF &&
 	$orig 1 1 2
+	QABC
 	$orig 2 2
+	QDEF
 	EOF
 	git blame --porcelain $final giraffe >actual.raw &&
-	grep "^$orig" actual.raw >actual &&
+	egrep "^($orig|	)" actual.raw >actual &&
 	test_cmp expect actual
 '

but IMHO that isn't much more readable (largely due to the
tab-handling in the here-doc).

-Peff

Patch
diff mbox series

diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh
index b871dd4f86..1e89494ef6 100755
--- a/t/t8003-blame-corner-cases.sh
+++ b/t/t8003-blame-corner-cases.sh
@@ -273,10 +273,6 @@  test_expect_success 'blame file with CRLF core.autocrlf=true' '
 	grep "A U Thor" actual
 '
 
-# Tests the splitting and merging of blame entries in blame_coalesce().
-# The output of blame is the same, regardless of whether blame_coalesce() runs
-# or not, so we'd likely only notice a problem if blame crashes or assigned
-# blame to the "splitting" commit ('SPLIT' below).
 test_expect_success 'blame coalesce' '
 	cat >giraffe <<-\EOF &&
 	ABC
@@ -302,10 +298,11 @@  test_expect_success 'blame coalesce' '
 	git commit -m "same contents as original" &&
 
 	cat >expect <<-EOF &&
-	$oid 1) ABC
-	$oid 2) DEF
+	$oid 1 1 2
+	$oid 2 2
 	EOF
-	git -c core.abbrev=$(test_oid hexsz) blame -s giraffe >actual &&
+	git blame --porcelain giraffe >actual.raw &&
+	grep "^$oid" actual.raw >actual &&
 	test_cmp expect actual
 '