# [PATCH 0/3] t6025: updating tests

24 messages from 2020-01-16 to 2020-01-21. Participants: Shourya Shukla, Johannes Schindelin, Junio C Hamano, Eric Sunshine.
Thread: https://gitlist.dev/t/52653

## Shourya Shukla, 2020-01-16 20:36

Subject: [PATCH 0/3] t6025: updating tests
Message-ID: <20200116203622.4694-1-shouryashukla.oo@gmail.com>
URL: https://gitlist.dev/e/20200116203622.4694-1-shouryashukla.oo%40gmail.com

```
Greetings everyone!

This is my first ever contribution in the Open-Source Community and I chose Git
for this purpose as I have been using this important tool to maintain my projects
regularly.

In this patch, I have:

  - modernized these tests so that they meet current Git CodingGuidlines[1]
  - replaced the pipe operator with the redirection operator so that one can
	detect the errors easily and precisely
  - used helper function 'test_path_is_file()' to replace 'test -f' checks in
	in the program as it improves the readability of the code and provides 
	better error messages

Also, I have some questions to better my understanding of the code:
	- In the statement, 
		> git hash-object -t blob -w --stdin
	  is it necessary to explicitly specify the type 'blob' of the hash-object?
	  I have this question because it is the default type of hash-object.
	- In the statement, 
		> l=$(printf file | git hash-object -t blob -w --stdin)
	  I have not used the redirection operator as this sub-shell will be executed 
	  separately, hence its error cannot be captured therefore the presence of '>' 
	  will not matter. Will using '>' improve the code?

Thanks,
Shourya Shukla

Shourya Shukla (3):
  t6025: modernize style
  t6025: replace pipe with redirection operator
  t6025: use helpers to replace test -f <path>

 t/t6025-merge-symlinks.sh | 97 ++++++++++++++++++++-------------------
 1 file changed, 50 insertions(+), 47 deletions(-)

-- 
2.20.1

```

## Shourya Shukla, 2020-01-16 20:36

Subject: [PATCH 1/3] t6025: modernize style
Message-ID: <20200116203622.4694-2-shouryashukla.oo@gmail.com>
URL: https://gitlist.dev/e/20200116203622.4694-2-shouryashukla.oo%40gmail.com
In-Reply-To: <20200116203622.4694-1-shouryashukla.oo@gmail.com>

```
The tests in `t6025-merge-symlinks.sh` were written a long time ago, and
has a lot of style violations, including the mixed-use of tabs and spaces,
missing indentations, and other shell script style violations. Update it to
match the CodingGuidelines.

Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>
---
 t/t6025-merge-symlinks.sh | 97 ++++++++++++++++++++-------------------
 1 file changed, 50 insertions(+), 47 deletions(-)

diff --git a/t/t6025-merge-symlinks.sh b/t/t6025-merge-symlinks.sh
index 433c4de08f..b9219af659 100755
--- a/t/t6025-merge-symlinks.sh
+++ b/t/t6025-merge-symlinks.sh
@@ -10,52 +10,55 @@ if core.symlinks is false.'
 
 . ./test-lib.sh
 
-test_expect_success \
-'setup' '
-git config core.symlinks false &&
-> file &&
-git add file &&
-git commit -m initial &&
-git branch b-symlink &&
-git branch b-file &&
-l=$(printf file | git hash-object -t blob -w --stdin) &&
-echo "120000 $l	symlink" | git update-index --index-info &&
-git commit -m master &&
-git checkout b-symlink &&
-l=$(printf file-different | git hash-object -t blob -w --stdin) &&
-echo "120000 $l	symlink" | git update-index --index-info &&
-git commit -m b-symlink &&
-git checkout b-file &&
-echo plain-file > symlink &&
-git add symlink &&
-git commit -m b-file'
-
-test_expect_success \
-'merge master into b-symlink, which has a different symbolic link' '
-git checkout b-symlink &&
-test_must_fail git merge master'
-
-test_expect_success \
-'the merge result must be a file' '
-test -f symlink'
-
-test_expect_success \
-'merge master into b-file, which has a file instead of a symbolic link' '
-git reset --hard && git checkout b-file &&
-test_must_fail git merge master'
-
-test_expect_success \
-'the merge result must be a file' '
-test -f symlink'
-
-test_expect_success \
-'merge b-file, which has a file instead of a symbolic link, into master' '
-git reset --hard &&
-git checkout master &&
-test_must_fail git merge b-file'
-
-test_expect_success \
-'the merge result must be a file' '
-test -f symlink'
+test_expect_success 'setup' '
+	git config core.symlinks false &&
+	touch file &&
+	git add file &&
+	git commit -m initial &&
+	git branch b-symlink &&
+	git branch b-file &&
+	l=$(printf file | git hash-object -t blob -w --stdin) &&
+	echo "120000 $l	symlink" |
+	git update-index --index-info &&
+	git commit -m master &&
+	git checkout b-symlink &&
+	l=$(printf file-different | git hash-object -t blob -w --stdin) &&
+	echo "120000 $l	symlink" |
+	git update-index --index-info &&
+	git commit -m b-symlink &&
+	git checkout b-file &&
+	echo plain-file >symlink &&
+	git add symlink &&
+	git commit -m b-file
+'
+
+test_expect_success 'merge master into b-symlink, which has a different symbolic link' '
+	git checkout b-symlink &&
+	test_must_fail git merge master
+'
+
+test_expect_success 'the merge result must be a file' '
+	test -f symlink
+'
+
+test_expect_success 'merge master into b-file, which has a file instead of a symbolic link' '
+	git reset --hard &&
+	git checkout b-file &&
+	test_must_fail git merge master
+'
+
+test_expect_success 'the merge result must be a file' '
+	test -f symlink
+'
+
+test_expect_success 'merge b-file, which has a file instead of a symbolic link, into master' '
+	git reset --hard &&
+	git checkout master &&
+	test_must_fail git merge b-file
+'
+
+test_expect_success 'the merge result must be a file' '
+	test -f symlink
+'
 
 test_done
-- 
2.20.1


```

## Shourya Shukla, 2020-01-16 20:36

Subject: [PATCH 2/3] t6025: replace pipe with redirection operator
Message-ID: <20200116203622.4694-3-shouryashukla.oo@gmail.com>
URL: https://gitlist.dev/e/20200116203622.4694-3-shouryashukla.oo%40gmail.com
In-Reply-To: <20200116203622.4694-1-shouryashukla.oo@gmail.com>

```
The exit code of pipes(|) are always ignored, which will create
errors in subsequent statements. Let's handle it by redirecting
its output to a file and capturing return values. Replace pipe
with redirect(>) operator.

Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>
---
 t/t6025-merge-symlinks.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t6025-merge-symlinks.sh b/t/t6025-merge-symlinks.sh
index b9219af659..41bae56ea9 100755
--- a/t/t6025-merge-symlinks.sh
+++ b/t/t6025-merge-symlinks.sh
@@ -18,13 +18,13 @@ test_expect_success 'setup' '
 	git branch b-symlink &&
 	git branch b-file &&
 	l=$(printf file | git hash-object -t blob -w --stdin) &&
-	echo "120000 $l	symlink" |
-	git update-index --index-info &&
+	echo "120000 $l	symlink" >foo &&
+	git update-index --index-info <foo &&
 	git commit -m master &&
 	git checkout b-symlink &&
 	l=$(printf file-different | git hash-object -t blob -w --stdin) &&
-	echo "120000 $l	symlink" |
-	git update-index --index-info &&
+	echo "120000 $l	symlink" >foo &&
+	git update-index --index-info <foo &&
 	git commit -m b-symlink &&
 	git checkout b-file &&
 	echo plain-file >symlink &&
-- 
2.20.1


```

## Shourya Shukla, 2020-01-16 20:36

Subject: [PATCH 3/3] t6025: use helpers to replace test -f <path>
Message-ID: <20200116203622.4694-4-shouryashukla.oo@gmail.com>
URL: https://gitlist.dev/e/20200116203622.4694-4-shouryashukla.oo%40gmail.com
In-Reply-To: <20200116203622.4694-1-shouryashukla.oo@gmail.com>

```
Take advantage of helper function 'test_path_is_file()' to
replace 'test -f' since the function makes the code more
readable and gives better error messages.

Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>
---
 t/t6025-merge-symlinks.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t6025-merge-symlinks.sh b/t/t6025-merge-symlinks.sh
index 41bae56ea9..ebbbc03f1d 100755
--- a/t/t6025-merge-symlinks.sh
+++ b/t/t6025-merge-symlinks.sh
@@ -38,7 +38,7 @@ test_expect_success 'merge master into b-symlink, which has a different symbolic
 '
 
 test_expect_success 'the merge result must be a file' '
-	test -f symlink
+	test_path_is_file symlink
 '
 
 test_expect_success 'merge master into b-file, which has a file instead of a symbolic link' '
@@ -48,7 +48,7 @@ test_expect_success 'merge master into b-file, which has a file instead of a sym
 '
 
 test_expect_success 'the merge result must be a file' '
-	test -f symlink
+	test_path_is_file symlink
 '
 
 test_expect_success 'merge b-file, which has a file instead of a symbolic link, into master' '
@@ -58,7 +58,7 @@ test_expect_success 'merge b-file, which has a file instead of a symbolic link,
 '
 
 test_expect_success 'the merge result must be a file' '
-	test -f symlink
+	test_path_is_file symlink
 '
 
 test_done
-- 
2.20.1


```

## Johannes Schindelin, 2020-01-16 21:42

Subject: Re: [PATCH 1/3] t6025: modernize style
Message-ID: <nycvar.QRO.7.76.6.2001162239400.46@tvgsbejvaqbjf.bet>
URL: https://gitlist.dev/e/nycvar.QRO.7.76.6.2001162239400.46%40tvgsbejvaqbjf.bet
In-Reply-To: <20200116203622.4694-2-shouryashukla.oo@gmail.com>

```
Hi,

On Fri, 17 Jan 2020, Shourya Shukla wrote:

> The tests in `t6025-merge-symlinks.sh` were written a long time ago, and
> has a lot of style violations, including the mixed-use of tabs and spaces,
> missing indentations, and other shell script style violations. Update it to
> match the CodingGuidelines.
>
> Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>
> ---

Sounds good. Just one nit:

>  t/t6025-merge-symlinks.sh | 97 ++++++++++++++++++++-------------------
>  1 file changed, 50 insertions(+), 47 deletions(-)
>
> diff --git a/t/t6025-merge-symlinks.sh b/t/t6025-merge-symlinks.sh
> index 433c4de08f..b9219af659 100755
> --- a/t/t6025-merge-symlinks.sh
> +++ b/t/t6025-merge-symlinks.sh
> @@ -10,52 +10,55 @@ if core.symlinks is false.'
>
>  . ./test-lib.sh
>
> -test_expect_success \
> -'setup' '
> -git config core.symlinks false &&
> -> file &&

Here, `file` is written as a 0-byte file, and...

> -git add file &&
> -git commit -m initial &&
> -git branch b-symlink &&
> -git branch b-file &&
> -l=$(printf file | git hash-object -t blob -w --stdin) &&
> -echo "120000 $l	symlink" | git update-index --index-info &&
> -git commit -m master &&
> -git checkout b-symlink &&
> -l=$(printf file-different | git hash-object -t blob -w --stdin) &&
> -echo "120000 $l	symlink" | git update-index --index-info &&
> -git commit -m b-symlink &&
> -git checkout b-file &&
> -echo plain-file > symlink &&
> -git add symlink &&
> -git commit -m b-file'
> -
> -test_expect_success \
> -'merge master into b-symlink, which has a different symbolic link' '
> -git checkout b-symlink &&
> -test_must_fail git merge master'
> -
> -test_expect_success \
> -'the merge result must be a file' '
> -test -f symlink'
> -
> -test_expect_success \
> -'merge master into b-file, which has a file instead of a symbolic link' '
> -git reset --hard && git checkout b-file &&
> -test_must_fail git merge master'
> -
> -test_expect_success \
> -'the merge result must be a file' '
> -test -f symlink'
> -
> -test_expect_success \
> -'merge b-file, which has a file instead of a symbolic link, into master' '
> -git reset --hard &&
> -git checkout master &&
> -test_must_fail git merge b-file'
> -
> -test_expect_success \
> -'the merge result must be a file' '
> -test -f symlink'
> +test_expect_success 'setup' '
> +	git config core.symlinks false &&
> +	touch file &&

... here we now use `touch` instead. We do prefer `>file` in this
instance, though, I think. At least we do not prohibit it.

Otherwise it looks very good!
Johannes

> +	git add file &&
> +	git commit -m initial &&
> +	git branch b-symlink &&
> +	git branch b-file &&
> +	l=$(printf file | git hash-object -t blob -w --stdin) &&
> +	echo "120000 $l	symlink" |
> +	git update-index --index-info &&
> +	git commit -m master &&
> +	git checkout b-symlink &&
> +	l=$(printf file-different | git hash-object -t blob -w --stdin) &&
> +	echo "120000 $l	symlink" |
> +	git update-index --index-info &&
> +	git commit -m b-symlink &&
> +	git checkout b-file &&
> +	echo plain-file >symlink &&
> +	git add symlink &&
> +	git commit -m b-file
> +'
> +
> +test_expect_success 'merge master into b-symlink, which has a different symbolic link' '
> +	git checkout b-symlink &&
> +	test_must_fail git merge master
> +'
> +
> +test_expect_success 'the merge result must be a file' '
> +	test -f symlink
> +'
> +
> +test_expect_success 'merge master into b-file, which has a file instead of a symbolic link' '
> +	git reset --hard &&
> +	git checkout b-file &&
> +	test_must_fail git merge master
> +'
> +
> +test_expect_success 'the merge result must be a file' '
> +	test -f symlink
> +'
> +
> +test_expect_success 'merge b-file, which has a file instead of a symbolic link, into master' '
> +	git reset --hard &&
> +	git checkout master &&
> +	test_must_fail git merge b-file
> +'
> +
> +test_expect_success 'the merge result must be a file' '
> +	test -f symlink
> +'
>
>  test_done
> --
> 2.20.1
>
>

```

## Johannes Schindelin, 2020-01-16 21:46

Subject: Re: [PATCH 0/3] t6025: updating tests
Message-ID: <nycvar.QRO.7.76.6.2001162243100.46@tvgsbejvaqbjf.bet>
URL: https://gitlist.dev/e/nycvar.QRO.7.76.6.2001162243100.46%40tvgsbejvaqbjf.bet
In-Reply-To: <20200116203622.4694-1-shouryashukla.oo@gmail.com>

```
Hi,

On Fri, 17 Jan 2020, Shourya Shukla wrote:

> Greetings everyone!
>
> This is my first ever contribution in the Open-Source Community and I chose Git
> for this purpose as I have been using this important tool to maintain my projects
> regularly.
>
> In this patch, I have:
>
>   - modernized these tests so that they meet current Git CodingGuidlines[1]
>   - replaced the pipe operator with the redirection operator so that one can
> 	detect the errors easily and precisely
>   - used helper function 'test_path_is_file()' to replace 'test -f' checks in
> 	in the program as it improves the readability of the code and provides
> 	better error messages
>
> Also, I have some questions to better my understanding of the code:
> 	- In the statement,
> 		> git hash-object -t blob -w --stdin
> 	  is it necessary to explicitly specify the type 'blob' of the hash-object?
> 	  I have this question because it is the default type of hash-object.

It is the default type, but:

1) the code is not broken, so why fix it?

2) it _might_ be possible that the default changes, or can be configured
   in the future. The original author might just have wanted to stay safe.

> 	- In the statement,
> 		> l=$(printf file | git hash-object -t blob -w --stdin)
> 	  I have not used the redirection operator as this sub-shell will be executed
> 	  separately, hence its error cannot be captured therefore the presence of '>'
> 	  will not matter. Will using '>' improve the code?

It will be enhanced, though:

	printf file >file &&
	l=$(git hash-object -t blob -w --stdin)

will have a non-zero exit code if the `hash_object` call fails.

>
> Thanks,
> Shourya Shukla
>
> Shourya Shukla (3):
>   t6025: modernize style
>   t6025: replace pipe with redirection operator
>   t6025: use helpers to replace test -f <path>

Apart from one little issue in the first patch, this all looks very good
to me.

Thanks,
Johannes

>
>  t/t6025-merge-symlinks.sh | 97 ++++++++++++++++++++-------------------
>  1 file changed, 50 insertions(+), 47 deletions(-)
>
> --
> 2.20.1
>

```

## Junio C Hamano, 2020-01-16 22:57

Subject: Re: [PATCH 2/3] t6025: replace pipe with redirection operator
Message-ID: <xmqqftgff1r0.fsf@gitster-ct.c.googlers.com>
URL: https://gitlist.dev/e/xmqqftgff1r0.fsf%40gitster-ct.c.googlers.com
In-Reply-To: <20200116203622.4694-3-shouryashukla.oo@gmail.com>

```
Shourya Shukla <shouryashukla.oo@gmail.com> writes:

> -	echo "120000 $l	symlink" |
> -	git update-index --index-info &&
> +	echo "120000 $l	symlink" >foo &&
> +	git update-index --index-info <foo &&

If we had "git" on the left-hand-side (i.e. upstream) of a pipe, it
would make sense to split the pipeline like this, but this (and the
other one this patch touches) is on the right side, whose exit
status is not lost.  And we are not in the business of preparing for
broken implementation of "echo".

So this rewrite is unnecessary and unwarranted.

By the way, I think the pipeline

	echo ... | git update-index --index-info &&

should be written on a single line in the previous step 1/3.

>  	git commit -m master &&
>  	git checkout b-symlink &&
>  	l=$(printf file-different | git hash-object -t blob -w --stdin) &&
> -	echo "120000 $l	symlink" |
> -	git update-index --index-info &&
> +	echo "120000 $l	symlink" >foo &&
> +	git update-index --index-info <foo &&
>  	git commit -m b-symlink &&
>  	git checkout b-file &&
>  	echo plain-file >symlink &&

```

## Junio C Hamano, 2020-01-16 22:58

Subject: Re: [PATCH 3/3] t6025: use helpers to replace test -f <path>
Message-ID: <xmqqblr3f1q3.fsf@gitster-ct.c.googlers.com>
URL: https://gitlist.dev/e/xmqqblr3f1q3.fsf%40gitster-ct.c.googlers.com
In-Reply-To: <20200116203622.4694-4-shouryashukla.oo@gmail.com>

```
Shourya Shukla <shouryashukla.oo@gmail.com> writes:

> Take advantage of helper function 'test_path_is_file()' to
> replace 'test -f' since the function makes the code more
> readable and gives better error messages.
>
> Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>
> ---
>  t/t6025-merge-symlinks.sh | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Makes sense.

> diff --git a/t/t6025-merge-symlinks.sh b/t/t6025-merge-symlinks.sh
> index 41bae56ea9..ebbbc03f1d 100755
> --- a/t/t6025-merge-symlinks.sh
> +++ b/t/t6025-merge-symlinks.sh
> @@ -38,7 +38,7 @@ test_expect_success 'merge master into b-symlink, which has a different symbolic
>  '
>  
>  test_expect_success 'the merge result must be a file' '
> -	test -f symlink
> +	test_path_is_file symlink
>  '
>  
>  test_expect_success 'merge master into b-file, which has a file instead of a symbolic link' '
> @@ -48,7 +48,7 @@ test_expect_success 'merge master into b-file, which has a file instead of a sym
>  '
>  
>  test_expect_success 'the merge result must be a file' '
> -	test -f symlink
> +	test_path_is_file symlink
>  '
>  
>  test_expect_success 'merge b-file, which has a file instead of a symbolic link, into master' '
> @@ -58,7 +58,7 @@ test_expect_success 'merge b-file, which has a file instead of a symbolic link,
>  '
>  
>  test_expect_success 'the merge result must be a file' '
> -	test -f symlink
> +	test_path_is_file symlink
>  '
>  
>  test_done

```

## Shourya Shukla, 2020-01-17 20:44

Subject: [PATCH 0/3] t6025: amended changes after suggestions from the community
Message-ID: <20200117204426.9347-1-shouryashukla.oo@gmail.com>
URL: https://gitlist.dev/e/20200117204426.9347-1-shouryashukla.oo%40gmail.com
In-Reply-To: <xmqqblr3f1q3.fsf@gitster-ct.c.googlers.com>

```
Greetings everyone!

I have made the changes in my patch on the advise of Junio C Hamano and 
Johannes Schindelin. Thank you for looking into my changes, this has given
me confidence to contribute more in the Git Community.

Thank you,
Shourya Shukla

Shourya Shukla (3):
  t6025: modernize style
  t6025: replace pipe with redirection operator
  t6025: use helpers to replace test -f <path>

 t/t6025-merge-symlinks.sh | 96 ++++++++++++++++++++-------------------
 1 file changed, 49 insertions(+), 47 deletions(-)

-- 
2.20.1


```

## Shourya Shukla, 2020-01-17 20:44

Subject: [PATCH 1/3] t6025: modernize style
Message-ID: <20200117204426.9347-2-shouryashukla.oo@gmail.com>
URL: https://gitlist.dev/e/20200117204426.9347-2-shouryashukla.oo%40gmail.com
In-Reply-To: <20200117204426.9347-1-shouryashukla.oo@gmail.com>

```
The tests in `t6025-merge-symlinks.sh` were written a long time ago, and
has a lot of style violations, including the mixed-use of tabs and spaces,
missing indentations, and other shell script style violations. Update it to
match the CodingGuidelines.

Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>
---
 t/t6025-merge-symlinks.sh | 97 ++++++++++++++++++++-------------------
 1 file changed, 50 insertions(+), 47 deletions(-)

diff --git a/t/t6025-merge-symlinks.sh b/t/t6025-merge-symlinks.sh
index 433c4de08f..7a19ba8520 100755
--- a/t/t6025-merge-symlinks.sh
+++ b/t/t6025-merge-symlinks.sh
@@ -10,52 +10,55 @@ if core.symlinks is false.'
 
 . ./test-lib.sh
 
-test_expect_success \
-'setup' '
-git config core.symlinks false &&
-> file &&
-git add file &&
-git commit -m initial &&
-git branch b-symlink &&
-git branch b-file &&
-l=$(printf file | git hash-object -t blob -w --stdin) &&
-echo "120000 $l	symlink" | git update-index --index-info &&
-git commit -m master &&
-git checkout b-symlink &&
-l=$(printf file-different | git hash-object -t blob -w --stdin) &&
-echo "120000 $l	symlink" | git update-index --index-info &&
-git commit -m b-symlink &&
-git checkout b-file &&
-echo plain-file > symlink &&
-git add symlink &&
-git commit -m b-file'
-
-test_expect_success \
-'merge master into b-symlink, which has a different symbolic link' '
-git checkout b-symlink &&
-test_must_fail git merge master'
-
-test_expect_success \
-'the merge result must be a file' '
-test -f symlink'
-
-test_expect_success \
-'merge master into b-file, which has a file instead of a symbolic link' '
-git reset --hard && git checkout b-file &&
-test_must_fail git merge master'
-
-test_expect_success \
-'the merge result must be a file' '
-test -f symlink'
-
-test_expect_success \
-'merge b-file, which has a file instead of a symbolic link, into master' '
-git reset --hard &&
-git checkout master &&
-test_must_fail git merge b-file'
-
-test_expect_success \
-'the merge result must be a file' '
-test -f symlink'
+test_expect_success 'setup' '
+	git config core.symlinks false &&
+	>file &&
+	git add file &&
+	git commit -m initial &&
+	git branch b-symlink &&
+	git branch b-file &&
+	l=$(printf file | git hash-object -t blob -w --stdin) &&
+	echo "120000 $l	symlink" |
+	git update-index --index-info &&
+	git commit -m master &&
+	git checkout b-symlink &&
+	l=$(printf file-different | git hash-object -t blob -w --stdin) &&
+	echo "120000 $l	symlink" |
+	git update-index --index-info &&
+	git commit -m b-symlink &&
+	git checkout b-file &&
+	echo plain-file >symlink &&
+	git add symlink &&
+	git commit -m b-file
+'
+
+test_expect_success 'merge master into b-symlink, which has a different symbolic link' '
+	git checkout b-symlink &&
+	test_must_fail git merge master
+'
+
+test_expect_success 'the merge result must be a file' '
+	test -f symlink
+'
+
+test_expect_success 'merge master into b-file, which has a file instead of a symbolic link' '
+	git reset --hard &&
+	git checkout b-file &&
+	test_must_fail git merge master
+'
+
+test_expect_success 'the merge result must be a file' '
+	test -f symlink
+'
+
+test_expect_success 'merge b-file, which has a file instead of a symbolic link, into master' '
+	git reset --hard &&
+	git checkout master &&
+	test_must_fail git merge b-file
+'
+
+test_expect_success 'the merge result must be a file' '
+	test -f symlink
+'
 
 test_done
-- 
2.20.1


```

## Shourya Shukla, 2020-01-17 20:44

Subject: [PATCH 2/3] t6025: replace pipe with redirection operator
Message-ID: <20200117204426.9347-3-shouryashukla.oo@gmail.com>
URL: https://gitlist.dev/e/20200117204426.9347-3-shouryashukla.oo%40gmail.com
In-Reply-To: <20200117204426.9347-1-shouryashukla.oo@gmail.com>

```
The exit code of pipes(|) are always ignored, which will create
errors in subsequent statements. Let's handle it by redirecting
its output to a file and capturing return values. Replace pipe
with redirect(>) operator.

Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>
---
 t/t6025-merge-symlinks.sh | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/t/t6025-merge-symlinks.sh b/t/t6025-merge-symlinks.sh
index 7a19ba8520..5136bf1e13 100755
--- a/t/t6025-merge-symlinks.sh
+++ b/t/t6025-merge-symlinks.sh
@@ -17,14 +17,13 @@ test_expect_success 'setup' '
 	git commit -m initial &&
 	git branch b-symlink &&
 	git branch b-file &&
-	l=$(printf file | git hash-object -t blob -w --stdin) &&
-	echo "120000 $l	symlink" |
-	git update-index --index-info &&
+	printf file >file &&
+	l=$(git hash-object -t blob -w --stdin) &&
+	echo "120000 $l	symlink" | git update-index --index-info &&
 	git commit -m master &&
 	git checkout b-symlink &&
 	l=$(printf file-different | git hash-object -t blob -w --stdin) &&
-	echo "120000 $l	symlink" |
-	git update-index --index-info &&
+	echo "120000 $l	symlink" | git update-index --index-info &&
 	git commit -m b-symlink &&
 	git checkout b-file &&
 	echo plain-file >symlink &&
-- 
2.20.1


```

## Shourya Shukla, 2020-01-17 20:44

Subject: [PATCH 3/3] t6025: use helpers to replace test -f <path>
Message-ID: <20200117204426.9347-4-shouryashukla.oo@gmail.com>
URL: https://gitlist.dev/e/20200117204426.9347-4-shouryashukla.oo%40gmail.com
In-Reply-To: <20200117204426.9347-1-shouryashukla.oo@gmail.com>

```
Take advantage of helper function 'test_path_is_file()' to
replace 'test -f' since the function makes the code more
readable and gives better error messages.

Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>
---
 t/t6025-merge-symlinks.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t6025-merge-symlinks.sh b/t/t6025-merge-symlinks.sh
index 5136bf1e13..18a204bb65 100755
--- a/t/t6025-merge-symlinks.sh
+++ b/t/t6025-merge-symlinks.sh
@@ -37,7 +37,7 @@ test_expect_success 'merge master into b-symlink, which has a different symbolic
 '
 
 test_expect_success 'the merge result must be a file' '
-	test -f symlink
+	test_path_is_file symlink
 '
 
 test_expect_success 'merge master into b-file, which has a file instead of a symbolic link' '
@@ -47,7 +47,7 @@ test_expect_success 'merge master into b-file, which has a file instead of a sym
 '
 
 test_expect_success 'the merge result must be a file' '
-	test -f symlink
+	test_path_is_file symlink
 '
 
 test_expect_success 'merge b-file, which has a file instead of a symbolic link, into master' '
@@ -57,7 +57,7 @@ test_expect_success 'merge b-file, which has a file instead of a symbolic link,
 '
 
 test_expect_success 'the merge result must be a file' '
-	test -f symlink
+	test_path_is_file symlink
 '
 
 test_done
-- 
2.20.1


```

## Eric Sunshine, 2020-01-17 21:15

Subject: Re: [PATCH 1/3] t6025: modernize style
Message-ID: <CAPig+cTW4hATiMYVE7kzcLqK3do9=K8X0nmYjWLOhabgOxZ-XA@mail.gmail.com>
URL: https://gitlist.dev/e/CAPig%2BcTW4hATiMYVE7kzcLqK3do9%3DK8X0nmYjWLOhabgOxZ-XA%40mail.gmail.com
In-Reply-To: <20200117204426.9347-2-shouryashukla.oo@gmail.com>

```
On Fri, Jan 17, 2020 at 3:45 PM Shourya Shukla
<shouryashukla.oo@gmail.com> wrote:
> [PATCH 1/3] t6025: modernize style

When sending a new version of a patch series, indicate this via
"[PATCH v2 1/3]", for instance. The -v option of "git format-patch"
can help automate this for you.

> The tests in `t6025-merge-symlinks.sh` were written a long time ago, and
> has a lot of style violations, including the mixed-use of tabs and spaces,
> missing indentations, and other shell script style violations. Update it to
> match the CodingGuidelines.
>
> Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>
> ---
> diff --git a/t/t6025-merge-symlinks.sh b/t/t6025-merge-symlinks.sh
> @@ -10,52 +10,55 @@ if core.symlinks is false.'
> +test_expect_success 'setup' '
> +       git config core.symlinks false &&
> +       >file &&
> +       git add file &&
> +       git commit -m initial &&
> +       git branch b-symlink &&
> +       git branch b-file &&
> +       l=$(printf file | git hash-object -t blob -w --stdin) &&
> +       echo "120000 $l symlink" |
> +       git update-index --index-info &&

As mentioned[1] in the review of v1, this should be written:

    echo "120000 $l symlink" | git update-index --index-info &&

[1]: https://lore.kernel.org/git/xmqqftgff1r0.fsf@gitster-ct.c.googlers.com/

> +       git commit -m master &&
> +       git checkout b-symlink &&
> +       l=$(printf file-different | git hash-object -t blob -w --stdin) &&
> +       echo "120000 $l symlink" |
> +       git update-index --index-info &&
> +       git commit -m b-symlink &&
> +       git checkout b-file &&
> +       echo plain-file >symlink &&
> +       git add symlink &&
> +       git commit -m b-file
> +'

```

## Eric Sunshine, 2020-01-17 21:24

Subject: Re: [PATCH 2/3] t6025: replace pipe with redirection operator
Message-ID: <CAPig+cQX=jB1KTKcOMVE9u0jX-ZXt_vQBndkzqqQWORu5iFxeA@mail.gmail.com>
URL: https://gitlist.dev/e/CAPig%2BcQX%3DjB1KTKcOMVE9u0jX-ZXt_vQBndkzqqQWORu5iFxeA%40mail.gmail.com
In-Reply-To: <20200117204426.9347-3-shouryashukla.oo@gmail.com>

```
On Fri, Jan 17, 2020 at 3:45 PM Shourya Shukla
<shouryashukla.oo@gmail.com> wrote:
> The exit code of pipes(|) are always ignored, which will create
> errors in subsequent statements. Let's handle it by redirecting
> its output to a file and capturing return values. Replace pipe
> with redirect(>) operator.

This is not an accurate description. The proper way to explain this is
that exit code of a command upstream of a pipe is lost; the exit code
of a command downstream is not lost. We don't want to lose the exit
code of a git command, so a git command should not be upstream. (We
don't care about non-git commands being upstream when that command's
exit code is not relevant.)

> Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>
> ---
> diff --git a/t/t6025-merge-symlinks.sh b/t/t6025-merge-symlinks.sh
> @@ -17,14 +17,13 @@ test_expect_success 'setup' '
>         git commit -m initial &&
>         git branch b-symlink &&
>         git branch b-file &&
> -       l=$(printf file | git hash-object -t blob -w --stdin) &&

This command is fine as-is. We are interested in the exit code of the
git command (which is correctly downstream), and we don't care about
the exit code of 'printf' (which is upstream), so there is no reason
to rewrite this to use temporary files instead.

> +       printf file >file &&
> +       l=$(git hash-object -t blob -w --stdin) &&

Sorry, but this just doesn't make sense. You're telling "git
hash-object" to take its input from the standard input stream, yet you
don't feed anything to it on that stream. If anything, it should have
been written like this:

     l=$(git hash-object -t blob -w --stdin <file) &&

however, as noted above, there is no reason to avoid pipes in this
case, so this rewrite is unnecessary.

By the way, it's hard to imagine that this test passed once this
change was made (and, if it did pass, then that would likely indicate
that the test is somehow flawed.)

```

## Junio C Hamano, 2020-01-17 21:28

Subject: Re: [PATCH 1/3] t6025: modernize style
Message-ID: <xmqqk15pdb7q.fsf@gitster-ct.c.googlers.com>
URL: https://gitlist.dev/e/xmqqk15pdb7q.fsf%40gitster-ct.c.googlers.com
In-Reply-To: <CAPig+cTW4hATiMYVE7kzcLqK3do9=K8X0nmYjWLOhabgOxZ-XA@mail.gmail.com>

```
Eric Sunshine <sunshine@sunshineco.com> writes:

>> +       l=$(printf file | git hash-object -t blob -w --stdin) &&
>> +       echo "120000 $l symlink" |
>> +       git update-index --index-info &&
>
> As mentioned[1] in the review of v1, this should be written:
>
>     echo "120000 $l symlink" | git update-index --index-info &&
>
> [1]: https://lore.kernel.org/git/xmqqftgff1r0.fsf@gitster-ct.c.googlers.com/
>
>> +       git commit -m master &&
>> +       git checkout b-symlink &&
>> +       l=$(printf file-different | git hash-object -t blob -w --stdin) &&
>> +       echo "120000 $l symlink" |
>> +       git update-index --index-info &&

Same here.  Funnily, 2/3 improves on this, but I agree that we
should get it right from the get-go.

>> +       git commit -m b-symlink &&
>> +       git checkout b-file &&
>> +       echo plain-file >symlink &&
>> +       git add symlink &&
>> +       git commit -m b-file
>> +'

```

## Shourya Shukla, 2020-01-18 08:33

Subject: [PATCH 0/3] t6025: updating tests
Message-ID: <20200118083326.9643-1-shouryashukla.oo@gmail.com>
URL: https://gitlist.dev/e/20200118083326.9643-1-shouryashukla.oo%40gmail.com
In-Reply-To: <CAPig+cQX=jB1KTKcOMVE9u0jX-ZXt_vQBndkzqqQWORu5iFxeA@mail.gmail.com>

```
Greetings everyone!

This is my first ever contribution in the Open-Source Community and I chose Git
for this purpose as I have been using this important tool to maintain my projects
regularly.

In this patch, I have:

  - modernized these tests so that they meet current Git CodingGuidlines[1]
  - replaced the pipe operator with the redirection operator so that one can
	detect the errors easily and precisely
  - used helper function 'test_path_is_file()' to replace 'test -f' checks in
	in the program as it improves the readability of the code and provides 
	better error messages

Also, I have some questions to better my understanding of the code:
	- In the statement, 
		> git hash-object -t blob -w --stdin
	  is it necessary to explicitly specify the type 'blob' of the hash-object?
	  I have this question because it is the default type of hash-object.
	- In the statement, 
		> l=$(printf file | git hash-object -t blob -w --stdin)
	  I have not used the redirection operator as this sub-shell will be executed 
	  separately, hence its error cannot be captured therefore the presence of '>' 
	  will not matter. Will using '>' improve the code?

Thanks,
Shourya Shukla

Shourya Shukla (3):
  t6025: modernize style
  t6025: replace pipe with redirection operator
  t6025: use helpers to replace test -f <path>

 t/t6025-merge-symlinks.sh | 97 ++++++++++++++++++++-------------------
 1 file changed, 50 insertions(+), 47 deletions(-)

-- 
2.20.1

```

## Shourya Shukla, 2020-01-18 08:33

Subject: [PATCH 1/3] t6025: modernize style
Message-ID: <20200118083326.9643-2-shouryashukla.oo@gmail.com>
URL: https://gitlist.dev/e/20200118083326.9643-2-shouryashukla.oo%40gmail.com
In-Reply-To: <20200118083326.9643-1-shouryashukla.oo@gmail.com>

```
The tests in `t6025-merge-symlinks.sh` were written a long time ago, and
has a lot of style violations, including the mixed-use of tabs and spaces,
missing indentations, and other shell script style violations. Update it to
match the CodingGuidelines.

Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>
---
 t/t6025-merge-symlinks.sh | 97 ++++++++++++++++++++-------------------
 1 file changed, 50 insertions(+), 47 deletions(-)

diff --git a/t/t6025-merge-symlinks.sh b/t/t6025-merge-symlinks.sh
index 433c4de08f..b9219af659 100755
--- a/t/t6025-merge-symlinks.sh
+++ b/t/t6025-merge-symlinks.sh
@@ -10,52 +10,55 @@ if core.symlinks is false.'
 
 . ./test-lib.sh
 
-test_expect_success \
-'setup' '
-git config core.symlinks false &&
-> file &&
-git add file &&
-git commit -m initial &&
-git branch b-symlink &&
-git branch b-file &&
-l=$(printf file | git hash-object -t blob -w --stdin) &&
-echo "120000 $l	symlink" | git update-index --index-info &&
-git commit -m master &&
-git checkout b-symlink &&
-l=$(printf file-different | git hash-object -t blob -w --stdin) &&
-echo "120000 $l	symlink" | git update-index --index-info &&
-git commit -m b-symlink &&
-git checkout b-file &&
-echo plain-file > symlink &&
-git add symlink &&
-git commit -m b-file'
-
-test_expect_success \
-'merge master into b-symlink, which has a different symbolic link' '
-git checkout b-symlink &&
-test_must_fail git merge master'
-
-test_expect_success \
-'the merge result must be a file' '
-test -f symlink'
-
-test_expect_success \
-'merge master into b-file, which has a file instead of a symbolic link' '
-git reset --hard && git checkout b-file &&
-test_must_fail git merge master'
-
-test_expect_success \
-'the merge result must be a file' '
-test -f symlink'
-
-test_expect_success \
-'merge b-file, which has a file instead of a symbolic link, into master' '
-git reset --hard &&
-git checkout master &&
-test_must_fail git merge b-file'
-
-test_expect_success \
-'the merge result must be a file' '
-test -f symlink'
+test_expect_success 'setup' '
+	git config core.symlinks false &&
+	touch file &&
+	git add file &&
+	git commit -m initial &&
+	git branch b-symlink &&
+	git branch b-file &&
+	l=$(printf file | git hash-object -t blob -w --stdin) &&
+	echo "120000 $l	symlink" |
+	git update-index --index-info &&
+	git commit -m master &&
+	git checkout b-symlink &&
+	l=$(printf file-different | git hash-object -t blob -w --stdin) &&
+	echo "120000 $l	symlink" |
+	git update-index --index-info &&
+	git commit -m b-symlink &&
+	git checkout b-file &&
+	echo plain-file >symlink &&
+	git add symlink &&
+	git commit -m b-file
+'
+
+test_expect_success 'merge master into b-symlink, which has a different symbolic link' '
+	git checkout b-symlink &&
+	test_must_fail git merge master
+'
+
+test_expect_success 'the merge result must be a file' '
+	test -f symlink
+'
+
+test_expect_success 'merge master into b-file, which has a file instead of a symbolic link' '
+	git reset --hard &&
+	git checkout b-file &&
+	test_must_fail git merge master
+'
+
+test_expect_success 'the merge result must be a file' '
+	test -f symlink
+'
+
+test_expect_success 'merge b-file, which has a file instead of a symbolic link, into master' '
+	git reset --hard &&
+	git checkout master &&
+	test_must_fail git merge b-file
+'
+
+test_expect_success 'the merge result must be a file' '
+	test -f symlink
+'
 
 test_done
-- 
2.20.1


```

## Shourya Shukla, 2020-01-18 08:33

Subject: [PATCH 2/3] t6025: replace pipe with redirection operator
Message-ID: <20200118083326.9643-3-shouryashukla.oo@gmail.com>
URL: https://gitlist.dev/e/20200118083326.9643-3-shouryashukla.oo%40gmail.com
In-Reply-To: <20200118083326.9643-1-shouryashukla.oo@gmail.com>

```
The exit code of pipes(|) are always ignored, which will create
errors in subsequent statements. Let's handle it by redirecting
its output to a file and capturing return values. Replace pipe
with redirect(>) operator.

Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>
---
 t/t6025-merge-symlinks.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t6025-merge-symlinks.sh b/t/t6025-merge-symlinks.sh
index b9219af659..41bae56ea9 100755
--- a/t/t6025-merge-symlinks.sh
+++ b/t/t6025-merge-symlinks.sh
@@ -18,13 +18,13 @@ test_expect_success 'setup' '
 	git branch b-symlink &&
 	git branch b-file &&
 	l=$(printf file | git hash-object -t blob -w --stdin) &&
-	echo "120000 $l	symlink" |
-	git update-index --index-info &&
+	echo "120000 $l	symlink" >foo &&
+	git update-index --index-info <foo &&
 	git commit -m master &&
 	git checkout b-symlink &&
 	l=$(printf file-different | git hash-object -t blob -w --stdin) &&
-	echo "120000 $l	symlink" |
-	git update-index --index-info &&
+	echo "120000 $l	symlink" >foo &&
+	git update-index --index-info <foo &&
 	git commit -m b-symlink &&
 	git checkout b-file &&
 	echo plain-file >symlink &&
-- 
2.20.1


```

## Shourya Shukla, 2020-01-18 08:33

Subject: [PATCH 3/3] t6025: use helpers to replace test -f <path>
Message-ID: <20200118083326.9643-4-shouryashukla.oo@gmail.com>
URL: https://gitlist.dev/e/20200118083326.9643-4-shouryashukla.oo%40gmail.com
In-Reply-To: <20200118083326.9643-1-shouryashukla.oo@gmail.com>

```
Take advantage of helper function 'test_path_is_file()' to
replace 'test -f' since the function makes the code more
readable and gives better error messages.

Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>
---
 t/t6025-merge-symlinks.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t6025-merge-symlinks.sh b/t/t6025-merge-symlinks.sh
index 41bae56ea9..ebbbc03f1d 100755
--- a/t/t6025-merge-symlinks.sh
+++ b/t/t6025-merge-symlinks.sh
@@ -38,7 +38,7 @@ test_expect_success 'merge master into b-symlink, which has a different symbolic
 '
 
 test_expect_success 'the merge result must be a file' '
-	test -f symlink
+	test_path_is_file symlink
 '
 
 test_expect_success 'merge master into b-file, which has a file instead of a symbolic link' '
@@ -48,7 +48,7 @@ test_expect_success 'merge master into b-file, which has a file instead of a sym
 '
 
 test_expect_success 'the merge result must be a file' '
-	test -f symlink
+	test_path_is_file symlink
 '
 
 test_expect_success 'merge b-file, which has a file instead of a symbolic link, into master' '
@@ -58,7 +58,7 @@ test_expect_success 'merge b-file, which has a file instead of a symbolic link,
 '
 
 test_expect_success 'the merge result must be a file' '
-	test -f symlink
+	test_path_is_file symlink
 '
 
 test_done
-- 
2.20.1


```

## Shourya Shukla, 2020-01-18 08:33

Subject: [PATCH v3 0/2] t025: amended changes after suggestions from the community
Message-ID: <20200118083326.9643-5-shouryashukla.oo@gmail.com>
URL: https://gitlist.dev/e/20200118083326.9643-5-shouryashukla.oo%40gmail.com
In-Reply-To: <20200118083326.9643-1-shouryashukla.oo@gmail.com>

```
Greetings everyone!

After some advise from Eric Sunshine, I have removed the commit to replace pipes
with redirection operators. Also, as Junio Habano and Eric Sunshine pointed out,
I made a small mistake while committing changes in my first commit and it has been
corrected as well.

Thanks,
Shourya Shukla

Shourya Shukla (2):
  t6025: modernize style
  t6025: use helpers to replace test -f <path>

 t/t6025-merge-symlinks.sh | 95 ++++++++++++++++++++-------------------
 1 file changed, 48 insertions(+), 47 deletions(-)

-- 
2.20.1


```

## Shourya Shukla, 2020-01-18 08:33

Subject: [PATCH v3 1/2] t6025: modernize style
Message-ID: <20200118083326.9643-6-shouryashukla.oo@gmail.com>
URL: https://gitlist.dev/e/20200118083326.9643-6-shouryashukla.oo%40gmail.com
In-Reply-To: <20200118083326.9643-1-shouryashukla.oo@gmail.com>

```
The tests in `t6025-merge-symlinks.sh` were written a long time ago, and
has a lot of style violations, including the mixed-use of tabs and spaces,
missing indentations, and other shell script style violations. Update it to
match the CodingGuidelines.

Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>
---
 t/t6025-merge-symlinks.sh | 95 ++++++++++++++++++++-------------------
 1 file changed, 48 insertions(+), 47 deletions(-)

diff --git a/t/t6025-merge-symlinks.sh b/t/t6025-merge-symlinks.sh
index 433c4de08f..d257dcf34d 100755
--- a/t/t6025-merge-symlinks.sh
+++ b/t/t6025-merge-symlinks.sh
@@ -10,52 +10,53 @@ if core.symlinks is false.'
 
 . ./test-lib.sh
 
-test_expect_success \
-'setup' '
-git config core.symlinks false &&
-> file &&
-git add file &&
-git commit -m initial &&
-git branch b-symlink &&
-git branch b-file &&
-l=$(printf file | git hash-object -t blob -w --stdin) &&
-echo "120000 $l	symlink" | git update-index --index-info &&
-git commit -m master &&
-git checkout b-symlink &&
-l=$(printf file-different | git hash-object -t blob -w --stdin) &&
-echo "120000 $l	symlink" | git update-index --index-info &&
-git commit -m b-symlink &&
-git checkout b-file &&
-echo plain-file > symlink &&
-git add symlink &&
-git commit -m b-file'
-
-test_expect_success \
-'merge master into b-symlink, which has a different symbolic link' '
-git checkout b-symlink &&
-test_must_fail git merge master'
-
-test_expect_success \
-'the merge result must be a file' '
-test -f symlink'
-
-test_expect_success \
-'merge master into b-file, which has a file instead of a symbolic link' '
-git reset --hard && git checkout b-file &&
-test_must_fail git merge master'
-
-test_expect_success \
-'the merge result must be a file' '
-test -f symlink'
-
-test_expect_success \
-'merge b-file, which has a file instead of a symbolic link, into master' '
-git reset --hard &&
-git checkout master &&
-test_must_fail git merge b-file'
-
-test_expect_success \
-'the merge result must be a file' '
-test -f symlink'
+test_expect_success 'setup' '
+	git config core.symlinks false &&
+	>file &&
+	git add file &&
+	git commit -m initial &&
+	git branch b-symlink &&
+	git branch b-file &&
+	l=$(printf file | git hash-object -t blob -w --stdin) &&
+	echo "120000 $l	symlink" | git update-index --index-info &&
+	git commit -m master &&
+	git checkout b-symlink &&
+	l=$(printf file-different | git hash-object -t blob -w --stdin) &&
+	echo "120000 $l	symlink" | git update-index --index-info &&
+	git commit -m b-symlink &&
+	git checkout b-file &&
+	echo plain-file >symlink &&
+	git add symlink &&
+	git commit -m b-file
+'
+
+test_expect_success 'merge master into b-symlink, which has a different symbolic link' '
+	git checkout b-symlink &&
+	test_must_fail git merge master
+'
+
+test_expect_success 'the merge result must be a file' '
+	test -f symlink
+'
+
+test_expect_success 'merge master into b-file, which has a file instead of a symbolic link' '
+	git reset --hard &&
+	git checkout b-file &&
+	test_must_fail git merge master
+'
+
+test_expect_success 'the merge result must be a file' '
+	test -f symlink
+'
+
+test_expect_success 'merge b-file, which has a file instead of a symbolic link, into master' '
+	git reset --hard &&
+	git checkout master &&
+	test_must_fail git merge b-file
+'
+
+test_expect_success 'the merge result must be a file' '
+	test -f symlink
+'
 
 test_done
-- 
2.20.1


```

## Shourya Shukla, 2020-01-18 08:33

Subject: [PATCH v3 2/2] t6025: use helpers to replace test -f <path>
Message-ID: <20200118083326.9643-7-shouryashukla.oo@gmail.com>
URL: https://gitlist.dev/e/20200118083326.9643-7-shouryashukla.oo%40gmail.com
In-Reply-To: <20200118083326.9643-1-shouryashukla.oo@gmail.com>

```
Take advantage of helper function 'test_path_is_file()' to
replace 'test -f' since the function makes the code more
readable and gives better error messages.

Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>
---
 t/t6025-merge-symlinks.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t6025-merge-symlinks.sh b/t/t6025-merge-symlinks.sh
index d257dcf34d..6c0a90d044 100755
--- a/t/t6025-merge-symlinks.sh
+++ b/t/t6025-merge-symlinks.sh
@@ -36,7 +36,7 @@ test_expect_success 'merge master into b-symlink, which has a different symbolic
 '
 
 test_expect_success 'the merge result must be a file' '
-	test -f symlink
+	test_path_is_file symlink
 '
 
 test_expect_success 'merge master into b-file, which has a file instead of a symbolic link' '
@@ -46,7 +46,7 @@ test_expect_success 'merge master into b-file, which has a file instead of a sym
 '
 
 test_expect_success 'the merge result must be a file' '
-	test -f symlink
+	test_path_is_file symlink
 '
 
 test_expect_success 'merge b-file, which has a file instead of a symbolic link, into master' '
@@ -56,7 +56,7 @@ test_expect_success 'merge b-file, which has a file instead of a symbolic link,
 '
 
 test_expect_success 'the merge result must be a file' '
-	test -f symlink
+	test_path_is_file symlink
 '
 
 test_done
-- 
2.20.1


```

## Junio C Hamano, 2020-01-21 21:57

Subject: Re: [PATCH v3 1/2] t6025: modernize style
Message-ID: <xmqqy2u08obx.fsf@gitster-ct.c.googlers.com>
URL: https://gitlist.dev/e/xmqqy2u08obx.fsf%40gitster-ct.c.googlers.com
In-Reply-To: <20200118083326.9643-6-shouryashukla.oo@gmail.com>

```
Shourya Shukla <shouryashukla.oo@gmail.com> writes:

> The tests in `t6025-merge-symlinks.sh` were written a long time ago, and
> has a lot of style violations, including the mixed-use of tabs and spaces,
> missing indentations, and other shell script style violations. Update it to
> match the CodingGuidelines.

Looks good.  Will queue.

Thanks.

```

## Junio C Hamano, 2020-01-21 21:58

Subject: Re: [PATCH v3 2/2] t6025: use helpers to replace test -f <path>
Message-ID: <xmqqtv4o8ob0.fsf@gitster-ct.c.googlers.com>
URL: https://gitlist.dev/e/xmqqtv4o8ob0.fsf%40gitster-ct.c.googlers.com
In-Reply-To: <20200118083326.9643-7-shouryashukla.oo@gmail.com>

```
Shourya Shukla <shouryashukla.oo@gmail.com> writes:

> Take advantage of helper function 'test_path_is_file()' to
> replace 'test -f' since the function makes the code more
> readable and gives better error messages.

Looks good.  Thanks.

```