From: "Ævar Arnfjörð Bjarmason" <avarab@gmail•com>
To: "SZEDER Gábor" <szeder.dev@gmail•com>
Cc: Elijah Newren <newren@gmail•com>, Mooga <contact@m-mooga•com>,
Git Mailing List <git@vger•kernel.org>
Subject: Re: [GSoC] microporject test_path_is_*
Date: Wed, 27 Mar 2019 15:10:42 +0100 [thread overview]
Message-ID: <8736n8be71.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <20190327121411.GE32732@szeder.dev>
On Wed, Mar 27 2019, SZEDER Gábor wrote:
> On Wed, Mar 27, 2019 at 12:21:55PM +0100, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Wed, Mar 27 2019, SZEDER Gábor wrote:
>>
>> > On Wed, Mar 27, 2019 at 11:09:18AM +0100, Ævar Arnfjörð Bjarmason wrote:
>> >> > There are likewise several that use one of
>> >> > ! test -e path/to/filename
>> >> > or
>> >> > ! test -f path/to/filename
>> >> > or
>> >> > test ! -f path/to/filename
>> >> > which could be replaced by
>> >> > test_path_is_missing path/to/filename
>> >>
>> >> Interesting that for some we use the 'test_is_there/test_is_not_there'
>> >> pattern and for others 'test_is_there [!]'. E.g
>> >> test_path_exist/test_path_is_missing v.s. test_i18ngrep.
>> >
>> > It's unclear what the '!' should negate in case of 'test_path_is_file
>> > ! file'. What if 'file' does exists, but it's not a file but a
>> > directory, socket, fifo, or symlink? 'test ! -f file' returns success
>> > in these cases as well.
>> >
>> > OTOH, it's quite clear what the negation should mean in case of
>> > 'test_i18ngrep'.
>>
>> *Should* we make it better? Yeah sure, maybe. I'm just pointing out for
>> context to someone poking at this for the first time that now we
>> sometimes do "! foo <arg>" v.s. "foo <arg>" as "foo_is <arg>" and
>> "foo_not <arg>" and other times "foo [!] <arg>".
>>
>> So yeah, maybe we should improve things to disambiguate the cases you
>> mentioned, but right now e.g. "test_path_exists" and
>> "test_path_is_missing" are just "test -e" and "! test -e", respectively.
>
> I'm not sure why 'test_path_exists' exists, as I don't readily see a
> valid reason why a test should only be interested in whether the path
> exists, but but not whether it's a file or a directory.
In the general case the same reason we use "test -e". While the test
would pass in all sorts of unexpected cases, those probably aren't
plausible and we're just e.g. checking "did the thing create a file
it'll create in XYZ mode?"....
> Luckily it
> haven't caught on, and it's only used twice in the whole test suite.
Well, we have some >100 "test -e" though ... :)
> However, as shown above, the existend of 'test_path_is_missing' is
> very much justified.
>
>> The same caveats you've mentioned also apply to "test_i18ngrep" b.t.w.,
>> there we squash the 3x standard exit codes of grep[1] into a boolean,
>> and thus e.g. ignore the difference between <file> not matching an
>> <file> being a directory or whatever.
>
> I'm not sure I understand, 'test_i18ngrep' handles the missing file or
> not a file cases reasonably well:
>
> expecting success:
> test_i18ngrep ! foo nonexistent-file
>
> error: bug in the test script: test_i18ngrep requires a file to read as the last parameter
>
> or
>
> expecting success:
> mkdir dir &&
> test_i18ngrep ! foo dir
>
> error: bug in the test script: test_i18ngrep requires a file to read as the last parameter
Yeah you're right, I didn't read it carefully enough and it does handle
*that* particular case, but e.g. a grep of "file" where we can't read it
due to a permission error is the same as "didn't contain the string".
I'm not saying that's a bad thing, but the opposite. We assume a certain
base level of sanity, e.g. we do "test_must_fail <cmd>" only for git,
but "! <cmd>" for everything else, even though e.g. the system "grep"
may be segfaulting.
>>
>> 1. http://pubs.opengroup.org/onlinepubs/009695399/utilities/grep.html
next prev parent reply other threads:[~2019-03-27 14:10 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-26 21:07 [GSoC] microporject test_path_is_* Mooga
2019-03-26 22:06 ` Elijah Newren
2019-03-27 10:09 ` Ævar Arnfjörð Bjarmason
2019-03-27 10:49 ` SZEDER Gábor
2019-03-27 11:21 ` Ævar Arnfjörð Bjarmason
2019-03-27 12:14 ` SZEDER Gábor
2019-03-27 14:10 ` Ævar Arnfjörð Bjarmason [this message]
2019-03-28 9:58 ` SZEDER Gábor
2019-03-27 11:28 ` Mooga
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8736n8be71.fsf@evledraar.gmail.com \
--to=avarab@gmail$(echo .)com \
--cc=contact@m-mooga$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=newren@gmail$(echo .)com \
--cc=szeder.dev@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox