git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Dec 17, 2019, 18:55 UTC
Message-ID
<xmqqk16uhjdo.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<d39422505f16a14c64514b8a78ae351f41b75c44.1576583819.git.liu.denton@gmail.com>
Denton Liu <liu.denton@gmail.com> 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.

Show 9 quoted lines
>  	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.
Show 11 quoted lines
>  	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.
Previous: Denton LiuNext: Denton Liu
Message 17 of 64 in “t: replace incorrect test_must_fail usage (part 1)”
  1. 00/15 t: replace incorrect test_must_fail usage (part 1)Denton Liu, Dec 17, 2019
  2. 01/15 test-lib-functions: introduce test_non_git_might_fail()Denton Liu, Dec 17, 2019
  3. 03/15 t0000: replace test_must_fail with ! for run_sub_test_lib_test()Denton Liu, Dec 17, 2019
  4. Johannes SixtDec 17, 2019
  5. 05/15 t0003: use test_must_be_empty()Denton Liu, Dec 17, 2019
  6. 04/15 t0003: use named parameters in attr_check()Denton Liu, Dec 17, 2019
  7. Junio C HamanoDec 17, 2019
  8. Eric SunshineDec 17, 2019
  9. 07/15 t0020: drop redirections to /dev/nullDenton Liu, Dec 17, 2019
  10. Eric SunshineDec 17, 2019
  11. 06/15 t0003: don't use `test_must_fail attr_check`Denton Liu, Dec 17, 2019
  12. Junio C HamanoDec 17, 2019
  13. 08/15 t0020: s/test_must_fail has_cr/! has_cr/Denton Liu, Dec 17, 2019
  14. Eric SunshineDec 17, 2019
  15. 11/15 t1307: reorder `nongit test_must_fail`Denton Liu, Dec 17, 2019
  16. 10/15 t1306: convert `test_might_fail rm` to `rm -f`Denton Liu, Dec 17, 2019
  17. Junio C HamanoDec 17, 2019
  18. 09/15 t0020: use ! check_packed_refs_markedDenton Liu, Dec 17, 2019
  19. 13/15 t1409: use test_path_is_missing()Denton Liu, Dec 17, 2019
  20. Junio C HamanoDec 17, 2019
  21. 14/15 t1501: remove use of `test_might_fail cp`Denton Liu, Dec 17, 2019
  22. Eric SunshineDec 17, 2019
  23. 15/15 t1507: teach full_name() to accept `!` argDenton Liu, Dec 17, 2019
  24. Eric SunshineDec 17, 2019
  25. 12/15 t1409: let sed open its own filesDenton Liu, Dec 17, 2019
  26. Junio C HamanoDec 17, 2019
  27. 02/15 t/lib-git-p4: use test_path_is_missing()Denton Liu, Dec 17, 2019
  28. Junio C HamanoDec 17, 2019
  29. 00/16 t: replace incorrect test_must_fail usage (part 1)Denton Liu, Dec 19, 2019
  30. 01/16 test-lib-functions: introduce test_non_git_might_fail()Denton Liu, Dec 19, 2019
  31. 02/16 t/lib-git-p4: use test_path_is_missing()Denton Liu, Dec 19, 2019
  32. 06/16 t0003: don't use `test_must_fail attr_check`Denton Liu, Dec 19, 2019
  33. 07/16 t0020: don't use `test_must_fail has_cr`Denton Liu, Dec 19, 2019
  34. 04/16 t0003: use named parameters in attr_check()Denton Liu, Dec 19, 2019
  35. 03/16 t0000: replace test_must_fail with run_sub_test_lib_test_err()Denton Liu, Dec 19, 2019
  36. 10/16 t1307: reorder `nongit test_must_fail`Denton Liu, Dec 19, 2019
  37. 11/16 t1409: let sed open its own input fileDenton Liu, Dec 19, 2019
  38. 12/16 t1409: use test_path_is_missing()Denton Liu, Dec 19, 2019
  39. 09/16 t1306: convert `test_might_fail rm` to `rm -f`Denton Liu, Dec 19, 2019
  40. 08/16 t0020: use ! check_packed_refs_markedDenton Liu, Dec 19, 2019
  41. 05/16 t0003: use test_must_be_empty()Denton Liu, Dec 19, 2019
  42. 13/16 t1501: remove use of `test_might_fail cp`Denton Liu, Dec 19, 2019
  43. Eric SunshineDec 19, 2019
  44. Denton LiuDec 19, 2019
  45. Eric SunshineDec 20, 2019
  46. 14/16 t1507: stop losing return codes of git commandsDenton Liu, Dec 19, 2019
  47. 16/16 t1507: inline full_name()Denton Liu, Dec 19, 2019
  48. 15/16 t1507: run commands within test_expect_successDenton Liu, Dec 19, 2019
  49. 00/15 t: replace incorrect test_must_fail usage (part 1)Denton Liu, Dec 20, 2019
  50. 01/15 t/lib-git-p4: use test_path_is_missing()Denton Liu, Dec 20, 2019
  51. 02/15 t0000: replace test_must_fail with run_sub_test_lib_test_err()Denton Liu, Dec 20, 2019
  52. 03/15 t0003: use named parameters in attr_check()Denton Liu, Dec 20, 2019
  53. 04/15 t0003: use test_must_be_empty()Denton Liu, Dec 20, 2019
  54. 06/15 t0020: don't use `test_must_fail has_cr`Denton Liu, Dec 20, 2019
  55. 05/15 t0003: don't use `test_must_fail attr_check`Denton Liu, Dec 20, 2019
  56. 08/15 t1306: convert `test_might_fail rm` to `rm -f`Denton Liu, Dec 20, 2019
  57. 07/15 t0020: use ! check_packed_refs_markedDenton Liu, Dec 20, 2019
  58. 11/15 t1409: use test_path_is_missing()Denton Liu, Dec 20, 2019
  59. 10/15 t1409: let sed open its own input fileDenton Liu, Dec 20, 2019
  60. 12/15 t1501: remove use of `test_might_fail cp`Denton Liu, Dec 20, 2019
  61. 13/15 t1507: stop losing return codes of git commandsDenton Liu, Dec 20, 2019
  62. 09/15 t1307: reorder `nongit test_must_fail`Denton Liu, Dec 20, 2019
  63. 14/15 t1507: run commands within test_expect_successDenton Liu, Dec 20, 2019
  64. 15/15 t1507: inline full_name()Denton Liu, Dec 20, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.