Subject: Re: [PATCH] t1050-large: replace dd by test-genrandom

From: Junio C Hamano <[email-masked]>
Date: Tue, 13 Jan 2015 10:56:32 -0800

Johannes Sixt <[email-masked]> writes:

> For some unknown reason, the dd on my Windows box segfaults every now
> and than, but since recently, it does so much more often than it used
> to, which makes running the test suite burdensome.
>
> Get rid of four invocations of dd and use test-genrandom instead.
>
> The new code does change some properties of the generated files:
>
>  - They are a bit smaller.
>  - They are not sparse anymore.
>  - They do not compress well anymore.
>  - The smaller of the four files is now a prefix of the larger.
>
> Fortunately, the tests do not depend on these properties, which would
> have a big influence on the size of the generated pack files. There *is*
> a test in t1050 that checks the size of pack files generated from large
> blobs, but it runs in its own repository with its own set of files (that
> are already generated with test-genrandom!).
>
> To emphasize that three of the large blobs are exact copies, use cp to
> allocate them.
>
> While we are here, replace cmp with test_cmp_bin to document the
> binary-ness of the comparison, which was hinted at by a comment, but not
> stated explicitly.
>
> Signed-off-by: Johannes Sixt <[email-masked]>
> ---
>  I won't mind if the first paragraph of the proposed commit message is
>  removed, but without the motivation, this commit reads as if it were
>  merely code churn.

I agree that it is good to see the motivation.  Thanks.

>
>  The existing test-genrandom invocations look like this:
>
>    test-genrandom "a" $(( 66 * 1024 )) >mid1
>
>  I chose not to mimick this style because being precise with the file
>  size is not important for the files generated here.
>
>  t/t1050-large.sh | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/t/t1050-large.sh b/t/t1050-large.sh
> index f5a9119..f653121 100755
> --- a/t/t1050-large.sh
> +++ b/t/t1050-large.sh
> @@ -9,10 +9,10 @@ test_expect_success setup '
>  	# clone does not allow us to pass core.bigfilethreshold to
>  	# new repos, so set core.bigfilethreshold globally
>  	git config --global core.bigfilethreshold 200k &&
> -	echo X | dd of=large1 bs=1k seek=2000 &&
> -	echo X | dd of=large2 bs=1k seek=2000 &&
> -	echo X | dd of=large3 bs=1k seek=2000 &&
> -	echo Y | dd of=huge bs=1k seek=2500 &&
> +	test-genrandom seed1 2000000 >large1 &&
> +	cp large1 large2 &&
> +	cp large1 large3 &&
> +	test-genrandom seed2 2500000 >huge &&
>  	GIT_ALLOC_LIMIT=1500k &&
>  	export GIT_ALLOC_LIMIT
>  '
> @@ -61,7 +61,7 @@ test_expect_success 'checkout a large file' '
>  	large1=$(git rev-parse :large1) &&
>  	git update-index --add --cacheinfo 100644 $large1 another &&
>  	git checkout another &&
> -	cmp large1 another ;# this must not be test_cmp
> +	test_cmp_bin large1 another
>  '
>  
>  test_expect_success 'packsize limit' '
> @@ -162,7 +162,7 @@ test_expect_success 'pack-objects with large loose object' '
>  	test_create_repo packed &&
>  	mv pack-* packed/.git/objects/pack &&
>  	GIT_DIR=packed/.git git cat-file blob $SHA1 >actual &&
> -	cmp huge actual
> +	test_cmp_bin huge actual
>  '
>  
>  test_expect_success 'tar achiving' '