All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brandon Williams <bmwill@google.com>
To: Stefan Beller <sbeller@google.com>
Cc: gitster@pobox.com, Johannes.Schindelin@gmx.de, avarab@gmail.com,
	git@vger.kernel.org, jrnieder@gmail.com,
	phillip.wood@dunelm.org.uk
Subject: Re: [PATCH] t4005: modernize style and drop hard coded sha1
Date: Wed, 7 Jun 2017 10:39:59 -0700	[thread overview]
Message-ID: <20170607173959.GA110638@google.com> (raw)
In-Reply-To: <20170607021805.11849-1-sbeller@google.com>

On 06/06, Stefan Beller wrote:
> Use modern style in the test t4005. Remove hard coded sha1 values.
> Combine test prep work and the actual test. Rename the first
> test to contain the word "setup".
> 
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> 
> Junio wrote:
> > If it helps, I _can_ make any set of declarations to make it sound
> > more official, e.g. (the remainder of) June is the "make sure our
> > tests are ready" 
> 
> If it helps, I can write code for that. :)
> 
> Do get a good grasp on which tests need to be fixed, I changed the seed
> value for the sha1 computation and then run the test suite. There are a lot
> of tests passing for this, but also quite a few failing. Then I picked t4005
> randomly to start with. This patch works even with a crippled hash function
> as we use hash-object to get the object id.
> 
> Thanks,
> Stefan
> 
>  t/t4005-diff-rename-2.sh | 95 ++++++++++++++++++++++--------------------------
>  1 file changed, 43 insertions(+), 52 deletions(-)
> 
> diff --git a/t/t4005-diff-rename-2.sh b/t/t4005-diff-rename-2.sh
> index 135addbfbd..f542d2929d 100755
> --- a/t/t4005-diff-rename-2.sh
> +++ b/t/t4005-diff-rename-2.sh
> @@ -3,84 +3,75 @@
>  # Copyright (c) 2005 Junio C Hamano
>  #
>  
> -test_description='Same rename detection as t4003 but testing diff-raw.
> +test_description='Same rename detection as t4003 but testing diff-raw.'
>  
> -'
>  . ./test-lib.sh
>  . "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
>  
> -test_expect_success \
> -    'prepare reference tree' \
> -    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
> -     echo frotz >rezrov &&
> -    git update-index --add COPYING rezrov &&
> -    tree=$(git write-tree) &&
> -    echo $tree'
> -
> -test_expect_success \
> -    'prepare work tree' \
> -    'sed -e 's/HOWEVER/However/' <COPYING >COPYING.1 &&
> -    sed -e 's/GPL/G.P.L/g' <COPYING >COPYING.2 &&
> -    rm -f COPYING &&
> -    git update-index --add --remove COPYING COPYING.?'
> +test_expect_success 'setup reference tree' '
> +	cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
> +	echo frotz >rezrov &&
> +	git update-index --add COPYING rezrov &&
> +	tree=$(git write-tree) &&
> +	echo $tree &&
> +	sed -e 's/HOWEVER/However/' <COPYING >COPYING.1 &&
> +	sed -e 's/GPL/G.P.L/g' <COPYING >COPYING.2 &&
> +	origoid=$(git hash-object COPYING) &&
> +	oid1=$(git hash-object COPYING.1) &&
> +	oid2=$(git hash-object COPYING.2)
> +'

This conversation looks good to me.  The only thing that made me scratch
my head a bit were the shell variables origoid, oid1, and oid2.  It was
just slightly confusing figuring out where they came from in the tests
below before I noticed they were initialized up here.

>  
> +################################################################
>  # tree has COPYING and rezrov.  work tree has COPYING.1 and COPYING.2,
>  # both are slightly edited, and unchanged rezrov.  We say COPYING.1
>  # and COPYING.2 are based on COPYING, and do not say anything about
>  # rezrov.
>  
> -git diff-index -C $tree >current
> -
> -cat >expected <<\EOF
> -:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234	COPYING	COPYING.1
> -:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 06c67961bbaed34a127f76d261f4c0bf73eda471 R1234	COPYING	COPYING.2
> -EOF
> +test_expect_success 'validate output from rename/copy detection (#1)' '
> +	rm -f COPYING &&
> +	git update-index --add --remove COPYING COPYING.? &&
>  
> -test_expect_success \
> -    'validate output from rename/copy detection (#1)' \
> -    'compare_diff_raw current expected'
> +	cat <<-EOF >expected &&
> +	:100644 100644 $origoid $oid1 C1234	COPYING	COPYING.1
> +	:100644 100644 $origoid $oid2 R1234	COPYING	COPYING.2
> +	EOF
> +	git diff-index -C $tree >current &&
> +	compare_diff_raw expected current
> +'
>  
>  ################################################################
> -
> -test_expect_success \
> -    'prepare work tree again' \
> -    'mv COPYING.2 COPYING &&
> -     git update-index --add --remove COPYING COPYING.1 COPYING.2'
> -
>  # tree has COPYING and rezrov.  work tree has COPYING and COPYING.1,
>  # both are slightly edited, and unchanged rezrov.  We say COPYING.1
>  # is based on COPYING and COPYING is still there, and do not say anything
>  # about rezrov.
>  
> -git diff-index -C $tree >current
> -cat >expected <<\EOF
> -:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 06c67961bbaed34a127f76d261f4c0bf73eda471 M	COPYING
> -:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234	COPYING	COPYING.1
> -EOF
> +test_expect_success 'validate output from rename/copy detection (#2)' '
> +	mv COPYING.2 COPYING &&
> +	git update-index --add --remove COPYING COPYING.1 COPYING.2 &&
>  
> -test_expect_success \
> -    'validate output from rename/copy detection (#2)' \
> -    'compare_diff_raw current expected'
> +	cat <<-EOF >expected &&
> +	:100644 100644 $origoid $oid2 M	COPYING
> +	:100644 100644 $origoid $oid1 C1234	COPYING	COPYING.1
> +	EOF
> +	git diff-index -C $tree >current &&
> +	compare_diff_raw current expected
> +'
>  
>  ################################################################
> -
>  # tree has COPYING and rezrov.  work tree has the same COPYING and
>  # copy-edited COPYING.1, and unchanged rezrov.  We should not say
>  # anything about rezrov or COPYING, since the revised again diff-raw
>  # nows how to say Copy.
>  
> -test_expect_success \
> -    'prepare work tree once again' \
> -    'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
> -     git update-index --add --remove COPYING COPYING.1'
> -
> -git diff-index -C --find-copies-harder $tree >current
> -cat >expected <<\EOF
> -:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234	COPYING	COPYING.1
> -EOF
> +test_expect_success 'validate output from rename/copy detection (#3)' '
> +	cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING &&
> +	git update-index --add --remove COPYING COPYING.1 &&
>  
> -test_expect_success \
> -    'validate output from rename/copy detection (#3)' \
> -    'compare_diff_raw current expected'
> +	cat <<-EOF >expected &&
> +	:100644 100644 $origoid $oid1 C1234	COPYING	COPYING.1
> +	EOF
> +	git diff-index -C --find-copies-harder $tree >current &&
> +	compare_diff_raw current expected
> +'
>  
>  test_done
> -- 
> 2.13.0.17.gf3d7728391
> 

-- 
Brandon Williams

  reply	other threads:[~2017-06-07 17:40 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-31 10:42 [PATCH 0/3] Add regression tests for recent rebase -i fixes Phillip Wood
2017-05-31 10:42 ` [PATCH 1/3] rebase -i: Add test for reflog message Phillip Wood
2017-06-01  2:00   ` Junio C Hamano
2017-05-31 10:42 ` [PATCH 2/3] rebase: Add tests for console output Phillip Wood
2017-05-31 19:02   ` Phillip Wood
2017-06-01  1:59     ` Junio C Hamano
2017-06-01 12:56   ` Johannes Schindelin
2017-06-01 23:40     ` Junio C Hamano
2017-06-01 23:47       ` Stefan Beller
2017-06-02 12:47         ` pushing for a new hash, was " Johannes Schindelin
2017-06-02 17:54           ` Jonathan Nieder
2017-06-02 18:05             ` Jonathan Nieder
2017-06-02 20:29             ` Ævar Arnfjörð Bjarmason
2017-06-15 10:38               ` Johannes Schindelin
2017-06-03  0:36             ` Junio C Hamano
2017-06-06 22:22             ` Johannes Schindelin
2017-06-06 22:45               ` Jonathan Nieder
2017-06-07  1:09                 ` Junio C Hamano
2017-06-07  2:18                   ` [PATCH] t4005: modernize style and drop hard coded sha1 Stefan Beller
2017-06-07 17:39                     ` Brandon Williams [this message]
2017-06-06 22:45               ` pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console output Stefan Beller
2017-06-06 22:52                 ` Jonathan Nieder
2017-06-07  0:34                 ` Samuel Lijin
2017-06-07 14:47                 ` Johannes Schindelin
2017-06-07 16:53                   ` Stefan Beller
2017-06-07 10:47     ` Phillip Wood
2017-06-09 16:39       ` Junio C Hamano
2017-06-14 10:18         ` Phillip Wood
2017-06-14 12:51       ` Johannes Schindelin
2017-05-31 10:42 ` [PATCH 3/3] rebase: Add tests for console output with conflicting stash Phillip Wood
2017-06-14 10:24 ` [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes Phillip Wood
2017-06-14 10:24   ` [PATCH v2 1/3] rebase -i: Add test for reflog message Phillip Wood
2017-06-14 10:24   ` [PATCH v2 2/3] rebase: Add regression tests for console output Phillip Wood
2017-06-14 10:24   ` [PATCH v2 3/3] rebase: Add more " Phillip Wood
2017-06-14 20:35   ` [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes Johannes Schindelin
2017-06-15 23:05   ` Junio C Hamano
2017-06-15 23:23     ` Junio C Hamano
2017-06-15 23:29       ` Junio C Hamano
2017-06-16 13:49         ` Johannes Schindelin
2017-06-16 18:43           ` Johannes Sixt
2017-06-16 21:05             ` Junio C Hamano
2017-06-19 19:45             ` Johannes Sixt
2017-06-19 20:02               ` Junio C Hamano
2017-06-19  9:49           ` Phillip Wood
2017-06-19 15:45             ` Junio C Hamano
2017-06-19  9:52         ` Phillip Wood
2017-06-19 17:56 ` [PATCH v3 0/4] Add regression tests for recent " Phillip Wood
2017-06-19 17:56   ` [PATCH v3 1/4] sequencer: print autostash messages to stderr Phillip Wood
2017-06-19 17:56   ` [PATCH v3 2/4] rebase -i: Add test for reflog message Phillip Wood
2017-06-19 17:56   ` [PATCH v3 3/4] rebase: Add regression tests for console output Phillip Wood
2017-06-19 17:56   ` [PATCH v3 4/4] rebase: Add more " Phillip Wood
2017-06-23  4:17   ` [PATCH v3 0/4] Add regression tests for recent rebase -i fixes Junio C Hamano
2017-06-23  5:07     ` Junio C Hamano
2017-06-23  9:53       ` Phillip Wood
2017-06-23 17:03         ` Junio C Hamano
2017-06-23 18:53           ` Junio C Hamano
2017-06-26  9:17             ` Phillip Wood
2017-06-23 19:01           ` Junio C Hamano
2017-06-26  9:23             ` Phillip Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170607173959.GA110638@google.com \
    --to=bmwill@google.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.