Rollup merge of #107657 - chenyukang:yukang/add-only-modified, r=albertlarsan68

Add only modified subcommand for compiletest

r? `@jyn514`

From [discussion](https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap/topic/Add.20subcommand.20to.20retest.20all.20tests.20with.20different.20results)

Cargo.lock

@@ -895,6 +895,7 @@ dependencies = [
 name = "compiletest"
 version = "0.0.0"
 dependencies = [
+ "build_helper",
  "colored",
  "diff",
  "getopts",

src/bootstrap/builder/tests.rs

@@ -557,6 +557,7 @@ mod dist {
             rustfix_coverage: false,
             pass: None,
             run: None,
+            only_modified: false,
         };
 
         let build = Build::new(config);
@@ -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();

src/bootstrap/flags.rs

@@ -124,6 +124,7 @@ pub enum Subcommand {
         fail_fast: bool,
         doc_tests: DocTests,
         rustfix_coverage: bool,
+        only_modified: bool,
     },
     Bench {
         paths: Vec<PathBuf>,
@@ -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");
                 opts.optopt(
                     "",
                     "compare-mode",
@@ -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") {
@@ -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,

src/bootstrap/format.rs

@@ -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};
@@ -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)]

src/bootstrap/test.rs

@@ -1510,6 +1510,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()));

src/tools/build_helper/src/git.rs

@@ -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
@@ -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()
@@ -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))
+}

src/tools/compiletest/Cargo.toml

@@ -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"

src/tools/compiletest/src/common.rs

@@ -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>,
 }
 

src/tools/compiletest/src/main.rs

@@ -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;
@@ -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");
@@ -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),
@@ -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.
@@ -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() {
@@ -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() };
@@ -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());