{"thread":{"id":"50823","subject":"[GSoC] microporject test_path_is_*","startedAt":"2019-03-26T21:08:40Z","lastAt":"2019-03-28T09:58:47Z","messageCount":9,"participants":["Mooga","Elijah Newren","Ævar Arnfjörð Bjarmason","SZEDER Gábor"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"372548","messageId":"38B8E527-2338-485E-8840-29121F259687@m-mooga.com","threadId":"50823","inReplyTo":null,"subject":"[GSoC] microporject test_path_is_*","fromName":"Mooga","fromEmail":"contact@m-mooga.com","sentAt":"2019-03-26T21:07:19Z","receivedAt":"2019-03-26T21:08:40Z","isPatch":false,"sender":{"key":"contact@m-mooga.com","avatar":null},"body":"Hi, \nI am still a bit confused about the task itself \n\nit’s just text replacing for example: \nt1400-update-ref.sh , line 194 -> `test_path_is_missing` has to be ‘test_path_is_file’ \n\nThanks\n\n\n"},{"id":"372550","messageId":"CABPp-BHwUoqk79Kf=ynna5x+mCJyOLz66v6pDieyEeM7YCRS+g@mail.gmail.com","threadId":"50823","inReplyTo":"38B8E527-2338-485E-8840-29121F259687@m-mooga.com","subject":"Re: [GSoC] microporject test_path_is_*","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-03-26T22:06:36Z","receivedAt":"2019-03-26T22:06:50Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Tue, Mar 26, 2019 at 2:10 PM Mooga wrote:\n>\n> Hi,\n> I am still a bit confused about the task itself\n>\n> it’s just text replacing for example:\n> t1400-update-ref.sh , line 194 -> `test_path_is_missing` has to be ‘test_path_is_file’\n>\n> Thanks\n\nThere are several places in the code that use test with -e or -f or -d\n(or -h or...) in order to check for the presence of a\nfile/directory/symlink/etc. For example,\n test -f path1/file1\nThis could be made more clear and produce nicer error messages if it\nwere instead\n test_path_is_file path1/file1\n\nThere are likewise several that use one of\n ! test -e path/to/filename\nor\n ! test -f path/to/filename\nor\n test ! -f path/to/filename\nwhich could be replaced by\n test_path_is_missing path/to/filename\n\nThis GSoC microproject is just about picking one testfile that has\nsome of these constructs, and fixing the cases found within that\ntestfile.\n"},{"id":"372562","messageId":"877eckbpdd.fsf@evledraar.gmail.com","threadId":"50823","inReplyTo":"CABPp-BHwUoqk79Kf=ynna5x+mCJyOLz66v6pDieyEeM7YCRS+g@mail.gmail.com","subject":"Re: [GSoC] microporject test_path_is_*","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-03-27T10:09:18Z","receivedAt":"2019-03-27T10:09:23Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Mar 26 2019, Elijah Newren wrote:\n\n> Hi,\n>\n> On Tue, Mar 26, 2019 at 2:10 PM Mooga wrote:\n>>\n>> Hi,\n>> I am still a bit confused about the task itself\n>>\n>> it’s just text replacing for example:\n>> t1400-update-ref.sh , line 194 -> `test_path_is_missing` has to be ‘test_path_is_file’\n>>\n>> Thanks\n>\n> There are several places in the code that use test with -e or -f or -d\n> (or -h or...) in order to check for the presence of a\n> file/directory/symlink/etc. For example,\n> test -f path1/file1\n> This could be made more clear and produce nicer error messages if it\n> were instead\n> test_path_is_file path1/file1\n\nSee also the recent thread I started\nhttps://public-inbox.org/git/87sgwav8cp.fsf@evledraar.gmail.com/ asking\nif these wrappers were useless now. The consensus was to keep them (a\nbunch of use-cases I didn't know about). Useful if you're poking at them\nand wondering why we're using this / what it gives us.\n\n> There are likewise several that use one of\n> ! test -e path/to/filename\n> or\n> ! test -f path/to/filename\n> or\n> test ! -f path/to/filename\n> which could be replaced by\n> test_path_is_missing path/to/filename\n\nInteresting that for some we use the 'test_is_there/test_is_not_there'\npattern and for others 'test_is_there [!]'. E.g\ntest_path_exist/test_path_is_missing v.s. test_i18ngrep.\n\n> This GSoC microproject is just about picking one testfile that has\n> some of these constructs, and fixing the cases found within that\n> testfile.\n"},{"id":"372568","messageId":"20190327104935.GD32732@szeder.dev","threadId":"50823","inReplyTo":"877eckbpdd.fsf@evledraar.gmail.com","subject":"Re: [GSoC] microporject test_path_is_*","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-03-27T10:49:35Z","receivedAt":"2019-03-27T10:49:42Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Mar 27, 2019 at 11:09:18AM +0100, Ævar Arnfjörð Bjarmason wrote:\n> > There are likewise several that use one of\n> > ! test -e path/to/filename\n> > or\n> > ! test -f path/to/filename\n> > or\n> > test ! -f path/to/filename\n> > which could be replaced by\n> > test_path_is_missing path/to/filename\n> \n> Interesting that for some we use the 'test_is_there/test_is_not_there'\n> pattern and for others 'test_is_there [!]'. E.g\n> test_path_exist/test_path_is_missing v.s. test_i18ngrep.\n\nIt's unclear what the '!' should negate in case of 'test_path_is_file\n! file'. What if 'file' does exists, but it's not a file but a\ndirectory, socket, fifo, or symlink? 'test ! -f file' returns success\nin these cases as well.\n\nOTOH, it's quite clear what the negation should mean in case of\n'test_i18ngrep'.\n\n"},{"id":"372569","messageId":"874l7obm0c.fsf@evledraar.gmail.com","threadId":"50823","inReplyTo":"20190327104935.GD32732@szeder.dev","subject":"Re: [GSoC] microporject test_path_is_*","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-03-27T11:21:55Z","receivedAt":"2019-03-27T11:22:00Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Mar 27 2019, SZEDER Gábor wrote:\n\n> On Wed, Mar 27, 2019 at 11:09:18AM +0100, Ævar Arnfjörð Bjarmason wrote:\n>> > There are likewise several that use one of\n>> > ! test -e path/to/filename\n>> > or\n>> > ! test -f path/to/filename\n>> > or\n>> > test ! -f path/to/filename\n>> > which could be replaced by\n>> > test_path_is_missing path/to/filename\n>>\n>> Interesting that for some we use the 'test_is_there/test_is_not_there'\n>> pattern and for others 'test_is_there [!]'. E.g\n>> test_path_exist/test_path_is_missing v.s. test_i18ngrep.\n>\n> It's unclear what the '!' should negate in case of 'test_path_is_file\n> ! file'. What if 'file' does exists, but it's not a file but a\n> directory, socket, fifo, or symlink? 'test ! -f file' returns success\n> in these cases as well.\n>\n> OTOH, it's quite clear what the negation should mean in case of\n> 'test_i18ngrep'.\n\n*Should* we make it better? Yeah sure, maybe. I'm just pointing out for\ncontext to someone poking at this for the first time that now we\nsometimes do \"! foo \" v.s. \"foo \" as \"foo_is \" and\n\"foo_not \" and other times \"foo [!] \".\n\nSo yeah, maybe we should improve things to disambiguate the cases you\nmentioned, but right now e.g. \"test_path_exists\" and\n\"test_path_is_missing\" are just \"test -e\" and \"! test -e\", respectively.\n\nThe same caveats you've mentioned also apply to \"test_i18ngrep\" b.t.w.,\nthere we squash the 3x standard exit codes of grep[1] into a boolean,\nand thus e.g. ignore the difference between not matching an\n being a directory or whatever.\n\n1. http://pubs.opengroup.org/onlinepubs/009695399/utilities/grep.html\n"},{"id":"372570","messageId":"BBF4F6A7-E5CA-4493-B386-AB7A69EA6EFD@m-mooga.com","threadId":"50823","inReplyTo":"CABPp-BHwUoqk79Kf=ynna5x+mCJyOLz66v6pDieyEeM7YCRS+g@mail.gmail.com","subject":"Re: [GSoC] microporject test_path_is_*","fromName":"Mooga","fromEmail":"contact@m-mooga.com","sentAt":"2019-03-27T11:28:49Z","receivedAt":"2019-03-27T11:30:16Z","isPatch":false,"sender":{"key":"contact@m-mooga.com","avatar":null},"body":"\nSo for example that is git diff\n\ndiff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\nindex 286bba35d8..fc82965a0f 100755\n--- a/t/t2400-worktree-add.sh\n+++ b/t/t2400-worktree-add.sh\n@@ -66,7 +66,7 @@ test_expect_success '\"add\" worktree' '\ntest_expect_success '\"add\" worktree with lock' '\n git rev-parse HEAD >expect &&\n git worktree add --detach --lock here-with-lock master &&\n- test -f .git/worktrees/here-with-lock/locked\n+ test_path_is_file .git/worktrees/here-with-lock/locked\n\n\n\nOn 26.03.19, 23:07, \"Elijah Newren\" wrote:\n\n Hi,\n \n On Tue, Mar 26, 2019 at 2:10 PM Mooga wrote:\n >\n > Hi,\n > I am still a bit confused about the task itself\n >\n > it’s just text replacing for example:\n > t1400-update-ref.sh , line 194 -> `test_path_is_missing` has to be ‘test_path_is_file’\n >\n > Thanks\n \n There are several places in the code that use test with -e or -f or -d\n (or -h or...) in order to check for the presence of a\n file/directory/symlink/etc. For example,\n test -f path1/file1\n This could be made more clear and produce nicer error messages if it\n were instead\n test_path_is_file path1/file1\n \n There are likewise several that use one of\n ! test -e path/to/filename\n or\n ! test -f path/to/filename\n or\n test ! -f path/to/filename\n which could be replaced by\n test_path_is_missing path/to/filename\n \n This GSoC microproject is just about picking one testfile that has\n some of these constructs, and fixing the cases found within that\n testfile.\n \n\n\n"},{"id":"372571","messageId":"20190327121411.GE32732@szeder.dev","threadId":"50823","inReplyTo":"874l7obm0c.fsf@evledraar.gmail.com","subject":"Re: [GSoC] microporject test_path_is_*","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-03-27T12:14:11Z","receivedAt":"2019-03-27T12:14:18Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Mar 27, 2019 at 12:21:55PM +0100, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Wed, Mar 27 2019, SZEDER Gábor wrote:\n> \n> > On Wed, Mar 27, 2019 at 11:09:18AM +0100, Ævar Arnfjörð Bjarmason wrote:\n> >> > There are likewise several that use one of\n> >> > ! test -e path/to/filename\n> >> > or\n> >> > ! test -f path/to/filename\n> >> > or\n> >> > test ! -f path/to/filename\n> >> > which could be replaced by\n> >> > test_path_is_missing path/to/filename\n> >>\n> >> Interesting that for some we use the 'test_is_there/test_is_not_there'\n> >> pattern and for others 'test_is_there [!]'. E.g\n> >> test_path_exist/test_path_is_missing v.s. test_i18ngrep.\n> >\n> > It's unclear what the '!' should negate in case of 'test_path_is_file\n> > ! file'. What if 'file' does exists, but it's not a file but a\n> > directory, socket, fifo, or symlink? 'test ! -f file' returns success\n> > in these cases as well.\n> >\n> > OTOH, it's quite clear what the negation should mean in case of\n> > 'test_i18ngrep'.\n> \n> *Should* we make it better? Yeah sure, maybe. I'm just pointing out for\n> context to someone poking at this for the first time that now we\n> sometimes do \"! foo \" v.s. \"foo \" as \"foo_is \" and\n> \"foo_not \" and other times \"foo [!] \".\n> \n> So yeah, maybe we should improve things to disambiguate the cases you\n> mentioned, but right now e.g. \"test_path_exists\" and\n> \"test_path_is_missing\" are just \"test -e\" and \"! test -e\", respectively.\n\nI'm not sure why 'test_path_exists' exists, as I don't readily see a\nvalid reason why a test should only be interested in whether the path\nexists, but but not whether it's a file or a directory. Luckily it\nhaven't caught on, and it's only used twice in the whole test suite.\n\nHowever, as shown above, the existend of 'test_path_is_missing' is\nvery much justified.\n\n> The same caveats you've mentioned also apply to \"test_i18ngrep\" b.t.w.,\n> there we squash the 3x standard exit codes of grep[1] into a boolean,\n> and thus e.g. ignore the difference between not matching an\n> being a directory or whatever.\n\nI'm not sure I understand, 'test_i18ngrep' handles the missing file or\nnot a file cases reasonably well:\n\n expecting success: \n test_i18ngrep ! foo nonexistent-file\n \n error: bug in the test script: test_i18ngrep requires a file to read as the last parameter\n\nor\n\n expecting success: \n mkdir dir &&\n test_i18ngrep ! foo dir\n \n error: bug in the test script: test_i18ngrep requires a file to read as the last parameter\n\n\n> \n> 1. http://pubs.opengroup.org/onlinepubs/009695399/utilities/grep.html\n"},{"id":"372579","messageId":"8736n8be71.fsf@evledraar.gmail.com","threadId":"50823","inReplyTo":"20190327121411.GE32732@szeder.dev","subject":"Re: [GSoC] microporject test_path_is_*","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-03-27T14:10:42Z","receivedAt":"2019-03-27T14:10:48Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Mar 27 2019, SZEDER Gábor wrote:\n\n> On Wed, Mar 27, 2019 at 12:21:55PM +0100, Ævar Arnfjörð Bjarmason wrote:\n>>\n>> On Wed, Mar 27 2019, SZEDER Gábor wrote:\n>>\n>> > On Wed, Mar 27, 2019 at 11:09:18AM +0100, Ævar Arnfjörð Bjarmason wrote:\n>> >> > There are likewise several that use one of\n>> >> > ! test -e path/to/filename\n>> >> > or\n>> >> > ! test -f path/to/filename\n>> >> > or\n>> >> > test ! -f path/to/filename\n>> >> > which could be replaced by\n>> >> > test_path_is_missing path/to/filename\n>> >>\n>> >> Interesting that for some we use the 'test_is_there/test_is_not_there'\n>> >> pattern and for others 'test_is_there [!]'. E.g\n>> >> test_path_exist/test_path_is_missing v.s. test_i18ngrep.\n>> >\n>> > It's unclear what the '!' should negate in case of 'test_path_is_file\n>> > ! file'. What if 'file' does exists, but it's not a file but a\n>> > directory, socket, fifo, or symlink? 'test ! -f file' returns success\n>> > in these cases as well.\n>> >\n>> > OTOH, it's quite clear what the negation should mean in case of\n>> > 'test_i18ngrep'.\n>>\n>> *Should* we make it better? Yeah sure, maybe. I'm just pointing out for\n>> context to someone poking at this for the first time that now we\n>> sometimes do \"! foo \" v.s. \"foo \" as \"foo_is \" and\n>> \"foo_not \" and other times \"foo [!] \".\n>>\n>> So yeah, maybe we should improve things to disambiguate the cases you\n>> mentioned, but right now e.g. \"test_path_exists\" and\n>> \"test_path_is_missing\" are just \"test -e\" and \"! test -e\", respectively.\n>\n> I'm not sure why 'test_path_exists' exists, as I don't readily see a\n> valid reason why a test should only be interested in whether the path\n> exists, but but not whether it's a file or a directory.\n\nIn the general case the same reason we use \"test -e\". While the test\nwould pass in all sorts of unexpected cases, those probably aren't\nplausible and we're just e.g. checking \"did the thing create a file\nit'll create in XYZ mode?\"....\n\n> Luckily it\n> haven't caught on, and it's only used twice in the whole test suite.\n\nWell, we have some >100 \"test -e\" though ... :)\n\n> However, as shown above, the existend of 'test_path_is_missing' is\n> very much justified.\n>\n>> The same caveats you've mentioned also apply to \"test_i18ngrep\" b.t.w.,\n>> there we squash the 3x standard exit codes of grep[1] into a boolean,\n>> and thus e.g. ignore the difference between not matching an\n>> being a directory or whatever.\n>\n> I'm not sure I understand, 'test_i18ngrep' handles the missing file or\n> not a file cases reasonably well:\n>\n> expecting success:\n> test_i18ngrep ! foo nonexistent-file\n>\n> error: bug in the test script: test_i18ngrep requires a file to read as the last parameter\n>\n> or\n>\n> expecting success:\n> mkdir dir &&\n> test_i18ngrep ! foo dir\n>\n> error: bug in the test script: test_i18ngrep requires a file to read as the last parameter\n\nYeah you're right, I didn't read it carefully enough and it does handle\n*that* particular case, but e.g. a grep of \"file\" where we can't read it\ndue to a permission error is the same as \"didn't contain the string\".\n\nI'm not saying that's a bad thing, but the opposite. We assume a certain\nbase level of sanity, e.g. we do \"test_must_fail \" only for git,\nbut \"! \" for everything else, even though e.g. the system \"grep\"\nmay be segfaulting.\n\n>>\n>> 1. http://pubs.opengroup.org/onlinepubs/009695399/utilities/grep.html\n"},{"id":"372609","messageId":"20190328095839.GF32732@szeder.dev","threadId":"50823","inReplyTo":"8736n8be71.fsf@evledraar.gmail.com","subject":"Re: [GSoC] microporject test_path_is_*","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-03-28T09:58:39Z","receivedAt":"2019-03-28T09:58:47Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Mar 27, 2019 at 03:10:42PM +0100, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Wed, Mar 27 2019, SZEDER Gábor wrote:\n> \n> > On Wed, Mar 27, 2019 at 12:21:55PM +0100, Ævar Arnfjörð Bjarmason wrote:\n> >>\n> >> On Wed, Mar 27 2019, SZEDER Gábor wrote:\n> >>\n> >> > On Wed, Mar 27, 2019 at 11:09:18AM +0100, Ævar Arnfjörð Bjarmason wrote:\n> >> >> > There are likewise several that use one of\n> >> >> > ! test -e path/to/filename\n> >> >> > or\n> >> >> > ! test -f path/to/filename\n> >> >> > or\n> >> >> > test ! -f path/to/filename\n> >> >> > which could be replaced by\n> >> >> > test_path_is_missing path/to/filename\n> >> >>\n> >> >> Interesting that for some we use the 'test_is_there/test_is_not_there'\n> >> >> pattern and for others 'test_is_there [!]'. E.g\n> >> >> test_path_exist/test_path_is_missing v.s. test_i18ngrep.\n> >> >\n> >> > It's unclear what the '!' should negate in case of 'test_path_is_file\n> >> > ! file'. What if 'file' does exists, but it's not a file but a\n> >> > directory, socket, fifo, or symlink? 'test ! -f file' returns success\n> >> > in these cases as well.\n> >> >\n> >> > OTOH, it's quite clear what the negation should mean in case of\n> >> > 'test_i18ngrep'.\n> >>\n> >> *Should* we make it better? Yeah sure, maybe. I'm just pointing out for\n> >> context to someone poking at this for the first time that now we\n> >> sometimes do \"! foo \" v.s. \"foo \" as \"foo_is \" and\n> >> \"foo_not \" and other times \"foo [!] \".\n> >>\n> >> So yeah, maybe we should improve things to disambiguate the cases you\n> >> mentioned, but right now e.g. \"test_path_exists\" and\n> >> \"test_path_is_missing\" are just \"test -e\" and \"! test -e\", respectively.\n> >\n> > I'm not sure why 'test_path_exists' exists, as I don't readily see a\n> > valid reason why a test should only be interested in whether the path\n> > exists, but but not whether it's a file or a directory.\n> \n> In the general case the same reason we use \"test -e\". While the test\n> would pass in all sorts of unexpected cases, those probably aren't\n> plausible and we're just e.g. checking \"did the thing create a file\n> it'll create in XYZ mode?\"....\n\nWhen I look at a test using 'test -e' I can't readily tell what that\nreason might have been, i.e. whether there really was a compelling\nreason for using 'test -e' instead of 'test -(f|d|...), or the dev\nwriting the test was just \"meh, we could let it slide\".\n\n> > Luckily it\n> > haven't caught on, and it's only used twice in the whole test suite.\n> \n> Well, we have some >100 \"test -e\" though ... :)\n\nHopefully they will be turned into 'test_path_is_file' or\n'test_path_is_dir' instead of 'test_path_exists'... eventually, maybe\na decade or two worth of GSOC microprojects later... :)\n\n> > However, as shown above, the existend of 'test_path_is_missing' is\n> > very much justified.\n> >\n> >> The same caveats you've mentioned also apply to \"test_i18ngrep\" b.t.w.,\n> >> there we squash the 3x standard exit codes of grep[1] into a boolean,\n> >> and thus e.g. ignore the difference between not matching an\n> >> being a directory or whatever.\n> >\n> > I'm not sure I understand, 'test_i18ngrep' handles the missing file or\n> > not a file cases reasonably well:\n> >\n> > expecting success:\n> > test_i18ngrep ! foo nonexistent-file\n> >\n> > error: bug in the test script: test_i18ngrep requires a file to read as the last parameter\n> >\n> > or\n> >\n> > expecting success:\n> > mkdir dir &&\n> > test_i18ngrep ! foo dir\n> >\n> > error: bug in the test script: test_i18ngrep requires a file to read as the last parameter\n> \n> Yeah you're right, I didn't read it carefully enough and it does handle\n> *that* particular case, but e.g. a grep of \"file\" where we can't read it\n> due to a permission error is the same as \"didn't contain the string\".\n> \n> I'm not saying that's a bad thing, but the opposite. We assume a certain\n> base level of sanity, e.g. we do \"test_must_fail \" only for git,\n> but \"! \" for everything else, even though e.g. the system \"grep\"\n> may be segfaulting.\n\nThat's kind of the point: this is not about the sanity of the system,\nbut rather about the sanity of the developers :)\n\n'test_i18ngrep' does handle that particular case, because,\nunfortunately, paths do get misspelled (or downright forgotten!) every\nnow and then, but 'test_expect_failure', '! cmd', a pipe downstream,\nor some other circumstances can still hide those errors. I fixed\nseveral issues with bogus filenames in the past.\n\nHowever, since we assume that the system we are testing on is sane,\nthere is indeed not much point in preparing for cases that could only\ngo wrong if the system was not sane, e.g. the lack of permissions to\nread a just written file. I don't recall that we had any such\npermission-related issues in the past (but I didn't actually check).\nHaving said that, I agree that checking whether 'grep' exited with 2\nwould have been more robust.\n\n\nAnd these are the cases where using test helper functions that know\nabout the negation like 'test_path_is_missing', 'test_must_be_empty',\nor 'test_i18ngrep !' instead of '! test -(f|d|s|...)' etc. starts to\npay off, as they can protect us from our own mistakes. Case in point:\nthe last remaining '! test -s' in our test suite is definitely wrong.\n\nSo I think if such a helper function is available, then we should use\nit, and we might want to consider adding even more, e.g. we have a\ncouple of instances of '! test_cmp', '! has_cr'. And these functions\ndon't have to be perfect and catch every case that could possibly go\nwrong, they just need to do a reasonably good job at catching those\ncases that would be our own fault if they were to go wrong.\n\n\n> >> 1. http://pubs.opengroup.org/onlinepubs/009695399/utilities/grep.html\n"}]}