Re: [PATCH 1/1] t/test-lib: make `test_dir_is_empty` more robust

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

William Chargin wrote:

> Subject: t/test-lib: make `test_dir_is_empty` more robust

This subject line will appear out of context in "git shortlog" output,
so it's useful to pack in enough information to briefly summarize what
the change does.  How about something like

	test_dir_is_empty: avoid being confused by $'.\n.' file

or

	test_dir_is_empty: simplify by using "ls --almost-all"

?

[...]
> +	test_expect_success 'should fail with dot-newline-dot filename' '
> +		mkdir pathological_dir &&
> +		printf \"pathological_dir/.\\\\n.\\\\0\" | xargs -0 touch &&

I don't think "xargs -0" is present on all supported platforms.  I'd
be tempted to use

		>pathological_dir/$'.\n.'

but $'' is too recent of a shell feature to count on (e.g., dash doesn't
support it).  See t/t3600-rm.sh for an example of a portable way to do
this.

Not all filesystems support newlines in filenames.  I think we'd want
to factor out the FUNNYNAMES prereq from there into a test_lazy_prereq
so this test can be skipped on such filenames.

[...]
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -568,7 +568,7 @@ test_path_is_dir () {
>  # Check if the directory exists and is empty as expected, barf otherwise.
>  test_dir_is_empty () {
>  	test_path_is_dir "$1" &&
> -	if test -n "$(ls -a1 "$1" | egrep -v '^\.\.?$')"
> +	if test "$(ls -A1 "$1" | wc -c)" != 0

Another portability gotcha: wc output includes a space on Mac so this
test would always return true there.  How about

	if test -n "$(ls -A1 "$1")"

?

"ls -A" was added in POSIX.1-2017.  Its rationale section explains

	Earlier versions of this standard did not describe the BSD -A
	option (like -a, but dot and dot-dot are not written out). It
	has been added due to widespread implementation.

That's very recent, but the widespread implementation it mentions is
less so.  This would be our first use of "ls -A", so I'd be interested
to hear from people on more obscure platforms.  It does seem to be
widespread.

Can you say a little more about where you ran into this?  Did you
discover it by code inspection?

I do think the resulting implementation using -A is simpler.  Thanks
for writing it.

Thanks and hope that helps,
Jonathan



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux