Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retire the legacy Makefile-based run-make test infra #136581

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1761,15 +1761,21 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
cmd.arg("--coverage-dump-path").arg(coverage_dump);
}

cmd.arg("--src-base").arg(builder.src.join("tests").join(suite));
cmd.arg("--build-base").arg(testdir(builder, compiler.host).join(suite));
cmd.arg("--src-root").arg(&builder.src);
cmd.arg("--src-test-suite-root").arg(builder.src.join("tests").join(suite));

// N.B. it's important to distinguish between the *root* build directory, the *host* build
// directory immediately under the root build directory, and the test-suite-specific build
// directory.
cmd.arg("--build-root").arg(&builder.out);
cmd.arg("--build-test-suite-root").arg(testdir(builder, compiler.host).join(suite));

// When top stage is 0, that means that we're testing an externally provided compiler.
// In that case we need to use its specific sysroot for tests to pass.
let sysroot = if builder.top_stage == 0 {
builder.initial_sysroot.clone()
} else {
builder.sysroot(compiler).to_path_buf()
builder.sysroot(compiler)
};

cmd.arg("--sysroot-base").arg(sysroot);
Expand Down
30 changes: 1 addition & 29 deletions src/doc/rustc-dev-guide/src/tests/compiletest.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,7 @@ The following test suites are available, with links for more information:

### General purpose test suite

[`run-make`](#run-make-tests) are general purpose tests using Rust programs (or
Makefiles (legacy)).
[`run-make`](#run-make-tests) are general purpose tests using Rust programs.

### Rustdoc test suites

Expand Down Expand Up @@ -396,14 +395,6 @@ your test, causing separate files to be generated for 32bit and 64bit systems.

### `run-make` tests

> **Note on phasing out `Makefile`s**
>
> We are planning to migrate all existing Makefile-based `run-make` tests
> to Rust programs. You should not be adding new Makefile-based `run-make`
> tests.
>
> See <https://github.com/rust-lang/rust/issues/121876>.

The tests in [`tests/run-make`] are general-purpose tests using Rust *recipes*,
which are small programs (`rmake.rs`) allowing arbitrary Rust code such as
`rustc` invocations, and is supported by a [`run_make_support`] library. Using
Expand All @@ -424,11 +415,6 @@ Compiletest directives like `//@ only-<target>` or `//@ ignore-<target>` are
supported in `rmake.rs`, like in UI tests. However, revisions or building
auxiliary via directives are not currently supported.

Two `run-make` tests are ported over to Rust recipes as examples:

- <https://github.com/rust-lang/rust/tree/master/tests/run-make/CURRENT_RUSTC_VERSION>
- <https://github.com/rust-lang/rust/tree/master/tests/run-make/a-b-a-linker-guard>

#### Quickly check if `rmake.rs` tests can be compiled

You can quickly check if `rmake.rs` tests can be compiled without having to
Expand Down Expand Up @@ -481,20 +467,6 @@ Then add a corresponding entry to `"rust-analyzer.linkedProjects"`
],
```

#### Using Makefiles (legacy)

<div class="warning">
You should avoid writing new Makefile-based `run-make` tests.
</div>

Each test should be in a separate directory with a `Makefile` indicating the
commands to run.

There is a [`tools.mk`] Makefile which you can include which provides a bunch of
utilities to make it easier to run commands and compare outputs. Take a look at
some of the other tests for some examples on how to get started.

[`tools.mk`]: https://github.com/rust-lang/rust/blob/master/tests/run-make/tools.mk
[`tests/run-make`]: https://github.com/rust-lang/rust/tree/master/tests/run-make
[`run_make_support`]: https://github.com/rust-lang/rust/tree/master/src/tools/run-make-support

Expand Down
7 changes: 1 addition & 6 deletions src/doc/rustc-dev-guide/src/tests/directives.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@
FIXME(jieyouxu) completely revise this chapter.
-->

Directives are special comments that tell compiletest how to build and interpret
a test. They must appear before the Rust source in the test. They may also
appear in `rmake.rs` or legacy Makefiles for [run-make
tests](compiletest.md#run-make-tests).
Directives are special comments that tell compiletest how to build and interpret a test. They must appear before the Rust source in the test. They may also appear in `rmake.rs` [run-make tests](compiletest.md#run-make-tests).

They are normally put after the short comment that explains the point of this
test. Compiletest test suites use `//@` to signal that a comment is a directive.
Expand Down Expand Up @@ -218,8 +215,6 @@ The following directives will check LLVM support:
[`aarch64-gnu-debug`]), which only runs a
subset of `run-make` tests. Other tests with this directive will not
run at all, which is usually not what you want.
- Notably, the [`aarch64-gnu-debug`] CI job *currently* only runs `run-make`
tests which additionally contain `clang` in their test name.

See also [Debuginfo tests](compiletest.md#debuginfo-tests) for directives for
ignoring debuggers.
Expand Down
23 changes: 0 additions & 23 deletions src/doc/rustc-dev-guide/src/tests/running.md
Original file line number Diff line number Diff line change
Expand Up @@ -240,28 +240,6 @@ and they may not run the same without those options.

## Running `run-make` tests

### Windows

Running the `run-make` test suite on Windows is a currently bit more involved.
There are numerous prerequisites and environmental requirements:

- Install msys2: <https://www.msys2.org/>
- Specify `MSYS2_PATH_TYPE=inherit` in `msys2.ini` in the msys2 installation directory, run the
following with `MSYS2 MSYS`:
- `pacman -Syuu`
- `pacman -S make`
- `pacman -S diffutils`
- `pacman -S binutils`
- `./x test run-make` (`./x test tests/run-make` doesn't work)

There is [on-going work][port-run-make] to not rely on `Makefile`s in the
run-make test suite. Once this work is completed, you can run the entire
`run-make` test suite on native Windows inside `cmd` or `PowerShell` without
needing to install and use MSYS2. As of <!--date-check --> Oct 2024, it is
already possible to run the vast majority of the `run-make` test suite outside
of MSYS2, but there will be failures for the tests that still use `Makefile`s
due to not finding `make`.

## Running tests on a remote machine

Tests may be run on a remote machine (e.g. to test builds for a different
Expand Down Expand Up @@ -406,4 +384,3 @@ If you encounter bugs or problems, don't hesitate to open issues on the
repository](https://github.com/rust-lang/rustc_codegen_gcc/).

[`tests/ui`]: https://github.com/rust-lang/rust/tree/master/tests/ui
[port-run-make]: https://github.com/rust-lang/rust/issues/121876
26 changes: 17 additions & 9 deletions src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,11 +215,15 @@ pub struct Config {
/// `None` then these tests will be ignored.
pub run_clang_based_tests_with: Option<String>,

/// The directory containing the tests to run
pub src_base: PathBuf,
/// The directory containing the sources.
pub src_root: PathBuf,
/// The directory containing the test suite sources. Must be a subdirectory of `src_root`.
pub src_test_suite_root: PathBuf,

/// The directory where programs should be built
pub build_base: PathBuf,
/// Root build directory (e.g. `build/`).
pub build_root: PathBuf,
/// Test suite specific build directory (e.g. `build/host/test/ui/`).
pub build_test_suite_root: PathBuf,

/// The directory containing the compiler sysroot
pub sysroot_base: PathBuf,
Expand Down Expand Up @@ -345,7 +349,7 @@ pub struct Config {

/// If true, this will generate a coverage file with UI test files that run `MachineApplicable`
/// diagnostics but are missing `run-rustfix` annotations. The generated coverage file is
/// created in `/<build_base>/rustfix_missing_coverage.txt`
/// created in `<test_suite_build_root>/rustfix_missing_coverage.txt`
pub rustfix_coverage: bool,

/// whether to run `tidy` (html-tidy) when a rustdoc test fails
Expand Down Expand Up @@ -799,12 +803,16 @@ pub const UI_STDERR_16: &str = "16bit.stderr";
pub const UI_COVERAGE: &str = "coverage";
pub const UI_COVERAGE_MAP: &str = "cov-map";

/// Absolute path to the directory where all output for all tests in the given
/// `relative_dir` group should reside. Example:
/// /path/to/build/host-tuple/test/ui/relative/
/// Absolute path to the directory where all output for all tests in the given `relative_dir` group
/// should reside. Example:
///
/// ```text
/// /path/to/build/host-tuple/test/ui/relative/
/// ```
///
/// This is created early when tests are collected to avoid race conditions.
pub fn output_relative_path(config: &Config, relative_dir: &Path) -> PathBuf {
config.build_base.join(relative_dir)
config.build_test_suite_root.join(relative_dir)
}

/// Generates a unique name for the test, such as `testname.revision.mode`.
Expand Down
41 changes: 14 additions & 27 deletions src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -709,11 +709,11 @@ impl TestProps {
/// returns a struct containing various parts of the directive.
fn line_directive<'line>(
line_number: usize,
comment: &str,
original_line: &'line str,
) -> Option<DirectiveLine<'line>> {
// Ignore lines that don't start with the comment prefix.
let after_comment = original_line.trim_start().strip_prefix(comment)?.trim_start();
let after_comment =
original_line.trim_start().strip_prefix(COMPILETEST_DIRECTIVE_PREFIX)?.trim_start();

let revision;
let raw_directive;
Expand All @@ -722,7 +722,7 @@ fn line_directive<'line>(
// A comment like `//@[foo]` only applies to revision `foo`.
let Some((line_revision, after_close_bracket)) = after_open_bracket.split_once(']') else {
panic!(
"malformed condition directive: expected `{comment}[foo]`, found `{original_line}`"
"malformed condition directive: expected `{COMPILETEST_DIRECTIVE_PREFIX}[foo]`, found `{original_line}`"
)
};

Expand Down Expand Up @@ -836,6 +836,8 @@ pub(crate) fn check_directive<'a>(
CheckDirectiveResult { is_known_directive: is_known(&directive_name), trailing_directive }
}

const COMPILETEST_DIRECTIVE_PREFIX: &str = "//@";

fn iter_header(
mode: Mode,
_suite: &str,
Expand All @@ -849,8 +851,7 @@ fn iter_header(
}

// Coverage tests in coverage-run mode always have these extra directives, without needing to
// specify them manually in every test file. (Some of the comments below have been copied over
// from the old `tests/run-make/coverage-reports/Makefile`, which no longer exists.)
// specify them manually in every test file.
//
// FIXME(jieyouxu): I feel like there's a better way to do this, leaving for later.
if mode == Mode::CoverageRun {
Expand All @@ -867,9 +868,6 @@ fn iter_header(
}
}

// NOTE(jieyouxu): once we get rid of `Makefile`s we can unconditionally check for `//@`.
let comment = if testfile.extension().is_some_and(|e| e == "rs") { "//@" } else { "#" };

let mut rdr = BufReader::with_capacity(1024, rdr);
let mut ln = String::new();
let mut line_number = 0;
Expand All @@ -882,7 +880,7 @@ fn iter_header(
}
let ln = ln.trim();

let Some(directive_line) = line_directive(line_number, comment, ln) else {
let Some(directive_line) = line_directive(line_number, ln) else {
continue;
};

Expand Down Expand Up @@ -1026,19 +1024,6 @@ impl Config {
}
}

pub fn find_rust_src_root(&self) -> Option<PathBuf> {
let mut path = self.src_base.clone();
let path_postfix = Path::new("src/etc/lldb_batchmode.py");

while path.pop() {
if path.join(&path_postfix).is_file() {
return Some(path);
}
}

None
}

fn parse_edition(&self, line: &str) -> Option<String> {
self.parse_name_value_directive(line, "edition")
}
Expand Down Expand Up @@ -1083,10 +1068,11 @@ impl Config {
}
}

// FIXME(jieyouxu): fix some of these variable names to more accurately reflect what they do.
fn expand_variables(mut value: String, config: &Config) -> String {
const CWD: &str = "{{cwd}}";
const SRC_BASE: &str = "{{src-base}}";
const BUILD_BASE: &str = "{{build-base}}";
const TEST_SUITE_BUILD_BASE: &str = "{{build-base}}";
const RUST_SRC_BASE: &str = "{{rust-src-base}}";
const SYSROOT_BASE: &str = "{{sysroot-base}}";
const TARGET_LINKER: &str = "{{target-linker}}";
Expand All @@ -1098,15 +1084,16 @@ fn expand_variables(mut value: String, config: &Config) -> String {
}

if value.contains(SRC_BASE) {
value = value.replace(SRC_BASE, &config.src_base.to_string_lossy());
value = value.replace(SRC_BASE, &config.src_test_suite_root.to_str().unwrap());
}

if value.contains(BUILD_BASE) {
value = value.replace(BUILD_BASE, &config.build_base.to_string_lossy());
if value.contains(TEST_SUITE_BUILD_BASE) {
value =
value.replace(TEST_SUITE_BUILD_BASE, &config.build_test_suite_root.to_str().unwrap());
}

if value.contains(SYSROOT_BASE) {
value = value.replace(SYSROOT_BASE, &config.sysroot_base.to_string_lossy());
value = value.replace(SYSROOT_BASE, &config.sysroot_base.to_str().unwrap());
}

if value.contains(TARGET_LINKER) {
Expand Down
15 changes: 4 additions & 11 deletions src/tools/compiletest/src/header/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,10 @@ impl ConfigBuilder {
"--run-lib-path=",
"--python=",
"--jsondocck-path=",
"--src-base=",
"--build-base=",
"--src-root=",
"--src-test-suite-root=",
"--build-root=",
"--build-test-suite-root=",
"--sysroot-base=",
"--cc=c",
"--cxx=c++",
Expand Down Expand Up @@ -237,11 +239,6 @@ fn check_ignore(config: &Config, contents: &str) -> bool {
d.ignore
}

fn parse_makefile(config: &Config, contents: &str) -> EarlyProps {
let bytes = contents.as_bytes();
EarlyProps::from_reader(config, Path::new("Makefile"), bytes)
}

#[test]
fn should_fail() {
let config: Config = cfg().build();
Expand All @@ -259,10 +256,6 @@ fn revisions() {
let config: Config = cfg().build();

assert_eq!(parse_rs(&config, "//@ revisions: a b c").revisions, vec!["a", "b", "c"],);
assert_eq!(
parse_makefile(&config, "# revisions: hello there").revisions,
vec!["hello", "there"],
);
}

#[test]
Expand Down
Loading
Loading