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

Add only modified subcommand for compiletest #107657

Merged
merged 2 commits into from
Feb 11, 2023
Merged
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
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -887,6 +887,7 @@ dependencies = [
name = "compiletest"
version = "0.0.0"
dependencies = [
"build_helper",
"colored",
"diff",
"getopts",
Expand Down
2 changes: 2 additions & 0 deletions src/bootstrap/builder/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,7 @@ mod dist {
rustfix_coverage: false,
pass: None,
run: None,
only_modified: false,
};

let build = Build::new(config);
Expand Down Expand Up @@ -627,6 +628,7 @@ mod dist {
rustfix_coverage: false,
pass: None,
run: None,
only_modified: false,
};
// Make sure rustfmt binary not being found isn't an error.
config.channel = "beta".to_string();
Expand Down
10 changes: 10 additions & 0 deletions src/bootstrap/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ pub enum Subcommand {
fail_fast: bool,
doc_tests: DocTests,
rustfix_coverage: bool,
only_modified: bool,
},
Bench {
paths: Vec<PathBuf>,
Expand Down Expand Up @@ -301,6 +302,7 @@ To learn more about a subcommand, run `./x.py <subcommand> -h`",
opts.optflag("", "doc", "only run doc tests");
opts.optflag("", "bless", "update all stderr/stdout files of failing ui tests");
opts.optflag("", "force-rerun", "rerun tests even if the inputs are unchanged");
opts.optflag("", "only-modified", "only run tests that result has been changed");
chenyukang marked this conversation as resolved.
Show resolved Hide resolved
opts.optopt(
"",
"compare-mode",
Expand Down Expand Up @@ -598,6 +600,7 @@ Arguments:
rustc_args: matches.opt_strs("rustc-args"),
fail_fast: !matches.opt_present("no-fail-fast"),
rustfix_coverage: matches.opt_present("rustfix-coverage"),
only_modified: matches.opt_present("only-modified"),
doc_tests: if matches.opt_present("doc") {
DocTests::Only
} else if matches.opt_present("no-doc") {
Expand Down Expand Up @@ -777,6 +780,13 @@ impl Subcommand {
}
}

pub fn only_modified(&self) -> bool {
match *self {
Subcommand::Test { only_modified, .. } => only_modified,
_ => false,
}
}

pub fn force_rerun(&self) -> bool {
match *self {
Subcommand::Test { force_rerun, .. } => force_rerun,
Expand Down
18 changes: 3 additions & 15 deletions src/bootstrap/format.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
//! Runs rustfmt on the repository.

use crate::builder::Builder;
use crate::util::{output, output_result, program_out_of_date, t};
use build_helper::git::updated_master_branch;
use crate::util::{output, program_out_of_date, t};
use build_helper::git::get_git_modified_files;
use ignore::WalkBuilder;
use std::collections::VecDeque;
use std::path::{Path, PathBuf};
Expand Down Expand Up @@ -80,23 +80,11 @@ fn update_rustfmt_version(build: &Builder<'_>) {
///
/// Returns `None` if all files should be formatted.
fn get_modified_rs_files(build: &Builder<'_>) -> Result<Option<Vec<String>>, String> {
let Ok(updated_master) = updated_master_branch(Some(&build.config.src)) else { return Ok(None); };

if !verify_rustfmt_version(build) {
return Ok(None);
}

let merge_base =
output_result(build.config.git().arg("merge-base").arg(&updated_master).arg("HEAD"))?;
Ok(Some(
output_result(
build.config.git().arg("diff-index").arg("--name-only").arg(merge_base.trim()),
)?
.lines()
.map(|s| s.trim().to_owned())
.filter(|f| Path::new(f).extension().map_or(false, |ext| ext == "rs"))
.collect(),
))
get_git_modified_files(Some(&build.config.src), &vec!["rs"])
}

#[derive(serde::Deserialize)]
Expand Down
4 changes: 4 additions & 0 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1508,6 +1508,10 @@ note: if you're sure you want to do this, please open an issue as to why. In the
if builder.config.rust_optimize_tests {
cmd.arg("--optimize-tests");
}
if builder.config.cmd.only_modified() {
cmd.arg("--only-modified");
}

let mut flags = if is_rustdoc { Vec::new() } else { vec!["-Crpath".to_string()] };
flags.push(format!("-Cdebuginfo={}", builder.config.rust_debuginfo_level_tests));
flags.extend(builder.config.cmd.rustc_args().iter().map(|s| s.to_string()));
Expand Down
72 changes: 65 additions & 7 deletions src/tools/build_helper/src/git.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,24 @@
use std::process::Stdio;
use std::{path::Path, process::Command};

/// Runs a command and returns the output
fn output_result(cmd: &mut Command) -> Result<String, String> {
let output = match cmd.stderr(Stdio::inherit()).output() {
Ok(status) => status,
Err(e) => return Err(format!("failed to run command: {:?}: {}", cmd, e)),
};
if !output.status.success() {
return Err(format!(
"command did not execute successfully: {:?}\n\
expected success, got: {}\n{}",
cmd,
output.status,
String::from_utf8(output.stderr).map_err(|err| format!("{err:?}"))?
));
}
Ok(String::from_utf8(output.stdout).map_err(|err| format!("{err:?}"))?)
}

/// Finds the remote for rust-lang/rust.
/// For example for these remotes it will return `upstream`.
/// ```text
Expand All @@ -14,13 +33,7 @@ pub fn get_rust_lang_rust_remote(git_dir: Option<&Path>) -> Result<String, Strin
git.current_dir(git_dir);
}
git.args(["config", "--local", "--get-regex", "remote\\..*\\.url"]);

let output = git.output().map_err(|err| format!("{err:?}"))?;
if !output.status.success() {
return Err("failed to execute git config command".to_owned());
}

let stdout = String::from_utf8(output.stdout).map_err(|err| format!("{err:?}"))?;
let stdout = output_result(&mut git)?;

let rust_lang_remote = stdout
.lines()
Expand Down Expand Up @@ -73,3 +86,48 @@ pub fn updated_master_branch(git_dir: Option<&Path>) -> Result<String, String> {
// We could implement smarter logic here in the future.
Ok("origin/master".into())
}

/// Returns the files that have been modified in the current branch compared to the master branch.
/// The `extensions` parameter can be used to filter the files by their extension.
/// If `extensions` is empty, all files will be returned.
pub fn get_git_modified_files(
git_dir: Option<&Path>,
extensions: &Vec<&str>,
) -> Result<Option<Vec<String>>, String> {
let Ok(updated_master) = updated_master_branch(git_dir) else { return Ok(None); };

let git = || {
let mut git = Command::new("git");
if let Some(git_dir) = git_dir {
git.current_dir(git_dir);
}
git
};

let merge_base = output_result(git().arg("merge-base").arg(&updated_master).arg("HEAD"))?;
let files = output_result(git().arg("diff-index").arg("--name-only").arg(merge_base.trim()))?
.lines()
.map(|s| s.trim().to_owned())
.filter(|f| {
Path::new(f).extension().map_or(false, |ext| {
extensions.is_empty() || extensions.contains(&ext.to_str().unwrap())
})
})
.collect();
Ok(Some(files))
}

/// Returns the files that haven't been added to git yet.
pub fn get_git_untracked_files(git_dir: Option<&Path>) -> Result<Option<Vec<String>>, String> {
let Ok(_updated_master) = updated_master_branch(git_dir) else { return Ok(None); };
let mut git = Command::new("git");
if let Some(git_dir) = git_dir {
git.current_dir(git_dir);
}

let files = output_result(git.arg("ls-files").arg("--others").arg("--exclude-standard"))?
.lines()
.map(|s| s.trim().to_owned())
.collect();
Ok(Some(files))
}
1 change: 1 addition & 0 deletions src/tools/compiletest/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ diff = "0.1.10"
unified-diff = "0.2.1"
getopts = "0.2"
miropt-test-tools = { path = "../miropt-test-tools" }
build_helper = { path = "../build_helper" }
tracing = "0.1"
tracing-subscriber = { version = "0.3.3", default-features = false, features = ["fmt", "env-filter", "smallvec", "parking_lot", "ansi"] }
regex = "1.0"
Expand Down
3 changes: 3 additions & 0 deletions src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,9 @@ pub struct Config {
/// Whether to rerun tests even if the inputs are unchanged.
pub force_rerun: bool,

/// Only rerun the tests that result has been modified accoring to Git status
pub only_modified: bool,

pub target_cfg: LazyCell<TargetCfg>,
}

Expand Down
56 changes: 50 additions & 6 deletions src/tools/compiletest/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,17 @@ extern crate test;
use crate::common::{expected_output_path, output_base_dir, output_relative_path, UI_EXTENSIONS};
use crate::common::{CompareMode, Config, Debugger, Mode, PassMode, TestPaths};
use crate::util::logv;
use build_helper::git::{get_git_modified_files, get_git_untracked_files};
use core::panic;
use getopts::Options;
use lazycell::LazyCell;
use std::env;
use std::ffi::OsString;
use std::fs;
use std::io::{self, ErrorKind};
use std::path::{Path, PathBuf};
use std::process::{Command, Stdio};
use std::time::SystemTime;
use std::{env, vec};
use test::ColorConfig;
use tracing::*;
use walkdir::WalkDir;
Expand Down Expand Up @@ -145,9 +147,10 @@ pub fn parse_config(args: Vec<String>) -> Config {
"",
"rustfix-coverage",
"enable this to generate a Rustfix coverage file, which is saved in \
`./<build_base>/rustfix_missing_coverage.txt`",
`./<build_base>/rustfix_missing_coverage.txt`",
)
.optflag("", "force-rerun", "rerun tests even if the inputs are unchanged")
.optflag("", "only-modified", "only run tests that result been modified")
.optflag("h", "help", "show this message")
.reqopt("", "channel", "current Rust channel", "CHANNEL")
.optopt("", "edition", "default Rust edition", "EDITION");
Expand Down Expand Up @@ -279,6 +282,7 @@ pub fn parse_config(args: Vec<String>) -> Config {
lldb_python_dir: matches.opt_str("lldb-python-dir"),
verbose: matches.opt_present("verbose"),
quiet: matches.opt_present("quiet"),
only_modified: matches.opt_present("only-modified"),
color,
remote_test_client: matches.opt_str("remote-test-client").map(PathBuf::from),
compare_mode: matches.opt_str("compare-mode").map(CompareMode::parse),
Expand Down Expand Up @@ -521,8 +525,18 @@ pub fn test_opts(config: &Config) -> test::TestOpts {
pub fn make_tests(config: &Config, tests: &mut Vec<test::TestDescAndFn>) {
debug!("making tests from {:?}", config.src_base.display());
let inputs = common_inputs_stamp(config);
collect_tests_from_dir(config, &config.src_base, &PathBuf::new(), &inputs, tests)
.unwrap_or_else(|_| panic!("Could not read tests from {}", config.src_base.display()));
let modified_tests = modified_tests(config, &config.src_base).unwrap_or_else(|err| {
panic!("modified_tests got error from dir: {}, error: {}", config.src_base.display(), err)
});
collect_tests_from_dir(
config,
&config.src_base,
&PathBuf::new(),
&inputs,
tests,
&modified_tests,
)
.unwrap_or_else(|_| panic!("Could not read tests from {}", config.src_base.display()));
}

/// Returns a stamp constructed from input files common to all test cases.
Expand Down Expand Up @@ -561,12 +575,35 @@ fn common_inputs_stamp(config: &Config) -> Stamp {
stamp
}

fn modified_tests(config: &Config, dir: &Path) -> Result<Vec<PathBuf>, String> {
if !config.only_modified {
return Ok(vec![]);
}
let files =
get_git_modified_files(Some(dir), &vec!["rs", "stderr", "fixed"])?.unwrap_or(vec![]);
// Add new test cases to the list, it will be convenient in daily development.
let untracked_files = get_git_untracked_files(None)?.unwrap_or(vec![]);

let all_paths = [&files[..], &untracked_files[..]].concat();
let full_paths = {
let mut full_paths: Vec<PathBuf> = all_paths
.into_iter()
.map(|f| fs::canonicalize(&f).unwrap().with_extension("").with_extension("rs"))
.collect();
full_paths.dedup();
full_paths.sort_unstable();
full_paths
};
Ok(full_paths)
}

fn collect_tests_from_dir(
config: &Config,
dir: &Path,
relative_dir_path: &Path,
inputs: &Stamp,
tests: &mut Vec<test::TestDescAndFn>,
modified_tests: &Vec<PathBuf>,
) -> io::Result<()> {
// Ignore directories that contain a file named `compiletest-ignore-dir`.
if dir.join("compiletest-ignore-dir").exists() {
Expand Down Expand Up @@ -597,7 +634,7 @@ fn collect_tests_from_dir(
let file = file?;
let file_path = file.path();
let file_name = file.file_name();
if is_test(&file_name) {
if is_test(&file_name) && (!config.only_modified || modified_tests.contains(&file_path)) {
debug!("found test file: {:?}", file_path.display());
let paths =
TestPaths { file: file_path, relative_dir: relative_dir_path.to_path_buf() };
Expand All @@ -607,7 +644,14 @@ fn collect_tests_from_dir(
let relative_file_path = relative_dir_path.join(file.file_name());
if &file_name != "auxiliary" {
debug!("found directory: {:?}", file_path.display());
collect_tests_from_dir(config, &file_path, &relative_file_path, inputs, tests)?;
collect_tests_from_dir(
config,
&file_path,
&relative_file_path,
inputs,
tests,
modified_tests,
)?;
}
} else {
debug!("found other file/directory: {:?}", file_path.display());
Expand Down