On Mon, Sep 13, 2021 at 9:09 PM Taylor Blau <me@xxxxxxxxxxxx> wrote: > On Mon, Sep 13, 2021 at 07:32:07PM -0400, Eric Sunshine wrote: > > On Mon, Sep 13, 2021 at 7:19 PM Glen Choo <chooglen@xxxxxxxxxx> wrote: > > > If the variable really is set to false, how might we proceed here? Shall > > > we stick with test_when_finished? > > > > That's probably reasonable, however, for robustness, you should > > probably use test_unconfig() rather than raw `git config --unset` to > > clear the variable. > > Hmm. I'm not so sure, do other tests rely on the value of that variable? > If so, test_unconfig() won't restore them. There may be a misunderstanding. I wasn't saying that test_unconfig() alone would work. My "That's probably reasonable" referred to Glen's proposal of combining `git config --unset` with test_when_finished() to restore the variable. In addition to that, I suggested test_unconfig() as being a more robust choice in that recipe. > > Aside: This certainly makes one wonder if we should have a new > > function in t/test-lib-functions.sh which unsets a variable for the > > duration of a test only. However, that's outside the scope of this > > submission. > > :-). I thought the same thing to myself when reviewing earlier today. > That's why I recommended using test_when_finished upthread, but either > approach is fine (my comments are definitely cosmetic, and don't matter > to the substance of this thread, so ultimately I am fine with either). Yep.