coverage: Replace color terminal tests with HTML output tests

[Zalathar]

The main goal of these tests is to prevent #119033 from regressing. We can't do that with the usual plain text coverage reports, because they don't use column numbers to slice source code. So we need to emit either text reports with colour, or HTML reports.

Coloured text reports turn out to have two main problems:

• There's no way to tell llvm-cov to emit ANSI escape sequences on Windows, which means we have to ignore the tests on Windows.
• The output files contain raw ANSI escape sequences, making them awkward to read.
  • We also can't just render them to SVG, as some other tests do, due to rendering issues with full-width characters.

We can avoid those problems by switching to HTML reports instead. The tradeoffs are:

• Extra code for normalizing HTML output.
• The HTML snapshots have a lot more text in them, though with a few line breaks inserted they're still fairly readable.

@rustbot label +A-code-coverage

[rustbot]

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rustbot]

Some changes occurred in src/tools/compiletest

cc @jieyouxu

[Swatinem]

lgtm, though things are still extremely hard to read in the snapshot output.

[Zalathar]

Yeah, my thinking is that if it's going to be somewhat unreadable either way, I'd prefer the kind of unreadable that works on Windows, and can at least be manually inspected with a text editor and web browser.

I can also play around with adding even more line breaks (to make the code stand out more), though I'm not sure how much difference that will make.

(One of my constraints is that I ultimately want the modified HTML output to still work and resemble the real thing, which is why I haven't just removed the gunk.)

[Zalathar]

I've also stumbled on a different reason to want HTML output tests: the text and HTML reporters have subtly different output in some cases.

See taiki-e/cargo-llvm-cov#324 (comment) for an example. In text reports, the implied else arm is indicated by a ^ on the following line. In the HTML report it's present in the markup, but completely invisible.

(In the short term, I plan to work around this by making zero-width spans expand backwards instead of forwards. Eventually I'd like to get branch coverage into a state where I feel good about discarding zero-width spans completely.)

[Mark-Simulacrum]

I don't have a strong opinion here. It looks like @oli-obk has been reviewing patches in this area, so r? @oli-obk

This seems fine on a surface level though.

[oli-obk]

• We also can't just render them to SVG, as some other tests do, due to rendering issues with full-width characters.

Do you have an example of where this is an issue?

[Zalathar]

Do you have an example of where this is an issue?

When I tried the terminal SVG renderer out a few weeks ago, any output with East Asian Wide characters (ひらがな, 漢字 etc) would end up rendering with the background colours misaligned from the actual text.

(IIRC, this was due to the fact that the terminal SVG renderer doesn't actually apply a “background colour” at the SVG level; it just creates background rectangles and then tries to align the text on top of it. That happens to work for fixed-width fonts with known metrics, but goes out of alignment whenever font fallback causes the SVG-handling code to have no idea how big the displayed characters are going to be.)

[Zalathar]

And as mentioned above, there are cases where llvm-cov produces subtly different output for text reports vs HTML reports (specifically involving regions extending beyond the end of a line) that make me want to be able to specifically test HTML output anyway.

[oli-obk]

Then we have the wide char issues with regular diagnostics, too.

I don't think it's good to have an unreviewable (on github) format. I'd like to use svg and just avoid wide chars in color tests

[Zalathar]

I could try to modify the tests to use different Unicode characters to work around anstyle-svg limitations, but then we still wouldn't be able to run/bless them on Windows (because llvm-cov has no flag to force ANSI escapes on Windows).

[oli-obk]

Which parts of the HTML output do you actually care about? Can we strip most of it or use filecheck instead?

[Zalathar]

The main things we care about are:

• Making sure the text didn't accidentally get mangled (e.g. by slicing in the middle of a UTF-8 code point)
• Making sure the regions of text that are highlighted are the ones that we expect to be highlighted

(That second point is one of the reasons why I abandoned my earlier efforts with anstyle-svg. If the rendered output can't be trusted to actually reflect the highlighted regions, then it loses a lot of its value.)

When I was preparing this PR, I made the decision to post-process the HTML as little as possible, so that I could open the actual HTML files in a browser and make sure the results looked sensible (and also inspect the actual markup if necessary).

But if you think direct GitHub reviewability is more important, there should be a lot of scope to post-process the HTML and slash it down into something that can realistically be inspected in a PR context. It would basically be plain text with a little bit of markup indicating colour-highlighted spans.

(The end result would be very close to running the colour-terminal tests, and then replacing ANSI escape codes with ad-hoc markup. Except that we'd be able to run the stripped-HTML tests on Windows.)

[oli-obk]

Thanks for the thorough explanation.

The main issue I see now is that if we go with this PR, people will just bless and forget, while reviewers will mostly give it a glance to see that it didn't get totally messed up. But small important things will not be noticed. Without explicit annotations for what the specific test cares about, I fear they'll regress at some point.

We can just see how it goes in practice if you're alright with that risk

[Zalathar]

@rustbot author

The above conversation has changed my priorities a bit, so I'm taking this off the review queue until I've had a chance to figure out how I want to proceed.

[bors]

☔ The latest upstream changes (presumably #125331) made this pull request unmergeable. Please resolve the merge conflicts.

[Dylan-DPC]

@Zalathar any updates on this? thanks

[Zalathar]

I don't intend to pursue this any time soon, so for now I'll just close it.