Re: [PATCH v2 1/3] sequencer: comment checked-out branch properly

From: phillip.wood123@gmail.com <phillip.wood123@gmail.com>
Date: Nov 13, 2024, 14:47 UTC
Message-ID: <5ed77fab-678d-4a06-bbd0-ea25462a7562@gmail.com>
In-Reply-To: <xmqqikss2b7l.fsf@gitster.g>

On 13/11/2024 01:07, Junio C Hamano wrote:

Show 27 quoted lines

> kristofferhaugsbakk@fastmail.com writes:
> 
>> +test_expect_success 'git rebase --update-ref with core.commentChar and branch on worktree' '
>> +	test_when_finished git branch -D base topic2 &&
>> +	test_when_finished git checkout main &&
>> +	test_when_finished git branch -D wt-topic &&
>> +	test_when_finished git worktree remove wt-topic &&
>> +	git checkout main &&
>> +	git checkout -b base &&
>> +	git checkout -b topic2 &&
>> +	test_commit msg2 &&
>> +	git worktree add wt-topic &&
>> +	git checkout base &&
>> +	test_commit msg3 &&
>> +	git checkout topic2 &&
>> +	git -c core.commentChar=% rebase --update-refs base
>> +'
> 
> Can we improve this test a bit to give it more visibility into the
> breakage?
> 
> I am sure that the internal machinery gets confused because it wants
> to skip commented out lines assuming '%' is used for comments, and
> fails to skip lines that are commented with '#', but it is a bit too
> opaque how this would break without the fix.  Perhaps add a line or
> two of a comment to the test to describe what the expected mode of
> failure is?

Or check the todo list shown to the user with

	GIT_SEQUENCE_EDITOR="cat >actual" git -c core.commentChar=% \
		rebase -i --update-refs base &&
	test_grep "% Ref refs/heads/wt-topic checked out at " actual

so that we are sure that line exists - I had a quick look and I can't see any existing coverage checking that the todo list contains this comment.

Best Wishes

Phillip