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

[PATCH 4/4] tests: skip test_eval_ in internal chain-lint

From
Jeff King <peff@peff.net>
Date
Mar 28, 2023, 20:28 UTC
Message-ID
<20230328202839.GD1241631@coredump.intra.peff.net>
In-Reply-To
<20230328202043.GA1241391@coredump.intra.peff.net>

To check for broken &&-chains, we run "fail_117 && $1" as a test snippet, and check the exit code. We use test_eval_ to do so, because that's the way we run the actual test.

But we don't need any of its niceties, like "set -x" tracing. In fact, they hinder us, because we have to explicitly disable them. So let's skip that and use "eval" more directly, which is simpler. I had hoped it would also be faster, but it doesn't seem to produce a measurable improvement (probably because it's just running internal shell commands, with no subshells or forks).

Note that there is one gotcha: even though we don't intend to run any of the commands if the &&-chain is intact, an error like this:

   test_expect_success 'broken' '
	# this next line breaks the &&-chain
	true
	# and then this one is executed even by the linter
	return 1
   '

means we'll "return 1" from the eval, and thus from test_run_(). We actually do notice this in test_expect_success, but only by saying "hey, this test didn't say it was OK, so it must have failed", which is not right (it should say "broken &&-chain").

We can handle this by calling test_eval_inner_() instead, which is our trick for wrapping "return" in a test snippet. But to do that, we have to push the trace code out of that inner function and into test_eval_(). This is arguably where it belonged in the first place, but it never mattered because the "inner_" function had only one caller.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/test-lib.sh | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0048ec7b6f6..293caf0f20e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1041,10 +1041,7 @@ want_trace () {
 # (and we want to make sure we run any cleanup like
 # "set +x").
 test_eval_inner_ () {
-	# Do not add anything extra (including LF) after '$*'
-	eval "
-		want_trace && trace_level_=$(($trace_level_+1)) && set -x
-		$*"
+	eval "$*"
 }
 
 test_eval_ () {
@@ -1069,7 +1066,10 @@ test_eval_ () {
 	#     be _inside_ the block to avoid polluting the "set -x" output
 	#
 
-	test_eval_inner_ "$@" </dev/null >&3 2>&4
+	# Do not add anything extra (including LF) after '$*'
+	test_eval_inner_ </dev/null >&3 2>&4 "
+		want_trace && trace_level_=$(($trace_level_+1)) && set -x
+		$*"
 	{
 		test_eval_ret_=$?
 		if want_trace
@@ -1095,18 +1095,13 @@ test_run_ () {
 	expecting_failure=$2
 
 	if test "${GIT_TEST_CHAIN_LINT:-1}" != 0; then
-		# turn off tracing for this test-eval, as it simply creates
-		# confusing noise in the "-x" output
-		trace_tmp=$trace
-		trace=
 		# 117 is magic because it is unlikely to match the exit
 		# code of other programs
-		test_eval_ "fail_117 && $1"
+		test_eval_inner_ "fail_117 && $1" </dev/null >&3 2>&4
 		if test $? != 117
 		then
 			BUG "broken &&-chain: $1"
 		fi
-		trace=$trace_tmp
 	fi
 
 	setup_malloc_check
-- 
2.40.0.616.gf524ec75088
Previous: Jeff KingNext: Jeff King
Message 20 of 32 in “some chainlint fixes and performance improvements”
  1. 0/4 some chainlint fixes and performance improvementsJeff King, Mar 28, 2023
  2. 1/4 tests: run internal chain-linter under "make test"Jeff King, Mar 28, 2023
  3. Ævar Arnfjörð BjarmasonMar 29, 2023
  4. Junio C HamanoMar 29, 2023
  5. Jeff KingMar 29, 2023
  6. Junio C HamanoMar 30, 2023
  7. 2/4 tests: replace chainlint subshell with a functionJeff King, Mar 28, 2023
  8. Junio C HamanoMar 28, 2023
  9. 3/4 tests: drop here-doc check from internal chain-linterJeff King, Mar 28, 2023
  10. Junio C HamanoMar 28, 2023
  11. Jeff KingMar 29, 2023
  12. Jeff KingMar 29, 2023
  13. Eric SunshineMar 29, 2023
  14. Eric SunshineMar 29, 2023
  15. Eric SunshineMar 29, 2023
  16. Jeff KingMar 29, 2023
  17. Eric SunshineMar 29, 2023
  18. Eric SunshineMar 29, 2023
  19. Jeff KingMar 29, 2023
  20. 4/4 tests: skip test_eval_ in internal chain-lintJeff King, Mar 28, 2023
  21. Jeff KingMar 28, 2023
  22. Jeff KingMar 30, 2023
  23. Junio C HamanoMar 30, 2023
  24. 0/5 some chainlint fixes and performance improvementsJeff King, Mar 30, 2023
  25. 1/5 tests: run internal chain-linter under "make test"Jeff King, Mar 30, 2023
  26. 2/5 tests: replace chainlint subshell with a functionJeff King, Mar 30, 2023
  27. 3/5 tests: diagnose unclosed here-doc in chainlint.plJeff King, Mar 30, 2023
  28. Eric SunshineMar 30, 2023
  29. 4/5 tests: drop here-doc check from internal chain-linterJeff King, Mar 30, 2023
  30. 5/5 tests: skip test_eval_ in internal chain-lintJeff King, Mar 30, 2023
  31. Junio C HamanoMar 30, 2023
  32. Jeff KingMar 30, 2023

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.