On Fri, Mar 30, 2012 at 9:52 AM, Jeff King <peff@xxxxxxxx> wrote: > 1. I split the find-in-path function so that Frans can build on it > with further checks if he wants. However, note that it will need a > little more refactoring; it finds the first entry in the PATH, > whereas I think he would want the first executable entry > (admittedly, having a non-executable entry followed by an > executable one which _also_ fails for a different reason is a > pretty wild corner case). Thanks. I think this corner case is something to fix when someone is running into it. Bash doesn't cover this case either (stops looking at the first found entry) so I would go as far as saying that it doesn't happen. > 2. I pulled the test from what Junio posted earlier. I started to > write a full test script that checked each of the cases I > mentioned earlier. However, in all but the alias case (which is > what is tested here), the behavior is really only distinguishable > by the error messages, and I didn't want to get into testing what > strerror(EACCES) prints. I can re-roll if we really want to go > there. I wouldn't think there is much point in testing strerror output. > +test_expect_success POSIXPERM 'unreadable directory in PATH' ' > + mkdir local-command && > + test_when_finished "chmod u+rwx local-command && rm -fr local-command" && > + git config alias.nitfol "!echo frotz" && > + chmod a-rx local-command && > + ( > + PATH=./local-command:$PATH && > + git nitfol >actual > + ) && > + echo frotz >expect && > + test_cmp expect actual > +' > + > test_done > -- > 1.7.9.5.7.g11b89 Hadn't looked into Junio's test earlier (probably missed it entirely), but isn't it rather more sensible from a unit-test perspective to see if start_command returns 127 instead of 128 in this specific case? Aside from that, this test doesn't seem to fit in t0061, looking at the t???? guidelines. Rest looks sensible to me. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html