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

Eric Wong [off-list ref] 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.

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...

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.

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.

Signed-off-by: Eric Wong <redacted>
---
  Thanks for the feedback, v2 changes as suggested:
  - remove -q flag from grep invocation for verbose git tests
  - do not remove stderr file after testing
  - fold verbosity check into existing tests for skip-ability

 t/t5570-git-daemon.sh | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh
index d76269a..225a022 100755
--- a/t/t5570-git-daemon.sh
+++ b/t/t5570-git-daemon.sh

@@ -6,6 +6,12 @@ test_description='test fetching over git protocol'
 . "$TEST_DIRECTORY"/lib-git-daemon.sh
 start_git_daemon
 
+check_verbose_connect () {
+	grep -F "Looking up 127.0.0.1 ..." stderr &&
+	grep -F "Connecting to 127.0.0.1 (port " stderr &&
+	grep -F "done." stderr
+}
+
 test_expect_success 'setup repository' '
 	git config push.default matching &&
 	echo content >file &&

@@ -24,7 +30,8 @@ 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 &&
+	check_verbose_connect &&
 	test_cmp file clone/file
 '
 

@@ -32,10 +39,21 @@ test_expect_success 'fetch changes via git protocol' '
 	echo content >>file &&
 	git commit -a -m two &&
 	git push public &&
-	(cd clone && git pull) &&
+	(cd clone && git pull -v) 2>stderr &&
+	check_verbose_connect &&
 	test_cmp file clone/file
 '
 
+test_expect_success 'no-op fetch -v stderr is as expected' '
+	(cd clone && git fetch -v) 2>stderr &&
+	check_verbose_connect
+'
+
+test_expect_success 'no-op fetch without "-v" is quiet' '
+	(cd clone && git fetch) 2>stderr &&
+	! test -s stderr
+'
+
 test_expect_success 'remote detects correct HEAD' '
 	git push public master:other &&
 	(cd clone &&

-- 
EW