Re: [GSOC PATCH v2] t5315: use test_path_is_file for loose-object check
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 19, 2026, 18:58 UTC
- Message-ID
- <xmqqv7erk5oh.fsf@gitster.g>
- In-Reply-To
- <20260319180803.164335-1-elkhatabibilal@gmail.com>
Bilal El Khatabi <elkhatabibilal@gmail.com> writes:
Show 5 quoted lines
> Use test_path_is_file instead of test -f when checking that the > loose object was written to the expected path. > > This uses Git's path-checking helper, which provides more specific > failure output than a raw test -f check.
These two sentences repeat almost the same thing in different phrasing, which starts to become a bit boring.
The gold standard way to write a proposed commit log message for this project is to:
- Give an observation on how the current system works in the present tense (so no need to say "Currently X is Y", or "Previously X was Y" to describe the state before your change; just "X is Y" is enough), and discuss what you perceive as a problem in it.
- Propose a solution (optional---often, problem description trivially leads to an obvious solution in reader's minds).
- Give commands to somebody editing the codebase to "make it so", instead of saying "This commit does X".
in this order. The above jumps directly to the execution, bypassing the observation and solution. Fully followed, it would become something like:
When a test in t5315 tries to see if a loose object file is
created at the expected path in the filesystem, "test -f" is
used. If a developer breaks "git" in such a way that the file
is no longer created at this expected place, however, "test -f"
will silently fail and exits with non-zero, causing the test
fail, but it is not obvious which step in the &&- chained
commands failed. Use test_path_is_file helper, which loudly reports the failure
when the expectation is not met. When running the test with the
"-v" option, i.e., "sh t5315-*.sh -v", the failure would become
more prominent, helping the developer.Will queue.
Show 18 quoted lines
> Signed-off-by: Bilal El Khatabi <elkhatabibilal@gmail.com> > --- > t/t5315-pack-objects-compression.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/t5315-pack-objects-compression.sh b/t/t5315-pack-objects-compression.sh > index 8bacd96275..d0feab17b4 100755 > --- a/t/t5315-pack-objects-compression.sh > +++ b/t/t5315-pack-objects-compression.sh > @@ -10,7 +10,7 @@ test_expect_success setup ' > # make sure it resulted in a loose object > ob=$(sed -e "s/\(..\).*/\1/" object-name) && > ject=$(sed -e "s/..\(.*\)/\1/" object-name) && > - test -f .git/objects/$ob/$ject > + test_path_is_file .git/objects/$ob/$ject > ' > > while read expect config