From: Phillip Wood <phillip.wood123@gmail.com>
Date: Fri, 24 Oct 2025 13:28:21 GMT
Subject: Re: [PATCH v4 3/3] replay: add replay.refAction config option
Message-ID: <a4cd31ad-7086-4d05-ba00-db65ec24b45a@gmail.com>
In-Reply-To: <20251022185045.29256-4-siddharthasthana31@gmail.com>

On 22/10/2025 19:50, Siddharth Asthana wrote:

This is looking pretty nice now, I've left some on he tests comments below

> diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
> index 54c86b87d8..307beb667e 100755
> --- a/t/t3650-replay-basics.sh
> +++ b/t/t3650-replay-basics.sh
> @@ -217,4 +217,46 @@ test_expect_success 'merge.directoryRenames=false' '
>   		--onto rename-onto rename-onto..rename-from
>   '
>   
> +test_expect_success 'replay.refAction config option' '
> +	# Store original state
> +	START=$(git rev-parse topic2) &&

Isn't there a tag we can use here from the initial setup?

> +	test_when_finished "git branch -f topic2 $START && git config --unset replay.refAction" &&
> +
> +	# Set config to print
> +	git config replay.refAction print &&
I think it would be better to use test_config here rather than having to 
clear the config manually with test_when_finished() above.

> +	git replay --onto main topic1..topic2 >output &&
> +	test_line_count = 1 output &&
> +	grep "^update refs/heads/topic2 " output &&

Rather than test_line_count and grep it would be better to use test_cmp 
here.

The same comments apply to the rest of the tests

Thanks

Phillip

> +
> +	# Reset and test update mode
> +	git branch -f topic2 $START &&
> +	git config replay.refAction update &&
> +	git replay --onto main topic1..topic2 >output &&
> +	test_must_be_empty output &&
> +
> +	# Verify ref was updated
> +	git log --format=%s topic2 >actual &&
> +	test_write_lines E D M L B A >expect &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'command-line --ref-action overrides config' '
> +	# Store original state
> +	START=$(git rev-parse topic2) &&
> +	test_when_finished "git branch -f topic2 $START && git config --unset replay.refAction" &&
> +
> +	# Set config to update but use --ref-action=print
> +	git config replay.refAction update &&
> +	git replay --ref-action=print --onto main topic1..topic2 >output &&
> +	test_line_count = 1 output &&
> +	grep "^update refs/heads/topic2 " output
> +'
> +
> +test_expect_success 'invalid replay.refAction value' '
> +	test_when_finished "git config --unset replay.refAction" &&
> +	git config replay.refAction invalid &&
> +	test_must_fail git replay --onto main topic1..topic2 2>error &&
> +	grep "invalid value for replay.refAction" error
> +'
> +
>   test_done