threads / patch / 24363

patcht3000 (ls-files -o): modernize style

Subject: [PATCH] t3000 (ls-files -o): modernize style

## tl;dr

4 messages between Jul 11, 2010 and Jul 11, 2010. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Jonathan Nieder· Jul 11, 2010, 04:20 UTC · lore

This script is part of the second batch of tests, from the same day the test infrastructure was added to git. Update it to use a more modern style in the spirit of v1.6.4-rc0~45^2~2 (2009-05-22). In particular:

 - Put setup code inside test assertions, to avoid unexpected
   breakages and avoid stray output without -v (as t/README
   recommends); and
 - Put the test title on the same line as the "test_expect_success",
   and end the line with a single-quote to begin the body of the test
   which is one multi-line string.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t3000-ls-files-others.sh |   99 +++++++++++++++++++++-----------------------
 1 files changed, 47 insertions(+), 52 deletions(-)
Show changes to t/t3000-ls-files-others.sh +47 −52
diff --git a/t/t3000-ls-files-others.sh b/t/t3000-ls-files-others.sh
index 86291e8..2eec011 100755
--- a/t/t3000-ls-files-others.sh
+++ b/t/t3000-ls-files-others.sh
@@ -17,57 +17,52 @@ filesystem.
 '
 . ./test-lib.sh
 
-date >path0
-if test_have_prereq SYMLINKS
-then
-	ln -s xyzzy path1
-else
-	date > path1
-fi
-mkdir path2 path3 path4
-date >path2/file2
-date >path2-junk
-date >path3/file3
-date >path3-junk
-git update-index --add path3-junk path3/file3
-
+test_expect_success 'setup ' '
+	date >path0 &&
+	if test_have_prereq SYMLINKS
+	then
+		ln -s xyzzy path1
+	else
+		date >path1
+	fi &&
+	mkdir path2 path3 path4 &&
+	date >path2/file2 &&
+	date >path2-junk &&
+	date >path3/file3 &&
+	date >path3-junk &&
+	git update-index --add path3-junk path3/file3
+'
+
-cat >expected1 <<EOF
-expected1
-expected2
-expected3
-output
-path0
-path1
-path2-junk
-path2/file2
-EOF
-sed -e 's|path2/file2|path2/|' <expected1 >expected2
-cat <expected2 >expected3
-echo path4/ >>expected2
-
+test_expect_success 'setup: expected output' '
+	cat >expected1 <<-\EOF &&
+	expected1
+	expected2
+	expected3
+	output
+	path0
+	path1
+	path2-junk
+	path2/file2
+	EOF
+
+	sed -e "s|path2/file2|path2/|" <expected1 >expected2 &&
+	cp expected2 expected3 &&
+	echo path4/ >>expected2
+'
+
-test_expect_success \
-    'git ls-files --others to show output.' \
-    'git ls-files --others >output'
-
-test_expect_success \
-    'git ls-files --others should pick up symlinks.' \
-    'test_cmp expected1 output'
-
+test_expect_success 'ls-files --others' '
+	git ls-files --others >output &&
+	test_cmp expected1 output
+'
+
-test_expect_success \
-    'git ls-files --others --directory to show output.' \
-    'git ls-files --others --directory >output'
-
-
-test_expect_success \
-    'git ls-files --others --directory should not get confused.' \
-    'test_cmp expected2 output'
-
+test_expect_success 'ls-files --others --directory' '
+	git ls-files --others --directory >output &&
+	test_cmp expected2 output
+'
+
-test_expect_success \
-    'git ls-files --others --directory --no-empty-directory to show output.' \
-    'git ls-files --others --directory --no-empty-directory >output'
-
-test_expect_success \
-    '--no-empty-directory hides empty directory' \
-    'test_cmp expected3 output'
+test_expect_success '--no-empty-directory hides empty directory' '
+	git ls-files --others --directory --no-empty-directory >output &&
+	test_cmp expected3 output
+'
 
 test_done
-- 
1.7.2.rc2
Jakub Narebski· Jul 11, 2010, 08:33 UTC · re: Jonathan Nieder · lore

Re: [PATCH] t3000 (ls-files -o): modernize style

Jonathan Nieder <jrnieder@gmail.com> writes:
Show 8 quoted lines
> This script is part of the second batch of tests, from the same day
> the test infrastructure was added to git.  Update it to use a more
> modern style in the spirit of v1.6.4-rc0~45^2~2 (2009-05-22).
> In particular:
> 
>  - Put setup code inside test assertions, to avoid unexpected
>    breakages and avoid stray output without -v (as t/README
>    recommends); and
O.K., but...
 
Show 30 quoted lines
> -cat >expected1 <<EOF
> -expected1
> -expected2
> -expected3
> -output
> -path0
> -path1
> -path2-junk
> -path2/file2
> -EOF
> -sed -e 's|path2/file2|path2/|' <expected1 >expected2
> -cat <expected2 >expected3
> -echo path4/ >>expected2
> -
> +test_expect_success 'setup: expected output' '
> +	cat >expected1 <<-\EOF &&
> +	expected1
> +	expected2
> +	expected3
> +	output
> +	path0
> +	path1
> +	path2-junk
> +	path2/file2
> +	EOF
> +
> +	sed -e "s|path2/file2|path2/|" <expected1 >expected2 &&
> +	cp expected2 expected3 &&
> +	echo path4/ >>expected2
> +'

... I don't think it is really needed. There isn't anything there that can fail, and there isn't anything there that produces any output.

BTW. do all sane shells that can be used to run test suite support '<<-' here-doc redirection operators? Is it in POSIX?

-- 
Jakub Narebski
Poland
ShadeHawk on #git
Ævar Arnfjörð Bjarmason· Jul 11, 2010, 09:10 UTC · re: Jakub Narebski · lore

Re: [PATCH] t3000 (ls-files -o): modernize style

On Sun, Jul 11, 2010 at 08:33, Jakub Narebski <jnareb@gmail.com> wrote:
Show 6 quoted lines
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> This script is part of the second batch of tests, from the same day
>> the test infrastructure was added to git.  Update it to use a more
>> modern style in the spirit of v1.6.4-rc0~45^2~2 (2009-05-22).
>> In particular:
On the patch:
    Acked-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Show 40 quoted lines
>>  - Put setup code inside test assertions, to avoid unexpected
>>    breakages and avoid stray output without -v (as t/README
>>    recommends); and
>
> O.K., but...
>
>> -cat >expected1 <<EOF
>> -expected1
>> -expected2
>> -expected3
>> -output
>> -path0
>> -path1
>> -path2-junk
>> -path2/file2
>> -EOF
>> -sed -e 's|path2/file2|path2/|' <expected1 >expected2
>> -cat <expected2 >expected3
>> -echo path4/ >>expected2
>> -
>> +test_expect_success 'setup: expected output' '
>> +     cat >expected1 <<-\EOF &&
>> +     expected1
>> +     expected2
>> +     expected3
>> +     output
>> +     path0
>> +     path1
>> +     path2-junk
>> +     path2/file2
>> +     EOF
>> +
>> +     sed -e "s|path2/file2|path2/|" <expected1 >expected2 &&
>> +     cp expected2 expected3 &&
>> +     echo path4/ >>expected2
>> +'
>
> ... I don't think it is really needed.  There isn't anything there
> that can fail, and there isn't anything there that produces any
> output.

The disk could fill up. In any case I like the test style of using setup tests instead of stray setup code inside the tests themselves.

> BTW. do all sane shells that can be used to run test suite support
> '<<-' here-doc redirection operators?  Is it in POSIX?

I don't know. But it's POSIX enough that nobody's complained about it thus far, it's used in a lot of existing code:

    $ ack -l '<<-EOF'
    t/t1012-read-tree-df.sh
    t/t7005-editor.sh
    t/t3507-cherry-pick-conflict.sh
    t/t7401-submodule-summary.sh
    t/t7201-co.sh
    t/t4132-apply-removal.sh
    t/t4041-diff-submodule-option.sh
    contrib/hooks/post-receive-email
Jonathan Nieder· Jul 11, 2010, 15:14 UTC · re: Jakub Narebski · lore

Re: [PATCH] t3000 (ls-files -o): modernize style

Jakub Narebski wrote:
> ... I don't think it is really needed.  There isn't anything there
> that can fail, and there isn't anything there that produces any
> output.

I should clarify. I wrote this patch as preparation to add some new tests to the same script. The new tests are not cooked yet, so I just sent the trivial part; there certainly is no urgent reason to apply it soon (though no reason I see not to apply it either).

A better explanation for the rationale would be "follow the modern style to make the script more readable and set a good example for future tests".

> BTW. do all sane shells that can be used to run test suite support
> '<<-' here-doc redirection operators?  Is it in POSIX?
Yes[1] and yes[2].

Thanks for looking it over. Jonathan

[1] $ git grep -l -F -e '<<-EO' -e '<<-\EO' origin/maint -- t | wc -l
    13
[2] http://www.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_07_04

← back to recent threads