Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name()

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Mar 20, 2013, 18:28 UTC
Message-ID: <20130320182830.GJ3655@google.com>
In-Reply-To: <1363783501-27981-4-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra wrote:

Show 6 quoted lines

> mk_test() creates a repository with the constant name "testrepo", and
> this may be limiting for tests that need to create more than one
> repository for testing.  To fix this, create a new mk_test_with_name()
> which accepts the repository name as $1.  Reimplement mk_test() as a
> special case of this function, making sure that no tests need to be
> rewritten.

Why not give mk_test an optional parameter?

	repo_name=${1:-testrepo}

Oh, it is because mk_test already takes arguments naming refs to push. I suppose the change description could make this clearer.

Why not use mk_test and then rename, like so?

	mk_test ...refs... &&
	mv testrepo testrepo-a &&

	mk_test ...refs... &&
	mv testrepo testrepo-b &&
	...

I dunno. The helper functions at the top of this test are already intimidating, so I guess I am looking for a way to avoid making that problem worse. One way would be to add an opening comment before the function definition explaining how it is meant to be used. See t/test-lib-functions.sh for examples, such as test_cmp.

Hope that helps, Jonathan