Add `cargo semver-checks` testsuite to rustdoc JSON output

[jyn514]

Rustdoc has a ... not-very-comprensive test suite. cargo-semver-checks has much more extensive tests. It would be really nice if we could test that we don't accidentally regress the JSON output (or the other output for that matter; I suspect most breakage affects both backends).

I think the easiest way to do this is to add a new tests/rustdoc-semver-checks submodule with a dependency on https://github.com/obi1kenobi/cargo-semver-checks. Then in src/bootstrap/test.rs, we'll add a new test suite that runs cargo test on cargo-semver-checks using the in-tree version of rustdoc to make sure there's no incompatibilities.

I am not sure how this should deal with breaking changes to the JSON format - maybe we can use toolstate which allows the tests to be broken until semver-checks supports the new version?

cc @obi1kenobi @rust-lang/rustdoc

[Manishearth]

Big +1

[Manishearth]

I am not sure how this should deal with breaking changes to the JSON format - maybe we can use toolstate which allows the tests to be broken until semver-checks supports the new version?

We could, though semver-checks isn't an official tool. I'm also fine with having an internal toggle that we can use, and we try and coordinate with semver-checks as much as possible.

[jyn514]

I think if we're going to have some "it's ok to break this testsuite" state we should reuse the toolstate mechanisms. We don't need to rewrite our own const LET_SEMVER_CHECKS_BREAK = true; mechanism and integrate it with CI somehow.

[obi1kenobi]

Adding support for new format versions is generally extremely straightforward. I'd be happy to give folks involved with this effort push privs on cargo-semver-checks / trustfall-rustdoc-adapter / trustfall-rustdoc and write up the steps to follow so new formats can be supported ASAP. I obviously already try my best to add support quickly, but it's a known result in systems engineering that "min of multiple latencies" improves overall latency.

[Manishearth]

Yeah the mechanism seems fine, my worry is more about what we show publicly on toolstate.

[jyn514]

ah, gotcha - I think it should be possible to add some toggle to the toolstate website that hides semver-checks from the public site.

[aDotInTheVoid]

Talked about this on zulip, but TLDR:

• This can't be enforced by CI, as it makes breaking changes onerous
• If it's not in CI, unclear how much better is is than getting issues when these suites chatch them on nightly
• Would be nice to know how to run the test suites locally, with stage2 rustdoc
• Possible to use these testsuites to find gaps in our testsuite's coverage, and it's a parallel + good first issue to port them, after I write docs.

[Manishearth]

Actually this is a reason why I'm even more skeptical toolstate is the right tool for the job here: having I feel like "enforced by CI by easily disabled" is the best option. Toolstate is very much for cases where you expect breakage and want a dashboard of breakage, here we also want to catch unintentional breakage. For that we need CI, and the CI needs to be something you can disable when you do intend to break things.

[obi1kenobi]

From a purely technical perspective, I think "enforced by CI so long as the rustdoc format version number is still X" where X is a hardcoded constant representing the largest version supported by trustfall-rustdoc-adapter / cargo-semver-checks could be done. When the version number changes, this would auto-disable that CI, and it would need to be manually re-enabled after new format versions are released. I'd be happy to open PRs for that re-enabling, and it should also be quite easy to write a bot to do it automatically as well.

[Manishearth]

When you say rustdoc version do you mean rustdoc-json format version?

But yes that would work.

[obi1kenobi]

Yes, format version — edited above to add that.

[obi1kenobi]

If anyone is curious about the kinds of test cases that are part of trustfall-rustdoc-adapter and cargo-semver-checks, here I'm adding more cursed Rust: https://github.com/obi1kenobi/trustfall-rustdoc-adapter/pull/172/files#diff-2f839cd5fedffb289957c24c103dbe0f8ac63ed3158f177549fafe83e9781806R1

The bug in this case was in my code, not in rustdoc. It was found by running cargo-semver-checks across all versions of a large number of Rust crates in the ecosystem, followed by a combination of automatic and manual triage of the results. This process is ongoing and I'm happy to share more information if you're curious.

For an example of a Rust (maybe-)bug found as a result of the same run, see #110831.

It seems that "run lots of rustdoc-based queries on lots of crates then look for weird outcomes" is a viable strategy for proactive bug-finding in Rust.