Re: [PATCH 11/13] contrib/subtree: Make each test self-contained

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 5, 2013, 05:13 UTC
Message-ID: <7vwqunwdfk.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1360037173-23291-12-git-send-email-greened@obbligato.org>

"David A. Greene" <greened@obbligato.org> writes:

Show 6 quoted lines

> +test_create_commit() (
> +	repo=$1
> +	commit=$2
> +	cd "$repo"
> +	mkdir -p "$(dirname "$commit")"
> +	echo "$commit" > "$commit"

Style.

> +	git add "$commit"
> +	git commit -m "$commit"
> +)

Very nice, but don't we want to check for possible errors in any of the above commands?

Show 26 quoted lines

>  last_commit_message()
>  {
>  	git log --pretty=format:%s -1
>  }
>  
> +#
> +# Tests for 'git subtree add'
> +#
>  
> -test_expect_success 'fetch subproj history' '
> -		git fetch ./subproj sub1 &&
> -		git branch sub1 FETCH_HEAD
> -'
>  
>  test_expect_success 'no pull from non-existant subtree' '
> -		test_must_fail git subtree pull --prefix=subdir ./subproj sub1
> +	test_create_repo "$test_count" &&
> +	test_create_repo "$test_count/subproj" &&
> +	test_create_commit "$test_count" main1 &&
> +	test_create_commit "$test_count/subproj" sub1 &&
> +	(
> +		cd "$test_count" &&
> +		git fetch ./subproj master &&
> +		test_must_fail git subtree pull --prefix=subdir ./subproj master
> +	)
>  '

The goal of making each tests indenendent is a very good one, but we'd really prefer not to see $test_count which is an implementation detail of the test framework to be used like this. It will make it unnecessarily harder to improve the test framework (e.g. it may want to stop using the $test_count variable).

This is not limited to this variable, but all other $test_anything variable. Earlier I wanted to update the definition of test_tick which happens to be decimal number of seconds since epoch, but some tests were comparing it with the raw values read from cat-file output for a commit object, and it was really painful.