Thread (2 messages) 2 messages, 2 authors, 2023-10-17

Re: [PATCH 2/8] t7900: setup and tear down clones

From: Kristoffer Haugsbakk <hidden>
Date: 2023-10-17 20:20:35

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help