Re: [PATCH v3 1/3] test-lib-functions: handle --add in test_config

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Dec 22, 2020, 05:19 UTC
Message-ID: <CAPig+cR1AUH6w8QBaumFwcbz+0LmzYdP61WtaENGHhSgts277A@mail.gmail.com>
In-Reply-To: <CAN8Z4-WpNuqN=HxL4AQU_+zi4hGhkC18d3ZSOJGzbdkMPkYAMQ@mail.gmail.com>

On Mon, Dec 21, 2020 at 9:25 PM Nipunn Koorapati <nipunn1313@gmail.com> wrote:

Show 5 quoted lines

> That being said, I suspect anyone in the future poking around with
> this will have to sourcedive
> through test_config and test_unconfig to make sense of it.
>
> I'll switch it over to the test_unconfig option on the next reroll if requested.

Indeed, it's not entirely uncommon to have to dig into the test functions when writing tests. My bigger concern was someone coming along and thinking that the mixed use of test_config() and manual `git config` in the same test was a mistake, and want to fix that mistake, which would lead the person down the same rabbit hole. Explicit cleanup via test_unconfig() and consistent use of `git config` within the test, on the other hand, does not look accidental, so the reader would be less likely to want to "fix the mistake". The comment you added in the re-roll above the manual cleanup saves the reader the trouble of having to dive into the implementation of test_config(), which is a nice bonus.