bootstrap: Consolidate coverage test suite steps into a single step

[Zalathar]

Now that I have more understanding of bootstrap steps, and a renewed distaste for unnecessary macros, I have managed to express the subtleties of the tests/coverage test suite in a single step defined in ordinary code, with no need for helper macros.

Deciding which modes to run is still a bit clunky due to limitations in existing ShouldRun/PathSet APIs, but I think it's a net improvement over having to declare several different steps to handle the suite path and aliases.

The interaction with --skip isn't as nice as I'd like, but all of the known limitations are limitations that already existed in the previous implementation.

One minor change is that by default compiletest is now invoked in coverage-run mode even when cross-compiling. However, in that situation compiletest still knows that it should skip all of the individual coverage-run tests.

r? jieyouxu (or reassign)

[jieyouxu]

I'm going to reroll because I'm not super sure about the PathSet matching logic. r? bootstrap

[Kobzol]

This is a nice simplification, thanks.

Something really weird is going on with the --skip logic though.
In

rust/src/bootstrap/src/core/builder/mod.rs

Line 403 in 089bd51

fn maybe_run(&self, builder: &Builder<'_>, mut pathsets: Vec<PathSet>) {

, a step is skipped only if all of its pathsets and aliases are skipped, which seems quite suspicious to me, that makes some default steps really hard to skip.

On the other hand, the logic in ensure_if_default skips the step if any of its paths is skipped, which makes much more sense to me.

At the very least, these two skip logics should probably be merged somehow.

In any case, this doesn't seem to make the status quo worse in this regard, as you stated in the PR description. I left two nits, otherwise you can r=me.

[Zalathar]

Addressed review feedback (diff).

[Kobzol]

Thanks! I'll approve once CI is green.

[Kobzol]

@bors r+

[bors]

📌 Commit 5298f85 has been approved by Kobzol

It is now in the queue for this repository.

[Zalathar]

, a step is skipped only if all of its pathsets and aliases are skipped, which seems quite suspicious to me, that makes some default steps really hard to skip.
On the other hand, the logic in ensure_if_default skips the step if any of its paths is skipped, which makes much more sense to me.

At the very least, these two skip logics should probably be merged somehow.

Note that a single step can have multiple paths representing different tools/crates, such that the different parts of the step can be run/skipped independently.

For example, the bootstrap tools src/tools/jsondoclint and src/tools/suggest-tests are tested by the same step (test::CrateBootstrap) via different paths. But passing --skip jsondoclint shouldn't cause the tests for suggest-tests to be skipped!

[Kobzol]

Well, I'd probably use separate steps for these things in that case 🤷‍♂️ It would be easier to reason about if a single step did one thing and all its paths were just aliases for that one thing. But of course this wouldn't really work for granular skipping, like "skip this one UI test from the whole UI tests suite", so it's a no-go.

Still, the skip logic at those two places should probably be somehow unified, I guess.