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.