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
