git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/2] t5556: replace test_i18ngrep with a simple grep

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 27, 2018, 20:16 UTC
Message-ID
<xmqq371mqjce.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<xmqqeflorc9m.fsf@gitster-ct.c.googlers.com>
Junio C Hamano <gitster@pobox.com> writes:
Show 15 quoted lines
> Jeff King <peff@peff.net> writes:
>
>> If I understand Gábor's patch correctly, it is using test_i18ngrep for
>> the specific lines we care about so that we don't have to worry about
>> other cruft lines that may or may not appear (including the hangup one).
>>
>> The downside is that we would not notice if a _new_ error message
>> (beyond the ones we expect and the one we were explicitly ignoring)
>> appeared. IMHO that's probably fine.
>
> Ah, OK, I didn't notice how the multi-line one was handled.  Unable
> to notice new error messages and undisturbed by possible "hung up"
> messages are the sides of the same coin---I myself am unsure if it
> is a good trade-off, but I'm inclined to defer to judgment of two
> people ;-)

OK, somehow I had the version from Ramsay on a topic branch that was not merged to 'pu'. Here is the replacement for 2/2 I'd be queuing.

We'd need SZEDER to sign it off (optionally correcting mistakes in the log message) if we are going with this solution.

Thanks.
-- >8 --
From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Tue, 13 Feb 2018 11:04:37 +0100
Subject: [PATCH] t5536: simplify checks for fetch error verification
The verify_stderr helper had this construct
	test_i18ngrep ...  error | grep -v ... >actual | sort &&
	...

in which 'sort' was clearly doing nothing (other than hiding the exit status of the "grep -v" from &&-chain). It obviously is a botched attempt to make sure "actual" can be compared with expected output without having to worry about the order of errors and warnings in the input file, i.e.

	test_i18ngrep ...  error | grep -v ... | sort >actual &&
	...

Instead of grabbing all errors and warnings from the command and seeing if they match what is expected after sorted, look for specific errors and warnings each test cares about and eliminate this buggy helper.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5536-fetch-conflicts.sh | 22 ++++------------------
 1 file changed, 4 insertions(+), 18 deletions(-)
diff --git a/t/t5536-fetch-conflicts.sh b/t/t5536-fetch-conflicts.sh
index 2e42cf3316..91f28c2f78 100755
--- a/t/t5536-fetch-conflicts.sh
+++ b/t/t5536-fetch-conflicts.sh
@@ -18,14 +18,6 @@ setup_repository () {
 	)
 }
 
-verify_stderr () {
-	cat >expected &&
-	# We're not interested in the error
-	# "fatal: The remote end hung up unexpectedly":
-	test_i18ngrep -E '^(fatal|warning):' <error | grep -v 'hung up' >actual | sort &&
-	test_i18ncmp expected actual
-}
-
 test_expect_success 'setup' '
 	git commit --allow-empty -m "Initial" &&
 	git branch branch1 &&
@@ -48,9 +40,7 @@ test_expect_success 'fetch conflict: config vs. config' '
 		"+refs/heads/branch2:refs/remotes/origin/branch1" && (
 		cd ccc &&
 		test_must_fail git fetch origin 2>error &&
-		verify_stderr <<-\EOF
-		fatal: Cannot fetch both refs/heads/branch1 and refs/heads/branch2 to refs/remotes/origin/branch1
-		EOF
+		test_i18ngrep "fatal: Cannot fetch both refs/heads/branch1 and refs/heads/branch2 to refs/remotes/origin/branch1" error
 	)
 '
 
@@ -77,9 +67,7 @@ test_expect_success 'fetch conflict: arg vs. arg' '
 		test_must_fail git fetch origin \
 			refs/heads/*:refs/remotes/origin/* \
 			refs/heads/branch2:refs/remotes/origin/branch1 2>error &&
-		verify_stderr <<-\EOF
-		fatal: Cannot fetch both refs/heads/branch1 and refs/heads/branch2 to refs/remotes/origin/branch1
-		EOF
+		test_i18ngrep "fatal: Cannot fetch both refs/heads/branch1 and refs/heads/branch2 to refs/remotes/origin/branch1" error
 	)
 '
 
@@ -90,10 +78,8 @@ test_expect_success 'fetch conflict: criss-cross args' '
 		git fetch origin \
 			refs/heads/branch1:refs/remotes/origin/branch2 \
 			refs/heads/branch2:refs/remotes/origin/branch1 2>error &&
-		verify_stderr <<-\EOF
-		warning: refs/remotes/origin/branch1 usually tracks refs/heads/branch1, not refs/heads/branch2
-		warning: refs/remotes/origin/branch2 usually tracks refs/heads/branch2, not refs/heads/branch1
-		EOF
+		test_i18ngrep "warning: refs/remotes/origin/branch1 usually tracks refs/heads/branch1, not refs/heads/branch2" error &&
+		test_i18ngrep "warning: refs/remotes/origin/branch2 usually tracks refs/heads/branch2, not refs/heads/branch1" error
 	)
 '
 
-- 
2.16.2-264-ge3a80781f5
Previous: Junio C HamanoNext: Junio C Hamano
Message 10 of 15 in “t5556: replace test_i18ngrep with a simple grep”
  1. 2/2 t5556: replace test_i18ngrep with a simple grepRamsay Jones, Feb 12, 2018
  2. Junio C HamanoFeb 12, 2018
  3. Junio C HamanoFeb 12, 2018
  4. Ramsay JonesFeb 13, 2018
  5. Junio C HamanoFeb 13, 2018
  6. SZEDER GáborFeb 13, 2018
  7. Junio C HamanoFeb 13, 2018
  8. Jeff KingFeb 13, 2018
  9. Junio C HamanoFeb 13, 2018
  10. Junio C HamanoFeb 27, 2018
  11. Junio C HamanoFeb 27, 2018
  12. Ramsay JonesFeb 27, 2018
  13. SZEDER GáborFeb 28, 2018
  14. Ramsay JonesFeb 28, 2018
  15. Junio C HamanoFeb 28, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.