Re: [PATCH 10/15] t1306: convert `test_might_fail rm` to `rm -f`

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Denton Liu <liu.denton@xxxxxxxxx> writes:

> The test_must_fail() family of functions (including test_might_fail())
> should only be used on git commands. Replace `test_might_fail rm` with
> `rm -f` so that we don't use `test_might_fail` on a non-git command.

"rm -f X" can return non-zero status if "X" exists and cannot be
removed (e.g. "X" is a directory, X is in a directory you cannot
write to).  The only thing "-f" prevents the command from returning
non-zero status is when "X" does not exist.

That means that this change will change the behaviour.  Let's see if
it does in a good way or a bad way.

>  test_expect_success 'Checking attributes in a non-XDG global attributes file' '
> -	test_might_fail rm .gitattributes &&
> +	rm -f .gitattributes &&

This is so that leftover .gitattributes from previous tests will not
affect the outcome of this test.  If .gitattributes left by earlier
tests cannot be removed for whatever reason, we would want to know
about it, so changing to "rm -f" to make the tests more strict is a
good move.

>  	echo "f attr_f=test" >"$HOME"/my_gitattributes &&
>  	git config core.attributesfile "$HOME"/my_gitattributes &&
>  	echo "f: attr_f: test" >expected &&
> @@ -165,7 +165,7 @@ test_expect_success 'Checking attributes in a non-XDG global attributes file' '
>  test_expect_success 'write: xdg file exists and ~/.gitconfig doesn'\''t' '
>  	mkdir -p "$HOME"/.config/git &&
>  	>"$HOME"/.config/git/config &&
> -	test_might_fail rm "$HOME"/.gitconfig &&
> +	rm -f "$HOME"/.gitconfig &&

Likewise.

>  	git config --global user.name "write_config" &&
>  	echo "[user]" >expected &&
>  	echo "	name = write_config" >>expected &&
> @@ -183,8 +183,8 @@ test_expect_success 'write: xdg file exists and ~/.gitconfig exists' '
>  
>  
>  test_expect_success 'write: ~/.config/git/ exists and config file doesn'\''t' '
> -	test_might_fail rm "$HOME"/.gitconfig &&
> -	test_might_fail rm "$HOME"/.config/git/config &&
> +	rm -f "$HOME"/.gitconfig &&
> +	rm -f "$HOME"/.config/git/config &&

Likewise, but I think it makes more sense to remove them with a
single invocation of "rm -f".

>  	git config --global user.name "write_gitconfig" &&
>  	echo "[user]" >expected &&
>  	echo "	name = write_gitconfig" >>expected &&

Thanks.  In short, I think all of the changes in the patch are good.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux