Re: [PATCH] Add more checkout tests

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 10, 2007, 03:42 UTC
Message-ID: <7vprxfmczi.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <Pine.LNX.4.64.0712092204200.5349@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

Show 14 quoted lines

> +test_expect_success "checkout with unrelated dirty tree without -m" '
> +
> +	git checkout -f master &&
> +	fill 0 1 2 3 4 5 6 7 8 >same &&
> +	cp same kept
> +	git checkout side >messages && 
> +	git diff same kept
> +	(cat > messages.expect <<EOF
> +M	same
> +EOF
> +) &&
> +	touch messages.expect &&
> +	git diff messages.expect messages
> +'

What is this "touch" about?

I do not recall the details, but we had problem reports that some shells do not handle here-documents (i.e. cmd <<EOF) inside test_expect_success well, and generally tried to keep them outside. I however see some of the newer tests do have here-doc inside expect-success, and do not recall hearing breakage reports on them. Maybe it was a false alarm and we were being overly cautious, or maybe not enough people (especially on more exotic systems) are running tests these days. Let's keep the here-doc as-is in your tests and see what happens.

Show 23 quoted lines

>  test_expect_success "checkout -m with dirty tree" '
>  
>  	git checkout -f master &&
>  	git clean -f &&
>  
>  	fill 0 1 2 3 4 5 6 7 8 >one &&
> -	git checkout -m side &&
> +	git checkout -m side > messages &&
>  
>  	test "$(git symbolic-ref HEAD)" = "refs/heads/side" &&
>  
> +	(cat >expect.messages <<EOF
> +Merging side with local
> +Merging:
> +ab76817 Side M one, D two, A three
> +virtual local
> +found 1 common ancestor(s):
> +7329388 Initial A one, A two
> +Auto-merged one
> +M	one
> +EOF
> +) &&
> +	git diff expect.messages messages &&

I do not like the idea of testing the exact wording of messages this way.

I do not think we care about the exact wording of these messages, and I think our tests should check what we do care about without casting the UI in stone. Otherwise, it will make it harder to clean-up the user experience later. Perhaps it would be sufficient to make sure that (1) this checkout succeeds with exit 0 status, and that (2) the contents of the merged 'one' is a reasonable merge result, i.e. "git diff HEAD one" gets the same patch-id as "git diff HEAD one" taken before switching the branches.

Show 15 quoted lines

> @@ -145,7 +176,16 @@ test_expect_success 'checkout -m with merge conflict' '
>  test_expect_success 'checkout to detach HEAD' '
>  
>  	git checkout -f renamer && git clean -f &&
> -	git checkout renamer^ &&
> +	git checkout renamer^ 2>messages &&
> +	(cat >messages.expect <<EOF
> +Note: moving to "renamer^" which isn'"'"'t a local branch
> +If you want to create a new branch from this checkout, you may do so
> +(now or later) by using -b with the checkout command again. Example:
> +  git checkout -b <new_branch_name>
> +HEAD is now at 7329388... Initial A one, A two
> +EOF
> +) &&
> +	git diff messages.expect messages &&

Same here. If we want to make sure the head is detached at the intended commit, make sure "rev-parse HEAD" gives the expected result, and make sure "symbolic-ref HEAD" says it is not symbolic.

Show 5 quoted lines

>  	H=$(git rev-parse --verify HEAD) &&
>  	M=$(git show-ref -s --verify refs/heads/master) &&
>  	test "z$H" = "z$M" &&
> -- 
> 1.5.3.6.886.gb204