Hi Ævar, On Sun, 21 Mar 2021, Ævar Arnfjörð Bjarmason wrote: > On Fri, Mar 19 2021, Johannes Schindelin wrote: > > > On Tue, 9 Mar 2021, Ævar Arnfjörð Bjarmason wrote: > > > >> Rewrite the home-brew JUnit output generation added in > >> 22231908151 (tests: optionally write results as JUnit-style .xml, > >> 2019-01-29) and other subsequent commits to be generated via the TAP > >> output. > >> > >> This is now possible that the TAP output emitted with --tee is > >> guaranteed to be valid TAP, see the preceding commit. > >> > >> The JUnit output is not bug-for-bug compatible with the previous > >> output, but it looks better to me. > >> > >> This also requires installing TAP::Formatter::JUnit[1], perhaps that's > >> not palatable to the users of the --write-junit-xml option. > > > > Indeed. I am trying to keep the dependencies required for the Windows jobs > > of our CI/PR builds to a minimum. > > I'm taking this to mean the dependency of the TAP::Formatter::JUnit CPAN > module. Since the tests already depend on Perl, and presumably that Perl > isn't so broken as to not have the built in TAP modules > (e.g. TAP::Parser). I have no idea why you insist on modifying something that works quite well, but yeah, if you want to depend on the implementation detail that we _currently_ run even the Visual Studio tests (that do _not_ depend on `prove`) in an environment that has that dependency of `prove`, well, I guess I'll have to work on maintaining that state of affairs. Doesn't mean that it makes me happy. It really looks to me like a whole lot of work, without any benefit. > > Note, also, that the JUnit output was mostly relevant for when we used > > Azure Pipelines: it has a specific UI to study test results, figure out > > flaky tests, performance, etc. > > > > Now that we use GitHub Actions, we do not have such a nice test aggregator > > anymore, but we might get one again in the future, who knows. > > So there's no known current active user of this JUnit output target, or > at least if such a user exists it's not you anymore? > > I'm guessing we'd be unlikely to miss someone targeting JUnit from > git.git's tests who's not on this list to chime in. So is it not used > currently & could be removed? No, I want to keep it because I hope that we'll get that lost functionality back some time soon. Sorry for not spelling that out more clearly. > >> In any case, it'll be easy to whip up our own TAP emitter with a > >> TAP::Parser and TAP::Formatter, both of whom come with perl itself, > >> which we already rely on for tests. > >> > >> It should also be significantly faster on Windows, > > > > I really hate to have to harp on this when talking to you, but... well, > > how can I say it? Perl is _slooooooooooooow_ on Windows. > > > > Like, _really_ slow. Ridiculously slow. > > > > I know, you recently got riled up when Jeff suggested that the Perl hook > > calling FSMonior might be slow, but the truth is: it is super slow. It's > > not even funny how slow it is. And it fills me with no joy having to > > repeat it every time the question comes up whether running any part of Git > > that is written in Perl might affect performance on Windows. I really > > dislike having to be that messenger. > > Riled up? No, perplexed: yes. You're referring to > https://lore.kernel.org/git/87h7lgfchm.fsf@xxxxxxxxxxxxxxxxxxx/ > > So as an aside about that particular issue / slowness I haven't been > able to reproduce: > > If it's really a reference to some Windows-specific issue with Perl on > Windows in particular that would certainly be helpful for the rest of us > trying to follow along wondering where these wildly different > performance numbers come from. > > So not that we don't want to more to some other fsmonitor implementation ETOOMANYNEGATIONS, can't parse. > for other reasons, but the Perl part of that hook would be rather easy > to replace with some C JSON encoder or whatever. For the time being, I'd rather avoid discussing that because we already have an experimental, built-in FSMonitor that I personally use. That FSMonitor will nicely side-step all of the Perl slowness. > > I doubt that you will ever be able to replace my (admittedly slightly > > hacky) C helper with anything written in Perl that does even come close to > > being faster. > > > > In other words, I fear that your work here might not have the outcome you > > hoped for. > > We shell out to your C helper at least once for every test we run. By > converting the TAP after the full run we're only invoking Perl once > per-test. > > If that one Perl invocation is still much more expensive than doing that > ~20-50 times per-test we can easily move the loop to the Perl script and > invoke Perl once per test suite run. At that point you've got 1 run > v.s. >20k runs for the current helper being called in a loop by > test-lib.sh > > But I don't have access to a Windows test box. So maybe it's slower than > I'm imagining. How long do the equivalent of these take on Windows (not > sure if the time built-in is there)?: > > $ time perl -MTAP::Harness -wE 'say "hello world"' > hello world > > real 0m0.018s > user 0m0.013s > sys 0m0.005s > > $ time (echo hi | helper/test-tool xml-encode) > hi
 > real 0m0.002s > user 0m0.001s > sys 0m0.002s $ time perl -MTAP::Harness -wE 'say "hello world"' hello world real 0m0.326s user 0m0.030s sys 0m0.124s $ time perl -MTAP::Harness -wE 'say "hello world"' hello world real 0m0.131s user 0m0.061s sys 0m0.046s $ time perl -MTAP::Harness -wE 'say "hello world"' hello world real 0m0.209s user 0m0.015s sys 0m0.156s $ time (echo hi | t/helper/test-tool xml-encode) hi
 real 0m0.165s user 0m0.015s sys 0m0.093s $ time (echo hi | t/helper/test-tool xml-encode) hi
 real 0m0.092s user 0m0.000s sys 0m0.047s $ time (echo hi | t/helper/test-tool xml-encode) hi
 real 0m0.075s user 0m0.000s sys 0m0.046s Completely unscientific, but maybe it gives you an idea. > In any case, I regret focusing on the TAP::Formatter::JUnit part in this > RFC. The real meaty part is being able to have stable TAP output, and > thus not having to add N number of output formats into test-lib.sh > itself. > > A tool that consumes that can then be e.g. Perl's TAP tooling, or just > some C program or shellscript that calls the code that the below quoted > diff is removing (aside from the test-lib.sh part, which would go away). > > But it would be easier to write such tools using Perl's tooling, as it > saves one from writing a parser for TAP. > > I'd be surprised if Perl was adding much overhead in that context, and I > see that the existing Windows CI jobs are running through "prove", so > presumably even on Windows e.g. this: > > time make T=t000*.sh DEFAULT_TEST_TARGET=prove > > Is not much slower than: > > time make T=t000*.sh DEFAULT_TEST_TARGET=test > > I think this is the Nth time we've had some variant of this "Perl in > git's tests" discussion. And it is still an issue because the Perl we use is running on top of a POSIX emulation layer which makes everything slow. > I'm all for finding solutions to whatever issues you have, but I find it > confusing that in these discussions you seem to conflate and jump > between some or all of (just the ones I'm remembering): > > 1. Some imagined future state where there's no Perl whatsoever in the > test suite or git.git's build toolchain, which isn't the case now, > or anywhere in the forseeable future. > > 2. A future state where GFW etc. don't need to ship a Perl to users (I > believe this is either close or already there, I haven't kept up..) > > 3. The slowness of Perl for: tight loops, a few per test, once per test > suite run. > > 4. How #3 e.g. in a tight loop compares to calling say sed or awk in a > tight shell loop. Is perl uniquely bad on Windows, or is it really a > gripe about our use of shellscripting in general? > > 6. Not wanting to (understandably) package any more non-core Perl > stuff, understandable, but we always have the option of extending > perl/FromCPAN/ if we find something truly useful (in this case just > having a custom JUnit XML emitter, if it's actually needed by anyone > anymore, should be rather easy). > > So e.g. in this case I can guess at some of the gaps and *think* that > some code in t/ that uses TAP::Parser (a perl core module) and was > invoked either once per test-lib.sh invocation, or better (but perhaps > not needed, as it would make it a bit more complex) once per test-suite > run should be fine. > > That code could even be implemented as a prove(1) plugin, at which point > we'd be invoking Perl exactly as many times as we are now, but I'd > rather not paint myself into that particular corner without good reason. Yes, I would be much happier if Perl was not required to run our tests. I would be even happier if we did not run our tests through shell, especially performance tests, because running performance tests through shell is like getting the most precise stop watch you can think of and then looking away from the athletes while timing their sprint. If you provide patches to bring us closer to that reality, I will try to set aside time to help. But this "let's remove the C helper that serves us well and instead use Perl, just to shuffle things around" business really looks like a lot of churn to me, and I want to spend my time elsewhere. Ciao, Dscho
