ladybird
ladybird copied to clipboard
LadybirdBrowser
CI: Move sanitizer output into a separate CI step
Instead of writing ASAN and UBSAN output into the same stream we use for test logging, direct them to log files, named asan.log.$PID and ubsan.log.$PID, and then output them in a separate CI job that runs afterwards. This should hopefully make it easier to see which tests are failing.
The downside is that it's now harder to tell which tests the *SAN errors are related to. Any ideas on that are appreciated. Right now, I think making the test logs actually readable is more important, (after all, we've had thousands-of-lines-long *SAN outputs for months and not solved those issues,) but maybe other people disagree.
For now, a failing workflow showing the results of this can be seen at https://github.com/AtkinsSJ/ladybird/actions/runs/11979541011?pr=1
Lol, I didn't need to link to my fork's failing tests, we've got the genuine san error output here!
We're likely going to leak allll sorts of things unless we take extra care to do 'full' GC collections b/w tests or destroy the VM on exit.
For some actual actionable feedback, I think that we should absolutely have this, and that it failing in the sanitizer step is appropriate.
A blurb of text to say "This failed because of a test, see the test step for more details" might be useful.
And, since there's 6k lines of LSAN output that we aren't sure why it's there, we should disable LSAN for the CI runs specifically. That would be export LSAN_OPTIONS=detect_leaks=0 in the CMakePresets.json(?) and perhaps in the CI yml as well. If that still doesn't disable it, we would want to remove (if present) detect_leaks from the ASAN_OPTIONS.
... and an issue to remind us to look into the detected leaks. I suspect (like I mentioned above) that it's related to not collecting the GC heap, but we have annotations on the GC heap to treat GC memory as LSAN roots. We also have a bunch of leaks that appear to be coming from ImageDecoder.
I've added the note to check the Test step. Though, that doesn't seem to print much that's useful other than giving you a rough idea of when it happened.
I've put an issue up about the leak: #2711
Otherwise, disabling it sounds good but I'll leave that for a separate PR, if nothing else than to make sure this one actually does print the output. :sweat_smile:
Changes: I thought putting ::notice in the log would make that line more visible, but it's actually a special workflow command that expects certain arguments, and was just printing ::notice verbatim. So, I've removed that.
The output-reduction Andrew suggested has been implemented separately by Tim in #2624 so I think that's everything addressed.