Re: [PATCH] t/t7601: Modernize test scripts using functions
From: Junio C Hamano <hidden>
Date: 2023-10-16 16:53:58
Dorcas AnonoLitunya [off-list ref] writes:
Subject: Re: [PATCH] t/t7601: Modernize test scripts using functions
Let's try if we can pack a bit more information. For example Subject: [PATCH] t7601: use "test_path_is_file" etc. instead of "test -f" would clarify what kind of modernization is done by this patch.
The test script is currently using the command format 'test -f' to check for existence or absence of files.
"is currently using" -> "uses".
Replace it with new helper functions following the format 'test_path_is_file'.
I am not sure what role "the format" plays in this picture. test_path_is_file is not new---it has been around for quite a while.
Consequently, the patch also replaces the inverse command '! test -f' or 'test ! -f' with new helper function following the format 'test_path_is_missing'
A bit more on this later.
This adjustment using helper functions makes the code more readable and easier to understand.
Looking good. If I were writing this, I'll make the whole thing
more like this, though:
t7601: use "test_path_is_file" etc. instead of "test -f"
Some tests in t7601 use "test -f" and "test ! -f" to see if a
path exists or is missing. Use test_path_is_file and
test_path_is_missing helper functions to clarify these tests a
bit better. This especially matters for the "missing" case,
because "test ! -f F" will be happy if "F" exists as a
directory, but the intent of the test is that "F" should not
exist, even as a directory.
quoted hunk
diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh index bd238d89b0..e08767df66 100755 --- a/t/t7601-merge-pull-config.sh +++ b/t/t7601-merge-pull-config.sh@@ -349,13 +349,13 @@ test_expect_success 'Cannot rebase with multiple heads' ' test_expect_success 'merge c1 with c2' ' git reset --hard c1 && - test -f c0.c && - test -f c1.c && - test ! -f c2.c && - test ! -f c3.c && + test_path_is_file c0.c && + test_path_is_file c1.c && + test_path_is_missing c2.c && + test_path_is_missing c3.c &&
The original says "We are happy if c2.c is not a file", so it would have been happy if by some mistake "git reset" created a directory there. But the _intent_ of the test is that we do not have anything at c2.c, and the updated code expresses it better.