Skip to content

src/tools/rustdoc-gui-test uses compiletest directives + TestProp handling internals in a questionable way #143827

Open
@jieyouxu

Description

@jieyouxu

I was wondering why compiletest's Config had a default, it turns out it's used by

if let Some(librs) = find_librs(entry.path()) {
let compiletest_c = compiletest::common::Config {
edition: None,
mode: compiletest::common::Mode::Rustdoc,
..Default::default()
};
let test_props = TestProps::from_file(
&camino::Utf8PathBuf::try_from(librs).unwrap(),
None,
&compiletest_c,
);
if !test_props.compile_flags.is_empty() {
cargo.env("RUSTDOCFLAGS", test_props.compile_flags.join(" "));
}
cargo.args(&test_props.run_flags);
}
if try_run(&mut cargo, config.verbose).is_err() {
eprintln!("failed to document `{}`", entry.path().display());
panic!("Cannot run rustdoc-gui tests");
}

to piggy-back off of compiletest's directive parsing and compile/run-flags extraction.

This happens to work because only //@ {compile,run}-flags directives are used, which do not otherwise happen to rely on other parts of the compiletest config or get validated in a way that requires other parts of the compiletest config (for now).

tests/rustdoc-gui/src/extend_css/lib.rs
1://@ compile-flags: --extend-css extra.css

tests/rustdoc-gui/src/theme_css/lib.rs
1://@ compile-flags: --theme custom-theme.css

tests/rustdoc-gui/src/scrape_examples/src/lib.rs
1://@ run-flags:-Zrustdoc-scrape-examples

tests/rustdoc-gui/src/link_to_definition/lib.rs
1://@ compile-flags: -Zunstable-options --generate-link-to-definition

IMO this is very questionable, because this bypasses any validation that compiletest might perform between parsing CLI args passed from bootstrap, and then constructing a validated Config. Furthermore, you'll appear to be able to use //@ only-xxx and other conditional test execution directives, where in reality they do not work at all (or are completely wrong) because the dummy config prevents compiletest conditional test directives from having any info to work with properly.

In other words, compiletest tries to be internally self-consistent / coherent, but it can't enforce that if its internals get exposed and "intercepted".

Possible mitigations / better approaches

  • If //@ {run,compile}-flags is all rustdoc-gui-test needs, it might be better to just reimplement a naive per-line //@ {compile,run}-flags.
  • Or try to merge rustdoc-gui-test into compiletest, but it runs tests with node in quite a different way versus other test suites.

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-test-infraArea: test infrastructure (may span bootstrap/compiletest/more)A-testsuiteArea: The testsuite used to check the correctness of rustcC-bugCategory: This is a bug.T-bootstrapRelevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)T-rustdocRelevant to the rustdoc team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions