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. Æ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. [1]: https://lore.kernel.org/git/20200921104000.2304-15-avarab@xxxxxxxxx/ [2]: https://lore.kernel.org/git/20200809060810.31370-1-sunshine@xxxxxxxxxxxxxx/ [3]: https://lore.kernel.org/git/7f408b7d4069403b969d334f4940ebf87f1dc797.1596906081.git.gitgitgadget@xxxxxxxxx/
