Re: [PATCH v5 1/3] builtin/config: introduce `--default`
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Apr 7, 2018, 00:58 UTC
- Message-ID
- <20180407005837.GD78042@syl.local>
- In-Reply-To
- <CAPig+cRrz_Reb1CJN0u1LSQ38LjED+tuBAT=RT5rESfJ05absg@mail.gmail.com>
On Fri, Apr 06, 2018 at 03:40:56AM -0400, Eric Sunshine wrote:
Show 22 quoted lines
> On Fri, Apr 6, 2018 at 2:53 AM, Eric Sunshine <sunshine@sunshineco.com> wrote: > > On Fri, Apr 6, 2018 at 2:30 AM, Taylor Blau <me@ttaylorr.com> wrote: > >> +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? > > One other issue. If "git config --default ..." fails, the --unset line > will never be invoked, thus cleanup won't happen. > > To ensure cleanup, either use: > > test_when_finished "git config --unset core.foo" && > > earlier in the test (just after the --add line) or use the > test_config() function to both set the value and ensure its cleanup.
Neat. I wasn't aware of 'test_when_finished'. I think that I prefer the explicitness of 'test_when_finished "git config ..."' over 'test_config()', but I am happy to use whichever is preferred. Since t1310 is new, there's no historical precedent to follow. Please let me know if you have a preference one way or another.
Thanks, Taylor