Re: [PATCH 1/3] Update t4001 to use modern syntax.
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 8, 2026, 20:48 UTC
- Message-ID
- <xmqqpkynv599.fsf@gitster.g>
- In-Reply-To
- <20260908-modernize-t4001-v1-1-cab3933a173f@fastmail.com>
"Mark C. Chu-Carroll via B4 Relay" <devnull+markchucarroll.fastmail.com@kernel.org> writes:
> Subject: Re: [PATCH 1/3] Update t4001 to use modern syntax.
Documentation/SubmittingPatches::[[describe-changes]] Documentation/SubmittingPatches::[[summary-section]]
> From: "Mark C. Chu-Carroll" <markchucarroll@fastmail.com> > > ---
Documentation/SubmittingPatches::[[sign-off]]
Show 11 quoted lines
> t/t4001-diff-rename.sh | 31 ++++++++++++++++--------------- > t/t4009-diff-rename-4.sh | 8 ++++---- > 2 files changed, 20 insertions(+), 19 deletions(-) > > diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh > index ad474100af..2aa161c217 100755 > --- a/t/t4001-diff-rename.sh > +++ b/t/t4001-diff-rename.sh > @@ -88,28 +88,29 @@ test_expect_success 'setup' ' > EOF > '
There are a bit more in the differences between this ancient style and the modern style. Not just the title appearing on the first line and the body is opened with a single quote at the end of the first line, the body is indented with a single tab.
Show 7 quoted lines
> > -test_expect_success \ > - 'update-index --add a file.' \ > - 'git update-index --add path0' > +test_expect_success 'update-index --add a file.' ' > + git update-index --add path0 > +'
Also in "modern style", the tests are split at more logical boundaries. As the topic of this test is "diff rename", our purpose of this test script is not to catch a crashing "update-index --add". We are not interested in finding "update-index --add" to fail and see "not ok" for such a failure. This step is merely the first step of building the tree object to be compared later with a modified index.
Show 6 quoted lines
> -test_expect_success \ > - 'write that tree.' \ > - 'tree=$(git write-tree) && echo $tree' > +test_expect_success 'write that tree.' ' > + tree=$(git write-tree) && echo $tree > +'
Likewise, we are not interested to find out what object name the resulting tree object gets. "echo" here were placed long ago merely for debugging purposes.
> sed -e 's/line/Line/' <path0 >path1 > rm -f path0
And in "modern style" tests, we strongly frown upon tests doing anything outside test_expect_success blocks. This is a preparation to pretend that path0 was "renamed" to path1, and it is concluded ...
Show 10 quoted lines
> -test_expect_success \ > - 'renamed and edited the file.' \ > - 'git update-index --add --remove path0 path1' > > -test_expect_success \ > - 'git diff-index -p -M after rename and editing.' \ > - 'git diff-index -p -M $tree >current' > +test_expect_success 'renamed and edited the file.' ' > + git update-index --add --remove path0 path1 > +'
... with this step.
> +test_expect_success 'git diff-index -p -M after rename and editing.' ' > + git diff-index -p -M $tree >current > +'
And the output is obtained. Again, it is not like we are happy that this "diff-index" does not crash, so in "modern style", we do not split a logically test like this at this point. We want to see the command produce, without segfaulting, its output to the file "current", and we also want to see that the result matches what we expect.
Show 6 quoted lines
> -test_expect_success \ > - 'validate the output.' \ > - 'compare_diff_patch current expected' > +test_expect_success 'validate the output.' ' > + compare_diff_patch current expected > +'
In addition, in "modern" style, it is more common to name the file that the actual output goes "actual", and the file that has the expected contents "expect", and compare "expect" with "actual". This test has compared contents in two files with wrong names, and compares them in a wrong order.
Taking all together, it would look more like this, I would imagine. Of course as "expected" has been renamed to "expect" in the initial set-up part, the fallouts in the remainder of the test script also needs to be dealt with, which is left as an exercise to the reader.
t/t4001-diff-rename.sh | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-)
diff --git c/t/t4001-diff-rename.sh w/t/t4001-diff-rename.sh index ad474100af..61d651d1db 100755 --- c/t/t4001-diff-rename.sh +++ w/t/t4001-diff-rename.sh @@ -26,7 +26,7 @@ test_expect_success 'setup' ' Line 14 Line 15 EOF - cat >expected <<-\EOF && + cat >expect <<-\EOF && diff --git a/path0 b/path1 rename from path0 rename to path1 @@ -88,28 +88,19 @@ test_expect_success 'setup' ' EOF ' -test_expect_success \ - 'update-index --add a file.' \ - 'git update-index --add path0' - -test_expect_success \ - 'write that tree.' \ - 'tree=$(git write-tree) && echo $tree' +test_expect_success 'path0 renamed to path1 with minor edit' ' + git update-index --add path0 && + tree=$(git write-tree) && -sed -e 's/line/Line/' <path0 >path1 -rm -f path0 -test_expect_success \ - 'renamed and edited the file.' \ - 'git update-index --add --remove path0 path1' + # edit and rename + sed -e 's/line/Line/' <path0 >path1 && + rm -f path0 && + git update-index --add --remove path0 path1 && -test_expect_success \ - 'git diff-index -p -M after rename and editing.' \ - 'git diff-index -p -M $tree >current' + git diff-index -p -M $tree >actual && - -test_expect_success \ - 'validate the output.' \ - 'compare_diff_patch current expected' + compare_diff_patch expect actual +' test_expect_success 'test diff.renames=true' ' git -c diff.renames=true diff --cached $tree >current &&