diff mbox series

[v4,2/3] rebase: use test_commit helper in setup

Message ID 5c40e116eba00b5b1a64191c6adf211d326e7f96.1647546828.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series rebase: update HEAD when is an oid | expand

Commit Message

John Cai March 17, 2022, 7:53 p.m. UTC
From: John Cai <johncai86@gmail.com>

To prepare for the next commit that will test rebase with oids instead
of branch names, update the rebase setup test to add a couple of tags we
can use. This uses the test_commit helper so we can replace some lines
that add a commit manually.

Setting logAllRefUpdates is not necessary because it's on by default for
repositories with a working tree.

Helped-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t3400-rebase.sh | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

Comments

Phillip Wood March 18, 2022, 11:14 a.m. UTC | #1
Hi John

On 17/03/2022 19:53, John Cai via GitGitGadget wrote:
> From: John Cai <johncai86@gmail.com>
> 
> To prepare for the next commit that will test rebase with oids instead
> of branch names, update the rebase setup test to add a couple of tags we
> can use. This uses the test_commit helper so we can replace some lines
> that add a commit manually.
> 
> Setting logAllRefUpdates is not necessary because it's on by default for
> repositories with a working tree.
> 
> Helped-by: Phillip Wood <phillip.wood123@gmail.com>
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
>   t/t3400-rebase.sh | 18 ++++--------------
>   1 file changed, 4 insertions(+), 14 deletions(-)
> 
> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
> index 5c4073f06d6..2fb3fabe60e 100755
> --- a/t/t3400-rebase.sh
> +++ b/t/t3400-rebase.sh
> @@ -18,10 +18,7 @@ GIT_AUTHOR_EMAIL=bogus@email@address
>   export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL
>   
>   test_expect_success 'prepare repository with topic branches' '
> -	git config core.logAllRefUpdates true &&
> -	echo First >A &&
> -	git update-index --add A &&
> -	git commit -m "Add A." &&
> +	test_commit "Add A." A First First &&
>   	git checkout -b force-3way &&
>   	echo Dummy >Y &&
>   	git update-index --add Y &&
> @@ -32,17 +29,13 @@ test_expect_success 'prepare repository with topic branches' '
>   	git mv A D/A &&
>   	git commit -m "Move A." &&
>   	git checkout -b my-topic-branch main &&
> -	echo Second >B &&
> -	git update-index --add B &&
> -	git commit -m "Add B." &&
> +	test_commit "Add B." B Second Second &&
>   	git checkout -f main &&
>   	echo Third >>A &&
>   	git update-index A &&
>   	git commit -m "Modify A." &&
>   	git checkout -b side my-topic-branch &&

This version has added some more conversions that are not mentioned it 
the commit message. If you want to covert the whole setup to use 
test_commit then that's great but I think you need to do the whole thing 
and say in the commit message that you're modernizing everything. As it 
stands a reader is left wondering why some of the setup that is not used 
in the next commit  has been converted but other bits such as the 
"Modify A." above are left as is. I think the two possibilities that 
make sense are (a) convert only what we need for the next commit, or (b) 
modernize the test and convert everything.

Best Wishes

Phillip

> -	echo Side >>C &&
> -	git add C &&
> -	git commit -m "Add C" &&
> +	test_commit --no-tag "Add C" C Side &&
>   	git checkout -f my-topic-branch &&
>   	git tag topic
>   '
> @@ -119,10 +112,7 @@ test_expect_success 'rebase off of the previous branch using "-"' '
>   test_expect_success 'rebase a single mode change' '
>   	git checkout main &&
>   	git branch -D topic &&
> -	echo 1 >X &&
> -	git add X &&
> -	test_tick &&
> -	git commit -m prepare &&
> +	test_commit prepare X 1 &&
>   	git checkout -b modechange HEAD^ &&
>   	echo 1 >X &&
>   	git add X &&
John Cai March 18, 2022, 1:06 p.m. UTC | #2
Hi Phillip,

On 18 Mar 2022, at 7:14, Phillip Wood wrote:

> Hi John
>
> On 17/03/2022 19:53, John Cai via GitGitGadget wrote:
>> From: John Cai <johncai86@gmail.com>
>>
>> To prepare for the next commit that will test rebase with oids instead
>> of branch names, update the rebase setup test to add a couple of tags we
>> can use. This uses the test_commit helper so we can replace some lines
>> that add a commit manually.
>>
>> Setting logAllRefUpdates is not necessary because it's on by default for
>> repositories with a working tree.
>>
>> Helped-by: Phillip Wood <phillip.wood123@gmail.com>
>> Signed-off-by: John Cai <johncai86@gmail.com>
>> ---
>>   t/t3400-rebase.sh | 18 ++++--------------
>>   1 file changed, 4 insertions(+), 14 deletions(-)
>>
>> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
>> index 5c4073f06d6..2fb3fabe60e 100755
>> --- a/t/t3400-rebase.sh
>> +++ b/t/t3400-rebase.sh
>> @@ -18,10 +18,7 @@ GIT_AUTHOR_EMAIL=bogus@email@address
>>   export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL
>>    test_expect_success 'prepare repository with topic branches' '
>> -	git config core.logAllRefUpdates true &&
>> -	echo First >A &&
>> -	git update-index --add A &&
>> -	git commit -m "Add A." &&
>> +	test_commit "Add A." A First First &&
>>   	git checkout -b force-3way &&
>>   	echo Dummy >Y &&
>>   	git update-index --add Y &&
>> @@ -32,17 +29,13 @@ test_expect_success 'prepare repository with topic branches' '
>>   	git mv A D/A &&
>>   	git commit -m "Move A." &&
>>   	git checkout -b my-topic-branch main &&
>> -	echo Second >B &&
>> -	git update-index --add B &&
>> -	git commit -m "Add B." &&
>> +	test_commit "Add B." B Second Second &&
>>   	git checkout -f main &&
>>   	echo Third >>A &&
>>   	git update-index A &&
>>   	git commit -m "Modify A." &&
>>   	git checkout -b side my-topic-branch &&
>
> This version has added some more conversions that are not mentioned it the commit message. If you want to covert the whole setup to use test_commit then that's great but I think you need to do the whole thing and say in the commit message that you're modernizing everything. As it stands a reader is left wondering why some of the setup that is not used in the next commit  has been converted but other bits such as the "Modify A." above are left as is. I think the two possibilities that make sense are (a) convert only what we need for the next commit, or (b) modernize the test and convert everything.

I see your point. Then for the sake of a smaller series for the patch, I'll opt
to only update the parts we need. Then maybe we can have a separate series to
modernize this suite.

I'll re-roll this series with the clarifications to the commit message that
Junio made.

>
> Best Wishes
>
> Phillip

Thanks!
John

>
>> -	echo Side >>C &&
>> -	git add C &&
>> -	git commit -m "Add C" &&
>> +	test_commit --no-tag "Add C" C Side &&
>>   	git checkout -f my-topic-branch &&
>>   	git tag topic
>>   '
>> @@ -119,10 +112,7 @@ test_expect_success 'rebase off of the previous branch using "-"' '
>>   test_expect_success 'rebase a single mode change' '
>>   	git checkout main &&
>>   	git branch -D topic &&
>> -	echo 1 >X &&
>> -	git add X &&
>> -	test_tick &&
>> -	git commit -m prepare &&
>> +	test_commit prepare X 1 &&
>>   	git checkout -b modechange HEAD^ &&
>>   	echo 1 >X &&
>>   	git add X &&
diff mbox series

Patch

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 5c4073f06d6..2fb3fabe60e 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -18,10 +18,7 @@  GIT_AUTHOR_EMAIL=bogus@email@address
 export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL
 
 test_expect_success 'prepare repository with topic branches' '
-	git config core.logAllRefUpdates true &&
-	echo First >A &&
-	git update-index --add A &&
-	git commit -m "Add A." &&
+	test_commit "Add A." A First First &&
 	git checkout -b force-3way &&
 	echo Dummy >Y &&
 	git update-index --add Y &&
@@ -32,17 +29,13 @@  test_expect_success 'prepare repository with topic branches' '
 	git mv A D/A &&
 	git commit -m "Move A." &&
 	git checkout -b my-topic-branch main &&
-	echo Second >B &&
-	git update-index --add B &&
-	git commit -m "Add B." &&
+	test_commit "Add B." B Second Second &&
 	git checkout -f main &&
 	echo Third >>A &&
 	git update-index A &&
 	git commit -m "Modify A." &&
 	git checkout -b side my-topic-branch &&
-	echo Side >>C &&
-	git add C &&
-	git commit -m "Add C" &&
+	test_commit --no-tag "Add C" C Side &&
 	git checkout -f my-topic-branch &&
 	git tag topic
 '
@@ -119,10 +112,7 @@  test_expect_success 'rebase off of the previous branch using "-"' '
 test_expect_success 'rebase a single mode change' '
 	git checkout main &&
 	git branch -D topic &&
-	echo 1 >X &&
-	git add X &&
-	test_tick &&
-	git commit -m prepare &&
+	test_commit prepare X 1 &&
 	git checkout -b modechange HEAD^ &&
 	echo 1 >X &&
 	git add X &&