Re: [PATCH 1/1] tests: replace `test -(d|f)` with test_path_is_(dir|file)

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

 



On Tue, 26 Feb 2019 at 14:43, Rohit Ashiwal via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
>
> t3600-rm.sh: Previously we were using `test -(d|f)`
> to verify the presencee of a directory/file, but we
> already have helper functions, viz, test_path_is_dir
> and test_path_is_file with same functionality. This
> patch will replace `test -(d|f)` calls in t3600-rm.sh.

I think this makes a lot of sense. If a test breaks, we'll get some
helpful error message. Thank you for working on this.

> -       ! test -d submod &&
> +       ! test_path_is_dir submod &&

Now, here I wonder. This (and other changes like this) means that every
time the test passes, we see "Directory submod doesn't exist.", which is
perhaps not too irritating. But more importantly, when the test fails,
we don't get any hint. So a failure is just as silent and "non-helpful"
as before. I can think of a few approaches:

 1 Teach `test_path_is_dir` and friends to handle "!" in a clever way, and
   write these as `test_path_is_dir ! foo`. (We already have helpers
   that do this, see, e.g., `test_i18ngrep`.)

 2 Don't be clever, and just introduce `test_path_is_not_dir`.

 3 Don't bother, because this small change here doesn't make the error
   case any worse.

 4 Don't do this small change here, and leave cases like this for a
   later change (something like 1 or 2 above).

What do you think?

There are a few of these "!". The other changes look good to me.

Cheers
Martin



[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