On Tue, Jun 26, 2018 at 2:06 PM Johannes Sixt <j6t@xxxxxxxx> wrote: > Am 26.06.2018 um 11:21 schrieb Eric Sunshine: > >> On Tue, Jun 26, 2018 at 12:29 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > >>> + p4 help client && > >>> + test_must_fail p4 help nosuchcommand > > [...] So, despite > > the (somewhat) misleading test title, this test doesn't care about the > > exact error code but rather cares only that "p4 help nosuchcommand" > > errors out, period. Hence, test_must_fail() again agrees with the > > spirit of the test. > > test_must_fail ensures that only "proper" failures are diagnosed as > expected; failures due to signals such as SEGV are not expected failures. > > In the test suite we expect all programs that are not our "git" to work > correctly; in particular, that they do not crash on anything that we ask > them to operate on. Under this assumption, the protection given by > test_must_fail is not needed. > > Hence, these lines should actually be > > p4 help client && > ! p4 help nosuchcommand Thanks for the comment; you're right, of course. I'll certainly make this change if I have to re-roll for some other reason, but do you feel that this itself is worth a re-roll?