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: > > On Mon, Sep 13, 2021 at 03:29:25PM -0400, Taylor Blau wrote: > > > > + git config --unset core.commitGraph && > > > > > > But I'm not aware of a way to temporarily unset a configuration variable > > > for the duration of a test, so here I would probably write: > > > > > > test_must_fail git -c core.commitGraph= fsck > > > > > > which Git interprets as "pretend this variable is unset in-core". > > > > From my testing, I suspect that git does not pretend the variable is > > unset, but rather, it pretends that the variable is set to the empty > > string. It seems that git behaves as if the variable is set to "false". > > This is called out in Documentation/git.txt: > > > > Including the equals but with an empty value (like `git -c > > foo.bar= ...`) sets `foo.bar` to the empty string which `git config > > --type=bool` will convert to `false`. > > > > 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. Even if we don't have any such tests now, it seems like we should err on the side of leaving it alone (although I suppose that future tests could set core.commitGraph to whatever value they need as long as they use the test_config function). > 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). Thanks, Taylor
