Re: [RFC/PATCH 1/4] test-lib: add test_dir_is_empty()

2014-03-26 Thread Michael Haggerty
On 03/26/2014 09:29 AM, Jens Lehmann wrote:
 Am 25.03.2014 21:49, schrieb Junio C Hamano:
 Jens Lehmann jens.lehm...@web.de writes:

 For the upcoming submodule test framework we often need to assert that an
 empty directory exists in the work tree. Add the test_dir_is_empty()
 function which asserts that the given argument is an empty directory.

 Signed-off-by: Jens Lehmann jens.lehm...@web.de
 ---

 I believe this one is pretty straightforward (unless I missed that this
 functionality already exists someplace I forgot to look ;-).

 I am not very thrilled to see that it depends on . and .. to
 always exist, which may be true for all POSIX filesystems, but
 still...
 
 Agreed. I didn't find any one-liners to do that (ls -A isn't
 POSIX), so I decided to wrap that in a function. Testing that
 rmdir on the directory succeeds (because the directory is
 empty) would kinda work, but then we'd have to re-create the
 directory afterwards, which really doesn't sound like a good
 strategy either as the test would manipulate the to-be-tested
 object. I'm not terribly happy with depending on . and ..
 either, but couldn't come up with something better. At least
 the test should fail for any filesystem not having the dot
 files ...
 
 Do expected callsites of this helper care if $1 is a symbolic link
 that points at an empty directory?
 
 Yep, a symbolic link pointing to an empty directory should make
 the test fail.
 
 What do expected callsites really want to ensure?  In other words,
 why do they care if the directory is empty?  Is it to make sure,
 after some operation, they can rmdir the directory?
 
 To assert that a submodule is created but *not* populated. This
 is intended to catch any possible fallout from the recursive
 checkout later, in case that would kick in when it shouldn't.
 
  t/test-lib-functions.sh | 11 +++
  1 file changed, 11 insertions(+)

 diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
 index 158e10a..93d10cd 100644
 --- a/t/test-lib-functions.sh
 +++ b/t/test-lib-functions.sh
 @@ -489,6 +489,17 @@ test_path_is_dir () {
 fi
  }

 +# Check if the directory exists and is empty as expected, barf otherwise.
 +test_dir_is_empty () {
 +   test_path_is_dir $1 
 +   if test $(ls -a1 $1 | wc -l) != 2
 +   then
 +   echo Directory '$1' is not empty, it contains:
 +   ls -la $1
 +   return 1
 +   fi
 +}
 +
  test_path_is_missing () {
 if [ -e $1 ]
 then

Why not do something like

test -z $(ls -a1 $1 | egrep -v '^\.\.?$')

I.e., make the test ignore . and .. without depending on their
existence?

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH 1/4] test-lib: add test_dir_is_empty()

2014-03-26 Thread David Kastrup
Junio C Hamano gits...@pobox.com writes:

 Jens Lehmann jens.lehm...@web.de writes:

 For the upcoming submodule test framework we often need to assert that an
 empty directory exists in the work tree. Add the test_dir_is_empty()
 function which asserts that the given argument is an empty directory.

 Signed-off-by: Jens Lehmann jens.lehm...@web.de
 ---

 I believe this one is pretty straightforward (unless I missed that this
 functionality already exists someplace I forgot to look ;-).

 I am not very thrilled to see that it depends on . and .. to
 always exist, which may be true for all POSIX filesystems, but
 still...

Not even there, though few people will likely use / as their work
tree...

-- 
David Kastrup
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH 1/4] test-lib: add test_dir_is_empty()

2014-03-26 Thread Jens Lehmann
Am 26.03.2014 11:43, schrieb Michael Haggerty:
 On 03/26/2014 09:29 AM, Jens Lehmann wrote:
 Am 25.03.2014 21:49, schrieb Junio C Hamano:
 Jens Lehmann jens.lehm...@web.de writes:
  t/test-lib-functions.sh | 11 +++
  1 file changed, 11 insertions(+)

 diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
 index 158e10a..93d10cd 100644
 --- a/t/test-lib-functions.sh
 +++ b/t/test-lib-functions.sh
 @@ -489,6 +489,17 @@ test_path_is_dir () {
fi
  }

 +# Check if the directory exists and is empty as expected, barf otherwise.
 +test_dir_is_empty () {
 +  test_path_is_dir $1 
 +  if test $(ls -a1 $1 | wc -l) != 2
 +  then
 +  echo Directory '$1' is not empty, it contains:
 +  ls -la $1
 +  return 1
 +  fi
 +}
 +
  test_path_is_missing () {
if [ -e $1 ]
then
 
 Why not do something like
 
 test -z $(ls -a1 $1 | egrep -v '^\.\.?$')
 
 I.e., make the test ignore . and .. without depending on their
 existence?

Thanks, will do so in the next round.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH 1/4] test-lib: add test_dir_is_empty()

2014-03-25 Thread Junio C Hamano
Jens Lehmann jens.lehm...@web.de writes:

 For the upcoming submodule test framework we often need to assert that an
 empty directory exists in the work tree. Add the test_dir_is_empty()
 function which asserts that the given argument is an empty directory.

 Signed-off-by: Jens Lehmann jens.lehm...@web.de
 ---

 I believe this one is pretty straightforward (unless I missed that this
 functionality already exists someplace I forgot to look ;-).

I am not very thrilled to see that it depends on . and .. to
always exist, which may be true for all POSIX filesystems, but
still...

Do expected callsites of this helper care if $1 is a symbolic link
that points at an empty directory?

What do expected callsites really want to ensure?  In other words,
why do they care if the directory is empty?  Is it to make sure,
after some operation, they can rmdir the directory?

  t/test-lib-functions.sh | 11 +++
  1 file changed, 11 insertions(+)

 diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
 index 158e10a..93d10cd 100644
 --- a/t/test-lib-functions.sh
 +++ b/t/test-lib-functions.sh
 @@ -489,6 +489,17 @@ test_path_is_dir () {
   fi
  }

 +# Check if the directory exists and is empty as expected, barf otherwise.
 +test_dir_is_empty () {
 + test_path_is_dir $1 
 + if test $(ls -a1 $1 | wc -l) != 2
 + then
 + echo Directory '$1' is not empty, it contains:
 + ls -la $1
 + return 1
 + fi
 +}
 +
  test_path_is_missing () {
   if [ -e $1 ]
   then
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html