Skip to content

Commit 8b27aa0

Browse files
committed
Break out tidy test step into its own module
1 parent bbee4a5 commit 8b27aa0

File tree

2 files changed

+131
-105
lines changed

2 files changed

+131
-105
lines changed

src/bootstrap/src/core/build_steps/test/mod.rs

Lines changed: 4 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ use crate::utils::helpers::{
3131
use crate::utils::render_tests::{add_flags_and_try_run_tests, try_run_tests};
3232
use crate::{CLang, DocTests, GitRepo, Mode, PathSet, envify};
3333

34+
mod tidy;
35+
36+
pub(crate) use tidy::Tidy;
37+
3438
const ADB_TEST_DIR: &str = "/data/local/tmp/work";
3539

3640
/// Runs `cargo test` on various internal tools used by bootstrap.
@@ -1031,111 +1035,6 @@ impl Step for RustdocGUI {
10311035
}
10321036
}
10331037

1034-
/// Runs `src/tools/tidy` and `cargo fmt --check` to detect various style
1035-
/// problems in the repository.
1036-
///
1037-
/// (To run the tidy tool's internal tests, use the alias "tidyselftest" instead.)
1038-
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
1039-
pub struct Tidy;
1040-
1041-
impl Step for Tidy {
1042-
type Output = ();
1043-
const DEFAULT: bool = true;
1044-
const ONLY_HOSTS: bool = true;
1045-
1046-
/// Runs the `tidy` tool.
1047-
///
1048-
/// This tool in `src/tools` checks up on various bits and pieces of style and
1049-
/// otherwise just implements a few lint-like checks that are specific to the
1050-
/// compiler itself.
1051-
///
1052-
/// Once tidy passes, this step also runs `fmt --check` if tests are being run
1053-
/// for the `dev` or `nightly` channels.
1054-
fn run(self, builder: &Builder<'_>) {
1055-
let mut cmd = builder.tool_cmd(Tool::Tidy);
1056-
cmd.arg(&builder.src);
1057-
cmd.arg(&builder.initial_cargo);
1058-
cmd.arg(&builder.out);
1059-
// Tidy is heavily IO constrained. Still respect `-j`, but use a higher limit if `jobs` hasn't been configured.
1060-
let jobs = builder.config.jobs.unwrap_or_else(|| {
1061-
8 * std::thread::available_parallelism().map_or(1, std::num::NonZeroUsize::get) as u32
1062-
});
1063-
cmd.arg(jobs.to_string());
1064-
if builder.is_verbose() {
1065-
cmd.arg("--verbose");
1066-
}
1067-
if builder.config.cmd.bless() {
1068-
cmd.arg("--bless");
1069-
}
1070-
if let Some(s) = builder.config.cmd.extra_checks() {
1071-
cmd.arg(format!("--extra-checks={s}"));
1072-
}
1073-
let mut args = std::env::args_os();
1074-
if args.any(|arg| arg == OsStr::new("--")) {
1075-
cmd.arg("--");
1076-
cmd.args(args);
1077-
}
1078-
1079-
if builder.config.channel == "dev" || builder.config.channel == "nightly" {
1080-
if !builder.config.json_output {
1081-
builder.info("fmt check");
1082-
if builder.initial_rustfmt().is_none() {
1083-
let inferred_rustfmt_dir = builder.initial_sysroot.join("bin");
1084-
eprintln!(
1085-
"\
1086-
ERROR: no `rustfmt` binary found in {PATH}
1087-
INFO: `rust.channel` is currently set to \"{CHAN}\"
1088-
HELP: if you are testing a beta branch, set `rust.channel` to \"beta\" in the `config.toml` file
1089-
HELP: to skip test's attempt to check tidiness, pass `--skip src/tools/tidy` to `x.py test`",
1090-
PATH = inferred_rustfmt_dir.display(),
1091-
CHAN = builder.config.channel,
1092-
);
1093-
crate::exit!(1);
1094-
}
1095-
let all = false;
1096-
crate::core::build_steps::format::format(
1097-
builder,
1098-
!builder.config.cmd.bless(),
1099-
all,
1100-
&[],
1101-
);
1102-
} else {
1103-
eprintln!(
1104-
"WARNING: `--json-output` is not supported on rustfmt, formatting will be skipped"
1105-
);
1106-
}
1107-
}
1108-
1109-
builder.info("tidy check");
1110-
cmd.delay_failure().run(builder);
1111-
1112-
builder.info("x.py completions check");
1113-
let [bash, zsh, fish, powershell] = ["x.py.sh", "x.py.zsh", "x.py.fish", "x.py.ps1"]
1114-
.map(|filename| builder.src.join("src/etc/completions").join(filename));
1115-
if builder.config.cmd.bless() {
1116-
builder.ensure(crate::core::build_steps::run::GenerateCompletions);
1117-
} else if get_completion(shells::Bash, &bash).is_some()
1118-
|| get_completion(shells::Fish, &fish).is_some()
1119-
|| get_completion(shells::PowerShell, &powershell).is_some()
1120-
|| crate::flags::get_completion(shells::Zsh, &zsh).is_some()
1121-
{
1122-
eprintln!(
1123-
"x.py completions were changed; run `x.py run generate-completions` to update them"
1124-
);
1125-
crate::exit!(1);
1126-
}
1127-
}
1128-
1129-
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
1130-
let default = run.builder.doc_tests != DocTests::Only;
1131-
run.path("src/tools/tidy").default_condition(default)
1132-
}
1133-
1134-
fn make_run(run: RunConfig<'_>) {
1135-
run.builder.ensure(Tidy);
1136-
}
1137-
}
1138-
11391038
fn testdir(builder: &Builder<'_>, host: TargetSelection) -> PathBuf {
11401039
builder.out.join(host).join("test")
11411040
}
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
//! Test step for `tidy` is somewhat special becomes it combines *two* roles:
2+
//!
3+
//! 1. Check code style (among other things) of *other* code in the source tree.
4+
//! 2. Running the `tidy` tool's *self-tests*.
5+
6+
// FIXME(#137178): currently, these two roles are combined into *one* step, presumably to make sure
7+
// that both steps get exercised in CI. However, this makes it annoying if you want to work on tidy
8+
// itself and *only* want to run tidy's self-tests (e.g. for faster iteration feedback).
9+
10+
use std::ffi::OsStr;
11+
12+
use clap_complete::shells;
13+
#[cfg(feature = "tracing")]
14+
use tracing::instrument;
15+
16+
use crate::DocTests;
17+
use crate::core::build_steps::tool::Tool;
18+
use crate::core::builder::{Builder, RunConfig, ShouldRun, Step};
19+
use crate::core::config::flags::get_completion;
20+
21+
/// Runs `src/tools/tidy` and `cargo fmt --check` to detect various style problems in the
22+
/// repository.
23+
///
24+
/// (To run the tidy tool's internal tests, use the alias "tidyselftest" instead.)
25+
// FIXME(#137178): break tidy self test out into its own step. If we still want to preserve the
26+
// current `./x test tidy` behavior, ensure the tidy self-test step *explicitly*.
27+
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
28+
pub struct Tidy;
29+
30+
impl Step for Tidy {
31+
type Output = ();
32+
const DEFAULT: bool = true;
33+
const ONLY_HOSTS: bool = true;
34+
35+
/// Runs the `tidy` tool.
36+
///
37+
/// This tool in `src/tools` checks up on various bits and pieces of style and
38+
/// otherwise just implements a few lint-like checks that are specific to the
39+
/// compiler itself.
40+
///
41+
/// Once tidy passes, this step also runs `fmt --check` if tests are being run
42+
/// for the `dev` or `nightly` channels.
43+
fn run(self, builder: &Builder<'_>) {
44+
let mut cmd = builder.tool_cmd(Tool::Tidy);
45+
cmd.arg(&builder.src);
46+
cmd.arg(&builder.initial_cargo);
47+
cmd.arg(&builder.out);
48+
// Tidy is heavily IO constrained. Still respect `-j`, but use a higher limit if `jobs`
49+
// hasn't been configured.
50+
let jobs = builder.config.jobs.unwrap_or_else(|| {
51+
8 * std::thread::available_parallelism().map_or(1, std::num::NonZeroUsize::get) as u32
52+
});
53+
cmd.arg(jobs.to_string());
54+
if builder.is_verbose() {
55+
cmd.arg("--verbose");
56+
}
57+
if builder.config.cmd.bless() {
58+
cmd.arg("--bless");
59+
}
60+
if let Some(s) = builder.config.cmd.extra_checks() {
61+
cmd.arg(format!("--extra-checks={s}"));
62+
}
63+
let mut args = std::env::args_os();
64+
if args.any(|arg| arg == OsStr::new("--")) {
65+
cmd.arg("--");
66+
cmd.args(args);
67+
}
68+
69+
if builder.config.channel == "dev" || builder.config.channel == "nightly" {
70+
if !builder.config.json_output {
71+
builder.info("fmt check");
72+
if builder.initial_rustfmt().is_none() {
73+
let inferred_rustfmt_dir = builder.initial_sysroot.join("bin");
74+
eprintln!(
75+
"\
76+
ERROR: no `rustfmt` binary found in {PATH}
77+
INFO: `rust.channel` is currently set to \"{CHAN}\"
78+
HELP: if you are testing a beta branch, set `rust.channel` to \"beta\" in the `config.toml` file
79+
HELP: to skip test's attempt to check tidiness, pass `--skip src/tools/tidy` to `x.py test`",
80+
PATH = inferred_rustfmt_dir.display(),
81+
CHAN = builder.config.channel,
82+
);
83+
crate::exit!(1);
84+
}
85+
let all = false;
86+
crate::core::build_steps::format::format(
87+
builder,
88+
!builder.config.cmd.bless(),
89+
all,
90+
&[],
91+
);
92+
} else {
93+
eprintln!(
94+
"WARNING: `--json-output` is not supported on rustfmt, formatting will be skipped"
95+
);
96+
}
97+
}
98+
99+
builder.info("tidy check");
100+
cmd.delay_failure().run(builder);
101+
102+
builder.info("x.py completions check");
103+
let [bash, zsh, fish, powershell] = ["x.py.sh", "x.py.zsh", "x.py.fish", "x.py.ps1"]
104+
.map(|filename| builder.src.join("src/etc/completions").join(filename));
105+
if builder.config.cmd.bless() {
106+
builder.ensure(crate::core::build_steps::run::GenerateCompletions);
107+
} else if get_completion(shells::Bash, &bash).is_some()
108+
|| get_completion(shells::Fish, &fish).is_some()
109+
|| get_completion(shells::PowerShell, &powershell).is_some()
110+
|| crate::flags::get_completion(shells::Zsh, &zsh).is_some()
111+
{
112+
eprintln!(
113+
"x.py completions were changed; run `x.py run generate-completions` to update them"
114+
);
115+
crate::exit!(1);
116+
}
117+
}
118+
119+
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
120+
let default = run.builder.doc_tests != DocTests::Only;
121+
run.path("src/tools/tidy").default_condition(default)
122+
}
123+
124+
fn make_run(run: RunConfig<'_>) {
125+
run.builder.ensure(Tidy);
126+
}
127+
}

0 commit comments

Comments
 (0)