Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes: > On Thu, Oct 15, 2020 at 8:17 PM Jeff King <peff@xxxxxxxx> wrote: >> On Sun, Aug 09, 2020 at 01:42:09PM -0400, Eric Sunshine wrote: >> > [...] Make it easier for test authors to discover such problems early >> > by sanity-checking the arguments to test_cmp(). [...] >> >> This patch caused some interesting confusion for me today. >> error: bug in the test script: test_cmp 'r2/.git/HEAD' missing >> which was somewhat unhelpful (or at least less helpful than a regular >> test failure). The test in question does this: >> test_cmp r0/.git/HEAD r2/.git/HEAD && >> and expects to fail if an earlier step didn't correctly create r2. Is it >> a bug or misuse of test_cmp for it to do so? I could see an argument >> that it is, but I'm also not sure if there's a convenient alternative. > > I can see the argument going both ways as to whether it's a misuse of > 'test_cmp'. > >> The best I could come up with is: >> >> test_path_is_file r2/.git/HEAD && >> test_cmp r0/.git/HEAD r2/.git/HEAD >> >> which isn't that great. Hmph, I agree that the "both must be file" is a bit too eager and ignores that "they must match, but the possible reasons they may not include one of them may be missing" use case. > Ævar ran into the same issue recently[1] and came up with the same > workaround. Despite its good intention (trying to catch bugs in > 'test_expect_failure' tests), this change[2] doesn't seem to have > caught any genuine bugs (it wouldn't even have caught the bug which > served as its inspiration[3]), but has nevertheless caused a couple > hiccups already. As such, I would not be opposed to seeing the change > reverted. Sounds good. Anybody wants to do the honors?
