Thanks for your GSoC microproject submission. See comments below...
On Tue, Mar 18, 2025 at 9:11 AM Aryan Pathania <contact@xxxxxxxx> wrote:
> Use `test_path_*` helper functions instead of `test -[efd]`
According to Documentation/SubmittingPatches, you'd probably instead
want to compose the commit message summary line something like this:
t9400: use test_path_*` functions to improve diagnostic output
> Change testcase `gitcvs.enabled = false` to check for missing path
> instead of a missing file. The change is justified as new assertion is
> stronger.
The description seems somehow outdated since this particular change is
being made to more than just that one test, isn't it?
Rather than describing the new assertion as "stronger", it might make
more sense to state that it is more semantically in line with what is
actually being tested (i.e. that the directory/path should not exist
when, as expected, the command fails).
> All other testcases remain equivalent.
Okay, but...
> @@ -296,8 +296,8 @@ test_expect_success 'gitcvs.ext.dbname' '
> test_cmp cvswork cvswork2 &&
> - test -f "$SERVERDIR/gitcvs1.ext.main.sqlite" &&
> - test ! -f "$SERVERDIR/gitcvs2.ext.main.sqlite" &&
> + test_path_is_file "$SERVERDIR/gitcvs1.ext.main.sqlite" &&
> + test_path_is_missing "$SERVERDIR/gitcvs2.ext.main.sqlite" &&
... although `test_path_is_file` is a faithful replacement for `test
-f`, it could be argued that `test_path_exists` would be a
semantically better choice, especially given the use of
`test_path_is_missing` for the sibling case.
The same comment applies to one or two other changes in this patch.
