From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Mon, 06 Feb 2023 21:50:05 GMT
Subject: Re: [PATCH v4 3/3] t4113: put executable lines to test_expect_success
Message-ID: <230206.86ttzy4g3c.gmgdl@evledraar.gmail.com>
In-Reply-To: <20230206211823.8651-4-cheskaqiqi@gmail.com>


On Mon, Feb 06 2023, Shuqi Liang wrote:

. ./test-lib.sh
>  
> -# setup
> -
> -cat >test-patch <<\EOF
> -diff --git a/file b/file
> ---- a/file
> -+++ b/file
> -@@ -1,2 +1,3 @@
> - a
> - b
> -+c
> -EOF
> -
> -echo 'a' >file
> -echo 'b' >>file
> -echo 'c' >>file
> -
>  test_expect_success setup '
> +	cat >test-patch <<-\EOF
> +	diff --git a/file b/file
> +	--- a/file
> +	+++ b/file
> +	@@ -1,2 +1,3 @@
> +	a
> +	b
> +	+c
> +	EOF
> +
> +	echo 'a' >file
> +	echo 'b' >>file
> +	echo 'c' >>file

I have not read the rest here, but this immediately fails with a very
large error from chain-lint by default, and even if you manually disable
it (which I assume you're doing, or just not testing these at all before
submission), you'll get:
	
	$ ./t4113-apply-ending.sh --no-chain-lint
	ok 1 - setup
	ok 2 - apply at the end
	ok 3 - apply at the beginning
	./t4113-apply-ending.sh: 44: b: not found
	./t4113-apply-ending.sh: 48: c
	        git update-index file
	        test_must_fail git apply --index test-patch
	: not found
	# passed all 3 test(s)
	1..3

Which shows that even with the &&-chaining fixed you have quoting issues
here, you're trying to execute 'b' etc.

I didn't read the rest of this topic, but please test with chain-lint,
see if there's any unexpected new output from the tests etc. before a v5
re-roll.