Re: [PATCH v2 11/14] t/t5505-remote: test multiple push/pull in remotes-file

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Jun 23, 2013, 08:28 UTC
Message-ID: <CALkWK0nFySj_Xztfwjp=FAhz=dBxgmwZ9YG=PY=7HZLfw0P8aw@mail.gmail.com>
In-Reply-To: <7vobaxfdx9.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

> I'll reject 04/14 and tweak this patch to use 'next' for the new ref
> mappings, not duplicated 'master'.

It's a matter of taste: I don't like "realistic" (albeit misleading) values when I'm testing configuration variables, while you think they make the tests more readable. Fine.

Show 10 quoted lines

> Patches 07/14, 12/14, 13/14, and 14/14 are bad idea (these will not
> be queued on tonight's pushout of 'pu'; neither 04/14 will be).  We
> may not be encouraging, but that is very different from deprecating
> the original mechanisms.  The tests that depend on them to work
> should be kept.  Otherwise, we will never know when we break them
> like we did at df93e33c accidenally.
>
> If we want to have tests that exercise the equivalents spelled with
> the modern in-config mechanism, they should be added as new tests,
> not by replacing the existing ones.

I disagree. It is trivial to prove that the tests in t/remote will break if this fringe feature breaks: I don't know where "we will never know when we break them" is coming from. Why should I know about this fringe feature when I'm reading/writing tests for fetch-merge-logic? And what is _advantage_ of depending on this fringe feature when testing fetch-merge-logic? More tests break?

But whatever. I've already spent more time discussing (bikeshedding?) this series than writing it. I regret having written a stupid cleanup series with no real consequence now; I'll be less likely to make the same mistake again in the future.

Thanks.