| Message ID | d39422505f16a14c64514b8a78ae351f41b75c44.1576583819.git.liu.denton@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | t: replace incorrect test_must_fail usage (part 1) | expand |
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. > 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.
diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh index 21e139a313..dd87b43be1 100755 --- a/t/t1306-xdg-files.sh +++ b/t/t1306-xdg-files.sh @@ -153,7 +153,7 @@ test_expect_success 'Checking attributes in both XDG and local attributes files' test_expect_success 'Checking attributes in a non-XDG global attributes file' ' - test_might_fail rm .gitattributes && + rm -f .gitattributes && 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 && 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 && git config --global user.name "write_gitconfig" && echo "[user]" >expected && echo " name = write_gitconfig" >>expected &&
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. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- t/t1306-xdg-files.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)