Re: [PATCH] t5570: add tests for "git fetch -v"

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 13, 2016, 20:48 UTC
Message-ID: <xmqqtwlcs5e6.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160213003526.GA15899@dcvr.yhbt.net>

Eric Wong <normalperson@yhbt.net> writes:

> Now that git_connect is more information about connectivity
> progress after: ("pass transport verbosity down to git_connect")
> we should ensure it remains so for future users who need to
> to diagnose networking problems.

Thanks.

Show 14 quoted lines

> diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh
> index d76269a..095e862 100755
> --- a/t/t5570-git-daemon.sh
> +++ b/t/t5570-git-daemon.sh
> @@ -6,6 +6,13 @@ test_description='test fetching over git protocol'
>  . "$TEST_DIRECTORY"/lib-git-daemon.sh
>  start_git_daemon
>  
> +check_verbose_connect () {
> +	grep -qF "Looking up 127.0.0.1 ..." stderr &&
> +	grep -qF "Connecting to 127.0.0.1 (port " stderr &&
> +	grep -qF "done." stderr &&
> +	rm stderr
> +}

If you run the test without "-v", you would not see these strings from grep hits, right? I'd prefer losing "-q" here. Also do we need "rm stderr" at the end? Everybody that uses this heler to check the result will create the file from scratch anyway, so...

Show 8 quoted lines

> @@ -24,18 +31,32 @@ test_expect_success 'create git-accessible bare repository' '
>  '
>  
>  test_expect_success 'clone git repository' '
> -	git clone "$GIT_DAEMON_URL/repo.git" clone &&
> +	git clone -v "$GIT_DAEMON_URL/repo.git" clone 2>stderr &&
>  	test_cmp file clone/file
>  '

What's the point of saving the error output without checking?

> +test_expect_success 'clone -v stderr is as expected' check_verbose_connect
> +

Ahh, you made it into two separate tests, one depending on the previous one not being skipped? I think it is better to fold this into the previous one, i.e.

	clone -v 2>stderr &&
        check_verbose_output &&
        test_cmp file clone/file

The same comment applies to the next one.

The test for "fetch -v" is already in that form, so no need to correct anything there. Also it is good that you test the case without "-v" and ensure it stays quiet.

Thanks.