On Fri, Apr 6, 2018 at 8:49 PM, Taylor Blau <m...@ttaylorr.com> wrote:
> On Fri, Apr 06, 2018 at 02:53:45AM -0400, Eric Sunshine wrote:
>> On Fri, Apr 6, 2018 at 2:30 AM, Taylor Blau <m...@ttaylorr.com> wrote:
>> > +test_expect_success 'uses --default when missing entry' '
>> > + echo quux >expect &&
>> > + git config -f config --default quux core.foo >actual &&
>> > + test_cmp expect actual
>> > +'
>> >
>> > +test_expect_success 'uses entry when available' '
>> > + echo bar >expect &&
>> > + git config --add core.foo bar &&
>> > + git config --default baz core.foo >actual &&
>> > + git config --unset core.foo &&
>> > + test_cmp expect actual
>> > +'
>>
>> If you happen to re-roll, can we move this test so it immediately
>> follows the "uses --default when missing entry" test? That's where I
>> had expected to find it and had even written a review comment saying
>> so (but deleted the comment once I got down to this spot in the
>> patch). Also, perhaps rename the test to make it clearer that it
>> complements the "uses --default when missing entry" test; perhaps
>> "does not fallback to --default when entry present" or something.
>
> That location makes much more sense, as does using --default=yes to
> ensure that canonicalization is taking place. I've updated that locally,
> as well as the preceding test to make it clearer that they are
> contrasting tests:
>
> - 'falls back to --default when missing entry'
> - 'does not fallback to --default when entry present'
>
> Though I am not sure about "falls back" to mean "uses --default". I am
> not sure which to pick here... what are your thoughts?
It's nice for the titles to show that the two tests complement one
another but the exact wording is not terribly important. Taking your
original test title (slightly modified), perhaps:
- 'uses --default when entry missing'
- 'does not use --default when entry present'
