[PATCH] D108031: [test] Avoid unportable echo in Other/lit-quoting.txt

Martin Storsjö via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 13 13:18:26 PDT 2021


mstorsjo added a comment.

In D108031#2944367 <https://reviews.llvm.org/D108031#2944367>, @rnk wrote:

> In D108031#2944321 <https://reviews.llvm.org/D108031#2944321>, @mstorsjo wrote:
>
>> I'm surprised to see that this actually seems to pass in the Windows CI environment (even if there are plenty of other unrelated failures in the CI run). On Windows, these tests aren't executed by a unix shell, but by the python lit internal shell, and `echo` is one of the commands handled internally there, but `printf` isn't.
>
> So, if I understand, the printf variant is actually a *better* test than the echo test because lit will actually call out to an external program. At least, I do believe this version does test the codepath that we care about, right? If you agree, I think this is fine.

Hmm, I'm pretty sure I tested these to make sure they did test the thing that mattered (invoking external executables - and that the test did fail before applying the fix).

In any case, I checked now, and both GnuWin (which I think some buildbots use?) and Git Bash (which the CI runner uses) contain a printf.exe, and it does seem to work with GnuWin's printf.exe too, so I guess it should be fine then.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108031/new/

https://reviews.llvm.org/D108031



More information about the llvm-commits mailing list