Thread (5 messages) flat view 5 messages, 3 authors, 3d ago

Re: [PATCH 1/2] Update shell tests to use semantic functions

From: Junio C Hamano <hidden>
Date: 2026-09-05 02:17:13

"Mark C. Chu-Carroll via B4 Relay"
[off-list ref] writes:
-		if test_have_prereq MINGW && test -f /proc/$shell_pid/winpid
+		if test_have_prereq MINGW && test_path_is_file /proc/$shell_pid/winpid
This is Wrong.

First think what the point of "test_path_is_file" and other helpers
is.  Once you understood them, you would never write them as part of
conditional expressions like the above one.

A conditional statement like this "if" does *not* want to shout when
the test condition it used to switch between its "then" (and "else"
if it has it) is not satisfied.  If this "test -f" does not find the
named /proc/$shell_pid/winpid file, it does not mean that we
detected a breakage in the system that is being tested.

After looking at a handful of hunks from the top of this patch, it
seems there are many more similarly wrong conversions among them
than a few that are good, and I won't look at the remainder of the
[1/2] or [2/2].  Sifting the two large patches that lack sign-off
that span across many files and finding salvageable bits is not
something I would spend my weekend on.

This kind of conversion first needs to understand what each
invocation of "test -X" is really trying to check and why.
Brain-less mechanical conversion is not sufficient and will produce
a wrong conversion like the above one.


Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help