On Sun, Aug 12, 2018 at 12:07 AM William Chargin <wchargin@xxxxxxxxx> wrote:
> While the `test_dir_is_empty` function appears correct in most normal
> use cases, it can fail when filenames contain newlines. This patch
> changes the implementation to check that the output of `ls -a` has at
> most two lines (for `.` and `..`), which should be better behaved.
>
> The newly added unit test fails before this change and passes after it.
>
> Signed-off-by: William Chargin <wchargin@xxxxxxxxx>
> ---
> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> @@ -821,6 +821,37 @@ test_expect_success 'tests clean up even on failures' "
> +test_expect_success FUNNYNAMES \
> + 'test_dir_is_empty behaves even in pathological cases' "
> + test_expect_success 'should fail with a normal filename' '
> + mkdir nonempty_dir &&
> + touch nonempty_dir/some_file &&
We usually avoid "touch" unless the timestamp of the file is
significant. In this case, it isn't, so it would be more idiomatic to
say simply:
>nonempty_dir/some_file &&
> + test_must_fail test_dir_is_empty nonempty_dir
This is an abuse of test_must_fail() which is intended strictly for
testing 'git' invocations which might fail for reasons other than the
expected one (for instance, git might crash). Instead, you should just
use '!', like this:
! test_dir_is_empty nonempty_dir
> + test_expect_success 'should fail with dot-newline-dot filename' '
> + mkdir pathological_dir &&
> + touch \"pathological_dir/.
> + .\" &&
> + test_must_fail test_dir_is_empty pathological_dir
Same comments as above.
