Jeff King <peff@xxxxxxxx> writes: >> +# Basic command line option parsing >> +test_expect_success '-L is incompatible with pathspec' ' >> + # This may fail due to "no such path a.c in commit", >> + # or "-L is incompatible with pathspec". Either is acceptable. >> + test_must_fail git log -L1,1:a.c -- a.c && > > This test confuses me. What are we looking for here? Presumably we'd > fail with: > > git log -L1,1:a.c > > too. If the test were "basic command line parsing", I could see checking > that. But that's only what the comment says. Yeah, I was undecided to have a single test that covers all (which I ended up with) or a sequence of individual tests (which I wrote on the title). >> + # This must fail due to "-L is incompatible with pathspec". >> + test_must_fail git log -L1,1:b.c -- b.c && > > Right, this is what we fixed. Would using test_i18ngrep on the stderr be > better than the comment? I do not care either way myself ;-) >> + # These must fail due to "follow requires one pathspec". >> + test_must_fail git log -L1,1:b.c --follow && >> + test_must_fail git log --follow -L1,1:b.c && > > These are really tests of --follow, but I don't mind seeing them here as > reinforcement for the concepts that the commit message claims. > >> + # This may fail due to "-L is incompatible with pathspec", >> + # or "-L is incompatible with pathspec". Either is acceptable. >> + test_must_fail git log --follow -L1,1:b.c -- b.c > > Should one of those be "-L is incompatible with --follow"? Though of > course we did not add such a check, so we know that it will be "-L is > incompatible with pathspec", even without the --follow. The comment seems utterly wrong here. I may reroll after taking a nap or something ;-)
