Re: [PATCH] Add more checkout tests

[Junio C Hamano <gitster@pobox.com>]

Daniel Barkalow <barkalow@iabervon.org> writes:

> +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.

>  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.

> @@ -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.

>  	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