From: Kristoffer Haugsbakk <code@khaugsbakk.name>
Date: Tue, 17 Oct 2023 20:20:12 GMT
Subject: Re: [PATCH 2/8] t7900: setup and tear down clones
Message-ID: <655ca147-c214-41be-919d-023c1b27b311@app.fastmail.com>
In-Reply-To: <xmqqwmvlgg71.fsf@gitster.g>

On Tue, Oct 17, 2023, at 22:13, Junio C Hamano wrote:
> As I already said in my response to the cover letter, while I am
> surprised that the series managed to make each step (and it alone)
> succeed after the set-up (applaud!), I am not sure if it is really
> worth doing.  As the business of test scripts is to test git, and it
> means that we should always assume that we are dealing with a
> potentially broken version of git.  By running so many git
> subcommands in test_when_finished, each of them may be from a buggy
> implementation of git, we cannot be really sure that we are
> resetting the environment to the pristine state.  We should strive
> to do as little as possible in test_when_finished.

I'll have to think more about this part in order to understand the
ramifications. Thanks for the feedback.

> This is even worse; it has to redo much of what the previous test
> did.  Developers cannot be reasonably expected to maintain this
> duplication when we need to change the earlier test.
>
> While I am impressed that "set-up + individual single test" was made
> to work, I am not convinced that the changes that took us to get
> there are reasonable.  The end result looks much less maintainable
> and more wasteful with duplicated steps.
>
> Thanks.

I can rewrite this one—as well as others—to use the `setup` keyword in the
original test instead.

But dropping the series is also fine. I am still very new to this test
suite.

Cheers

-- 
Kristoffer Haugsbakk