t/t0014: fix: eliminate additional lines from trace

[jglathe]

trace on git for windows delivers extra info which makes test
'run-command formats empty args properly' fail:

$ cat actual.raw
10:49:28.858579 exec-cmd.c:237 trace: resolved executable dir: C:/git-sdk-64/mingw64/bin
10:49:28.872991 git.c:702 trace: exec: git-frotz a '' b ' ' c
10:49:28.872991 run-command.c:663 trace: run_command: git-frotz a '' b ' ' c
10:49:28.872991 run-command.c:663 trace: run_command: 'C:\git-sdk-64\mingw64\bin\busybox.exe' --help
git: 'frotz' ist kein Git-Befehl. Siehe 'git --help'.

sed delivers 2 lines back. increase the recognition string

[rimrul]

@dscho and I had some prior discussion about this issue in the comments of #2495. I think we shouldn't look for busybox commands that start with git-, dscho seemed to favour a config option that controls wether we use busybox.

Your approach of filtering more strictly certainly narrows down what this test does to be closer to it's original intention, though.

[jglathe]

I just wanted the test to work and not crap out on aditional trace information, whatever that is. The sed command aims for the trace of the argument from run_command, so... why not aim for it directly?
The same test works on linux, and it isn't dependent on additional programs.

[dscho]

I wondered why the CI builds don't fail. After a while I figured it out: busybox.exe is missing when those tests are run, therefore this line is simply not printed.

But yes, I still think that it would be a lot better to turn this into a config option that piggy-backs onto the "very early" config in trace2_initialize().

[jglathe]

Now. All very well, but what about all the unknown cases due to future structural changes? The test shouldn‘t rely on getting only one line with run_command from trace. I mean, good that CI didn’t have the issue, *bad* that you have the issue on a windows box trying to do development (or whatever) for at least 6 months now. It should be portable code if possible. Von meinem iPhone gesendet

…

Am 02.06.2020 um 22:44 schrieb Johannes Schindelin ***@***.***>:  I wondered why the CI builds don't fail. After a while I figured it out: busybox.exe is missing when those tests are run, therefore this line is simply not printed. But yes, I still think that it would be a lot better to turn this into a config option that piggy-backs onto the "very early" config in trace2_initialize(). — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[dscho]

What I am trying to say is: I'd rather focus on fixing the underlying issue than on some fix that ignores the actual problem: that line that your patch excludes, that line never should be printed in the first place.

[jglathe]

Agreed.

…

------ Originalnachricht ------ Von: "Johannes Schindelin" <[email protected]> An: "git-for-windows/git" <[email protected]> Cc: "Jens Glathe" <[email protected]>; "Author" <[email protected]> Gesendet: 03.06.2020 08:32:20 Betreff: Re: [git-for-windows/git] t/t0014: fix: eliminate additional lines from trace (#2655)

What I am trying to say is: I'd rather focus on fixing the underlying issue than on some fix that ignores the actual problem: this line that your patch excludes, that line never should be printed in the first place. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2655 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AH5GA6E7EXWCFQTSHTWX2TLRUXU7JANCNFSM4NQVXJOQ>.

[dscho]

@jglathe thank you for contributing this fix. I had hoped to get around doing something more concrete about the BusyBox story, but obviously did not find the time. Therefore, I (finally!) admit that it is better to have your patch than no solution for the bug at all. Please accept my apologies for taking so long to come around.