adjust integration tests to rely on --repo-root using an absolute path

2bndy5 · 2bndy5 · commit fc628c5c438f · 2026-06-10T20:27:11.000-07:00
diff --git a/cpp-linter/src/clang_tools/clang_tidy.rs b/cpp-linter/src/clang_tools/clang_tidy.rs
@@ -2,9 +2,9 @@
 //! output.
 
 use std::{
-    env::{consts::OS, current_dir},
+    env::consts::OS,
     fs,
-    path::PathBuf,
+    path::{Path, PathBuf},
     process::Command,
     sync::{Arc, Mutex, MutexGuard},
 };
@@ -144,13 +144,13 @@ const NOTE_HEADER: &str = r"^(.+):(\d+):(\d+):\s(\w+):(.*)\[([a-zA-Z\d\-\.]+),?[
 fn parse_tidy_output(
     tidy_stdout: &[u8],
     database_json: &Option<Vec<CompilationUnit>>,
+    repo_root: &Path,
 ) -> Result<TidyAdvice, ClangCaptureError> {
     let note_header = Regex::new(NOTE_HEADER)?;
     let fixed_note = Regex::new(r"^.+:(\d+):\d+:\snote: FIX-IT applied suggested code changes$")?;
     let mut found_fix = false;
     let mut notification = None;
     let mut result = Vec::new();
-    let cur_dir = current_dir().map_err(ClangCaptureError::UnknownWorkingDirectory)?;
     for line in String::from_utf8(tidy_stdout.to_vec())
         .map_err(|e| ClangCaptureError::NonUtf8Output {
             task: "convert clang-tidy stdout".to_string(),
@@ -191,16 +191,16 @@ fn parse_tidy_output(
                         // file was not a named unit in the database;
                         // try to normalize path as if relative to working directory.
                         // NOTE: This shouldn't happen with a properly formed JSON database
-                        filename = normalize_path(&PathBuf::from_iter([&cur_dir, &filename]));
+                        filename = normalize_path(&PathBuf::from_iter([repo_root, &filename]));
                     }
                 } else {
                     // still need to normalize the relative path despite missing database info.
                     // let's assume the file is relative to current working directory.
-                    filename = normalize_path(&PathBuf::from_iter([&cur_dir, &filename]));
+                    filename = normalize_path(&PathBuf::from_iter([repo_root, &filename]));
                 }
-                debug_assert!(filename.is_absolute());
+
                 if filename.is_absolute()
-                    && let Ok(file_n) = filename.strip_prefix(&cur_dir)
+                    && let Ok(file_n) = filename.strip_prefix(repo_root)
                 {
                     // if this filename can't be made into a relative path, then it is
                     // likely not a member of the project's sources (ie /usr/include/stdio.h)
@@ -297,16 +297,17 @@ pub fn run_clang_tidy(
         );
         cmd.args(["--line-filter", filter.as_str()]);
     }
+    let repo_file_path = PathBuf::from_iter([&clang_params.repo_root, &file.name]);
     let original_content = if !clang_params.tidy_review {
         None
     } else {
         cmd.arg("--fix-errors");
-        Some(
-            fs::read_to_string(&file.name).map_err(|e| ClangCaptureError::ReadFileFailed {
+        Some(fs::read_to_string(&repo_file_path).map_err(|e| {
+            ClangCaptureError::ReadFileFailed {
                 file_name: file_name.clone(),
                 source: e,
-            })?,
-        )
+            }
+        })?)
     };
     if !clang_params.style.is_empty() {
         cmd.args(["--format-style", clang_params.style.as_str()]);
@@ -348,6 +349,7 @@ pub fn run_clang_tidy(
     file.tidy_advice = Some(parse_tidy_output(
         &output.stdout,
         &clang_params.database_json,
+        &clang_params.repo_root,
     )?);
     if clang_params.tidy_review
         && let Some(original_content) = &original_content
@@ -356,14 +358,14 @@ pub fn run_clang_tidy(
             // cache file changes in a buffer and restore the original contents for further analysis
             tidy_advice.patched =
                 Some(
-                    fs::read(&file_name).map_err(|e| ClangCaptureError::ReadFileFailed {
+                    fs::read(&repo_file_path).map_err(|e| ClangCaptureError::ReadFileFailed {
                         file_name: file_name.clone(),
                         source: e,
                     })?,
                 );
         }
         // original_content is guaranteed to be Some() value at this point
-        fs::write(&file_name, original_content).map_err(|e| {
+        fs::write(&repo_file_path, original_content).map_err(|e| {
             ClangCaptureError::WriteFileFailed {
                 file_name,
                 source: e,
@@ -565,7 +567,7 @@ TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:48:
    44 |   return diskFS.openFile(name) | kdl::and_then([](const auto& file) {
       |                                                ^
 "#;
-        let advice = parse_tidy_output(tidy_out.as_bytes(), &None).unwrap();
+        let advice = parse_tidy_output(tidy_out.as_bytes(), &None, &PathBuf::from(".")).unwrap();
         assert_eq!(advice.notes.len(), 4);
         for note in advice.notes {
             assert!(note.diagnostic.contains('-'));
diff --git a/cpp-linter/src/cli/structs.rs b/cpp-linter/src/cli/structs.rs
@@ -232,6 +232,7 @@ pub struct FeedbackInput {
     pub tidy_review: bool,
     pub format_review: bool,
     pub passive_reviews: bool,
+    pub repo_root: PathBuf,
 }
 
 #[cfg(feature = "bin")]
@@ -247,6 +248,7 @@ impl From<&Cli> for FeedbackInput {
             tidy_review: args.feedback_options.tidy_review,
             format_review: args.feedback_options.format_review,
             passive_reviews: args.feedback_options.passive_reviews,
+            repo_root: PathBuf::from(&args.source_options.repo_root),
         }
     }
 }
@@ -263,6 +265,7 @@ impl Default for FeedbackInput {
             tidy_review: false,
             format_review: false,
             passive_reviews: false,
+            repo_root: PathBuf::from("."),
         }
     }
 }
diff --git a/cpp-linter/src/common_fs/mod.rs b/cpp-linter/src/common_fs/mod.rs
@@ -1,6 +1,11 @@
 //! A module to hold all common file system functionality.
 
-use std::{fmt::Debug, fs, ops::RangeInclusive, path::PathBuf};
+use std::{
+    fmt::Debug,
+    fs,
+    ops::RangeInclusive,
+    path::{Path, PathBuf},
+};
 
 use gix_imara_diff::Hunk;
 
@@ -140,8 +145,10 @@ impl FileObj {
         &self,
         review_comments: &mut ReviewComments,
         summary_only: bool,
+        repo_root: &Path,
     ) -> Result<(), FileObjError> {
-        let original_content = fs::read_to_string(&self.name).map_err(FileObjError::ReadFile)?;
+        let original_content =
+            fs::read_to_string(repo_root.join(&self.name)).map_err(FileObjError::ReadFile)?;
         let file_name = self.name.to_str().unwrap_or_default().replace("\\", "/");
         if let Some(advice) = &self.format_advice {
             let patched = fs::read_to_string(&advice.patched).map_err(FileObjError::ReadFile)?;
diff --git a/cpp-linter/src/git.rs b/cpp-linter/src/git.rs
@@ -9,6 +9,7 @@ mod test {
     use std::{
         env::{self, current_dir, set_current_dir},
         fs,
+        path::PathBuf,
         process::Command,
     };
 
@@ -101,7 +102,7 @@ mod test {
                 &LinesChangedOnly::Off.into(),
                 &base_diff,
                 ignore_staged,
-                ".",
+                &PathBuf::from("."),
             )
             .await
             .unwrap()
diff --git a/cpp-linter/src/rest_client/mod.rs b/cpp-linter/src/rest_client/mod.rs
@@ -1,6 +1,6 @@
 use std::{
     env,
-    path::PathBuf,
+    path::{Path, PathBuf},
     sync::{Arc, Mutex},
 };
 
@@ -54,7 +54,7 @@ impl RestClient {
         lines_changed_only: &LinesChangedOnly,
         base_diff: &Option<String>,
         ignore_index: bool,
-        repo_root: &str,
+        repo_root: &Path,
     ) -> Result<Vec<FileObj>, ClientError> {
         let files = self
             .client
@@ -181,7 +181,11 @@ impl RestClient {
                 let file = file
                     .lock()
                     .map_err(|e| ClientError::MutexPoisoned(e.to_string()))?;
-                file.make_suggestions_from_patch(&mut review_comments, summary_only)?;
+                file.make_suggestions_from_patch(
+                    &mut review_comments,
+                    summary_only,
+                    &feedback_inputs.repo_root,
+                )?;
             }
 
             let mut options = ReviewOptions {
diff --git a/cpp-linter/src/run.rs b/cpp-linter/src/run.rs
@@ -101,6 +101,7 @@ pub async fn run_main(args: Vec<String>) -> Result<()> {
     }
 
     rest_api_client.start_log_group("Get list of specified source files");
+    let repo_root_path = PathBuf::from(&cli.source_options.repo_root);
     let files = if !matches!(cli.source_options.lines_changed_only, LinesChangedOnly::Off)
         || cli.source_options.files_changed_only
     {
@@ -111,7 +112,7 @@ pub async fn run_main(args: Vec<String>) -> Result<()> {
                 &cli.source_options.lines_changed_only.clone().into(),
                 &cli.source_options.diff_base,
                 cli.source_options.ignore_index,
-                &cli.source_options.repo_root,
+                &repo_root_path,
             )
             .await?
     } else {
@@ -123,7 +124,7 @@ pub async fn run_main(args: Vec<String>) -> Result<()> {
                 let file_path = PathBuf::from(&file_name);
                 FileObj::new(
                     file_path
-                        .strip_prefix(&cli.source_options.repo_root)
+                        .strip_prefix(&repo_root_path)
                         .map(PathBuf::from)
                         .unwrap_or(file_path),
                 )
@@ -136,7 +137,7 @@ pub async fn run_main(args: Vec<String>) -> Result<()> {
                     &LinesChangedOnly::Off.into(),
                     &cli.source_options.diff_base,
                     cli.source_options.ignore_index,
-                    &cli.source_options.repo_root,
+                    &repo_root_path,
                 )
                 .await?;
             for changed_file in changed_files {
diff --git a/cpp-linter/tests/comments.rs b/cpp-linter/tests/comments.rs
@@ -5,7 +5,7 @@ use cpp_linter::run::run_main;
 use git_bot_feedback::LinesChangedOnly;
 use mockito::Matcher;
 use std::{env, fmt::Display, io::Write, path::Path};
-use tempfile::NamedTempFile;
+use tempfile::{NamedTempFile, TempDir};
 
 mod common;
 use common::{create_test_space, mock_server};
@@ -62,7 +62,7 @@ impl Default for TestParams {
     }
 }
 
-async fn setup(lib_root: &Path, test_params: &TestParams) {
+async fn setup(lib_root: &Path, tmp_dir: &TempDir, test_params: &TestParams) {
     let mut event_payload_path = NamedTempFile::new_in("./").unwrap();
     let tmp_out_file = NamedTempFile::new().unwrap();
     unsafe {
@@ -253,6 +253,7 @@ async fn setup(lib_root: &Path, test_params: &TestParams) {
         format!("--no-lgtm={}", test_params.no_lgtm),
         "-p=build".to_string(),
         "-i=build".to_string(),
+        format!("--repo-root={}", tmp_dir.path().to_str().unwrap()),
     ];
     if test_params.force_lgtm {
         args.push("-e=c".to_string());
@@ -278,9 +279,7 @@ async fn setup(lib_root: &Path, test_params: &TestParams) {
 async fn test_comment(test_params: &TestParams) {
     let tmp_dir = create_test_space(true);
     let lib_root = env::current_dir().unwrap();
-    env::set_current_dir(tmp_dir.path()).unwrap();
-    setup(&lib_root, test_params).await;
-    env::set_current_dir(lib_root.as_path()).unwrap();
+    setup(&lib_root, &tmp_dir, test_params).await;
     drop(tmp_dir);
 }
 
diff --git a/cpp-linter/tests/common/mod.rs b/cpp-linter/tests/common/mod.rs
@@ -21,7 +21,7 @@ use tempfile::TempDir;
 /// The returned directory object will automatically delete the
 /// temporary folder when it is dropped out of scope.
 pub fn create_test_space(setup_meson: bool) -> TempDir {
-    let tmp = TempDir::new().unwrap();
+    let tmp = TempDir::with_prefix("cpp-linter-test").unwrap();
     fs::create_dir(tmp.path().join("src")).unwrap();
     let src = fs::read_dir("tests/demo").unwrap();
     for file in src {
@@ -48,11 +48,10 @@ pub fn create_test_space(setup_meson: bool) -> TempDir {
         "demo.cpp",
         "demo.hpp",
     ]);
-    let output = cmd.output().expect("Failed to run 'meson init'");
-    println!(
-        "meson init stdout:\n{}",
-        String::from_utf8(output.stdout.to_vec()).unwrap()
-    );
+    let status = cmd.status().expect("Failed to run 'meson init'");
+    if !status.success() {
+        panic!("Failed to initialize meson project with 'meson init'");
+    }
     let meson_build_dir = tmp.path().join("build");
     let mut cmd = Command::new("meson");
     cmd.args([
@@ -61,13 +60,12 @@ pub fn create_test_space(setup_meson: bool) -> TempDir {
         meson_build_dir.as_path().to_str().unwrap(),
         test_dir.to_str().unwrap(),
     ]);
-    let output = cmd
-        .output()
+    let status = cmd
+        .status()
         .expect("Failed to generate build assets with 'meson setup'");
-    println!(
-        "meson setup stdout:\n{}",
-        String::from_utf8(output.stdout.to_vec()).unwrap()
-    );
+    if !status.success() {
+        panic!("Failed to generate build assets with 'meson setup'");
+    }
     let db = fs::File::open(meson_build_dir.join("compile_commands.json"))
         .expect("Failed to open compilation database");
     for line in io::BufReader::new(db).lines().map_while(Result::ok) {
diff --git a/cpp-linter/tests/paginated_changed_files.rs b/cpp-linter/tests/paginated_changed_files.rs
@@ -7,7 +7,11 @@ use mockito::Matcher;
 use tempfile::{NamedTempFile, TempDir};
 
 use cpp_linter::{logger, rest_client::RestClient};
-use std::{env, io::Write, path::Path};
+use std::{
+    env,
+    io::Write,
+    path::{Path, PathBuf},
+};
 
 #[derive(PartialEq, Default)]
 enum EventType {
@@ -32,7 +36,7 @@ const RESET_RATE_LIMIT_HEADER: &str = "x-ratelimit-reset";
 const REMAINING_RATE_LIMIT_HEADER: &str = "x-ratelimit-remaining";
 const MALFORMED_RESPONSE_PAYLOAD: &str = "{\"message\":\"Resource not accessible by integration\"}";
 
-async fn get_paginated_changes(lib_root: &Path, test_params: &TestParams) {
+async fn get_paginated_changes(lib_root: &Path, _tmp_dir: &TempDir, test_params: &TestParams) {
     let tmp = TempDir::new().expect("Failed to create a temp dir for test");
     let mut event_payload = NamedTempFile::new_in(tmp.path())
         .expect("Failed to spawn a tmp file for test event payload");
@@ -145,7 +149,7 @@ async fn get_paginated_changes(lib_root: &Path, test_params: &TestParams) {
             &LinesChangedOnly::Off,
             &None::<String>,
             false,
-            ".",
+            &PathBuf::from("."),
         )
         .await;
     match files {
@@ -177,9 +181,7 @@ async fn get_paginated_changes(lib_root: &Path, test_params: &TestParams) {
 async fn test_get_changes(test_params: &TestParams) {
     let tmp_dir = create_test_space(false);
     let lib_root = env::current_dir().unwrap();
-    env::set_current_dir(tmp_dir.path()).unwrap();
-    get_paginated_changes(&lib_root, test_params).await;
-    env::set_current_dir(lib_root.as_path()).unwrap();
+    get_paginated_changes(&lib_root, &tmp_dir, test_params).await;
     drop(tmp_dir);
 }
 
diff --git a/cpp-linter/tests/reviews.rs b/cpp-linter/tests/reviews.rs
@@ -7,7 +7,7 @@ use cpp_linter::{
 use git_bot_feedback::LinesChangedOnly;
 use mockito::Matcher;
 use std::{env, io::Write, path::Path};
-use tempfile::NamedTempFile;
+use tempfile::{NamedTempFile, TempDir};
 
 mod common;
 use common::{create_test_space, mock_server};
@@ -70,7 +70,7 @@ fn generate_tool_summary(review_enabled: bool, force_lgtm: bool, tool_name: &str
     }
 }
 
-async fn setup(lib_root: &Path, test_params: &TestParams) {
+async fn setup(lib_root: &Path, tmp_dir: &TempDir, test_params: &TestParams) {
     let mut event_payload_path = NamedTempFile::new_in("./").unwrap();
     let event_payload = if test_params.bad_pr_info {
         "".to_string()
@@ -239,6 +239,7 @@ async fn setup(lib_root: &Path, test_params: &TestParams) {
         format!("--no-lgtm={}", test_params.no_lgtm),
         "-p=build".to_string(),
         "-i=build".to_string(),
+        format!("--repo-root={}", tmp_dir.path().to_str().unwrap()),
     ];
     if test_params.force_lgtm {
         if test_params.tidy_review {
@@ -272,9 +273,7 @@ async fn setup(lib_root: &Path, test_params: &TestParams) {
 async fn test_review(test_params: &TestParams) {
     let tmp_dir = create_test_space(true);
     let lib_root = env::current_dir().unwrap();
-    env::set_current_dir(tmp_dir.path()).unwrap();
-    setup(&lib_root, test_params).await;
-    env::set_current_dir(lib_root.as_path()).unwrap();
+    setup(&lib_root, &tmp_dir, test_params).await;
     drop(tmp_dir);
 }
 

Original file line number	Diff line number	Diff line change
`@@ -232,6 +232,7 @@ pub struct FeedbackInput {`
`232`	`232`	`pub tidy_review: bool,`
`233`	`233`	`pub format_review: bool,`
`234`	`234`	`pub passive_reviews: bool,`
	`235`	`+ pub repo_root: PathBuf,`
`235`	`236`	`}`
`236`	`237`
`237`	`238`	`#[cfg(feature = "bin")]`
`@@ -247,6 +248,7 @@ impl From<&Cli> for FeedbackInput {`
`247`	`248`	`tidy_review: args.feedback_options.tidy_review,`
`248`	`249`	`format_review: args.feedback_options.format_review,`
`249`	`250`	`passive_reviews: args.feedback_options.passive_reviews,`
	`251`	`+ repo_root: PathBuf::from(&args.source_options.repo_root),`
`250`	`252`	`}`
`251`	`253`	`}`
`252`	`254`	`}`
`@@ -263,6 +265,7 @@ impl Default for FeedbackInput {`
`263`	`265`	`tidy_review: false,`
`264`	`266`	`format_review: false,`
`265`	`267`	`passive_reviews: false,`
	`268`	`+ repo_root: PathBuf::from("."),`
`266`	`269`	`}`
`267`	`270`	`}`
`268`	`271`	`}`