From 7dbaaf7cdeb1d3bfc46801eede8d83b57017da25 Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Sun, 23 Nov 2025 16:17:06 +0000 Subject: [PATCH 1/4] Changing coverage tests to use sudo in instances where its required to test --- util/build-run-test-coverage-linux.sh | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/util/build-run-test-coverage-linux.sh b/util/build-run-test-coverage-linux.sh index 5a5b5af2ad0..98a24c5f99e 100755 --- a/util/build-run-test-coverage-linux.sh +++ b/util/build-run-test-coverage-linux.sh @@ -65,8 +65,18 @@ set +x run_test_and_aggregate() { echo "# Running coverage tests for ${1}" + # Determine if sudo is needed for this utility + local SUDO_CMD="" + case "${1}" in + chroot|chown|cp|dd|install|mknod) + if command -v sudo >/dev/null 2>&1 && sudo -n true 2>/dev/null; then + SUDO_CMD="sudo -E" + fi + ;; + esac + # Build and run tests for the UTIL - cargo nextest run \ + ${SUDO_CMD} cargo nextest run \ --profile coverage \ --no-fail-fast \ --color=always \ From 8e671e7ea80dbe2a0cb7d0ce0b60b0783b995cb8 Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Sun, 23 Nov 2025 16:26:52 +0000 Subject: [PATCH 2/4] Keeping the path in the home directory --- util/build-run-test-coverage-linux.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/build-run-test-coverage-linux.sh b/util/build-run-test-coverage-linux.sh index 98a24c5f99e..e38f1cd0d79 100755 --- a/util/build-run-test-coverage-linux.sh +++ b/util/build-run-test-coverage-linux.sh @@ -70,7 +70,7 @@ run_test_and_aggregate() { case "${1}" in chroot|chown|cp|dd|install|mknod) if command -v sudo >/dev/null 2>&1 && sudo -n true 2>/dev/null; then - SUDO_CMD="sudo -E" + SUDO_CMD="sudo -E env PATH=${PATH}" fi ;; esac From 667b293515da494ec2a6b4875b6b4847f5d768f9 Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Sun, 23 Nov 2025 17:07:05 +0000 Subject: [PATCH 3/4] Testing being able to run sudo on the CI fleet for getting coverage data --- tests/uutests/src/lib/util.rs | 64 +++++++++++++-------------- util/build-run-test-coverage-linux.sh | 12 +---- 2 files changed, 31 insertions(+), 45 deletions(-) diff --git a/tests/uutests/src/lib/util.rs b/tests/uutests/src/lib/util.rs index 4668e7ba8b3..4ede04c6eaa 100644 --- a/tests/uutests/src/lib/util.rs +++ b/tests/uutests/src/lib/util.rs @@ -3150,43 +3150,39 @@ pub fn run_ucmd_as_root_with_stdin_stdout( stdin: Option<&str>, stdout: Option<&str>, ) -> std::result::Result { - if is_ci() { - Err(format!("{UUTILS_INFO}: {}", "cannot run inside CI")) - } else { - // check if we can run 'sudo' - log_info("run", "sudo -E --non-interactive whoami"); - match Command::new("sudo") - .envs(DEFAULT_ENV) - .args(["-E", "--non-interactive", "whoami"]) - .output() - { - Ok(output) if String::from_utf8_lossy(&output.stdout).eq("root\n") => { - // we can run sudo and we're root - // run ucmd as root: - let mut cmd = ts.cmd("sudo"); - cmd.env("PATH", PATH) - .envs(DEFAULT_ENV) - .arg("-E") - .arg("--non-interactive") - .arg(&ts.bin_path) - .arg(&ts.util_name) - .args(args); - if let Some(stdin) = stdin { - cmd.set_stdin(File::open(stdin).unwrap()); - } - if let Some(stdout) = stdout { - cmd.set_stdout(File::open(stdout).unwrap()); - } - Ok(cmd.run()) + // check if we can run 'sudo' + log_info("run", "sudo -E --non-interactive whoami"); + match Command::new("sudo") + .envs(DEFAULT_ENV) + .args(["-E", "--non-interactive", "whoami"]) + .output() + { + Ok(output) if String::from_utf8_lossy(&output.stdout).eq("root\n") => { + // we can run sudo and we're root + // run ucmd as root: + let mut cmd = ts.cmd("sudo"); + cmd.env("PATH", PATH) + .envs(DEFAULT_ENV) + .arg("-E") + .arg("--non-interactive") + .arg(&ts.bin_path) + .arg(&ts.util_name) + .args(args); + if let Some(stdin) = stdin { + cmd.set_stdin(File::open(stdin).unwrap()); } - Ok(output) - if String::from_utf8_lossy(&output.stderr).eq("sudo: a password is required\n") => - { - Err("Cannot run non-interactive sudo".to_string()) + if let Some(stdout) = stdout { + cmd.set_stdout(File::open(stdout).unwrap()); } - Ok(_output) => Err("\"sudo whoami\" didn't return \"root\"".to_string()), - Err(e) => Err(format!("{UUTILS_WARNING}: {e}")), + Ok(cmd.run()) + } + Ok(output) + if String::from_utf8_lossy(&output.stderr).eq("sudo: a password is required\n") => + { + Err("Cannot run non-interactive sudo".to_string()) } + Ok(_output) => Err("\"sudo whoami\" didn't return \"root\"".to_string()), + Err(e) => Err(format!("{UUTILS_WARNING}: {e}")), } } diff --git a/util/build-run-test-coverage-linux.sh b/util/build-run-test-coverage-linux.sh index e38f1cd0d79..5a5b5af2ad0 100755 --- a/util/build-run-test-coverage-linux.sh +++ b/util/build-run-test-coverage-linux.sh @@ -65,18 +65,8 @@ set +x run_test_and_aggregate() { echo "# Running coverage tests for ${1}" - # Determine if sudo is needed for this utility - local SUDO_CMD="" - case "${1}" in - chroot|chown|cp|dd|install|mknod) - if command -v sudo >/dev/null 2>&1 && sudo -n true 2>/dev/null; then - SUDO_CMD="sudo -E env PATH=${PATH}" - fi - ;; - esac - # Build and run tests for the UTIL - ${SUDO_CMD} cargo nextest run \ + cargo nextest run \ --profile coverage \ --no-fail-fast \ --color=always \ From 64fb8ee9e218a6187176912d49d5c2e443086692 Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Wed, 26 Nov 2025 15:48:14 +0000 Subject: [PATCH 4/4] Creating temporary root command to run root tests in the CI without migrating all commands --- tests/by-util/test_chroot.rs | 25 +++++++++++++------------ tests/by-util/test_install.rs | 10 +++++++--- tests/uutests/src/lib/util.rs | 15 +++++++++++++++ 3 files changed, 35 insertions(+), 15 deletions(-) diff --git a/tests/by-util/test_chroot.rs b/tests/by-util/test_chroot.rs index 38c3727b1cd..e47531aaf9d 100644 --- a/tests/by-util/test_chroot.rs +++ b/tests/by-util/test_chroot.rs @@ -8,7 +8,7 @@ use uutests::at_and_ucmd; use uutests::new_ucmd; #[cfg(not(target_os = "android"))] use uutests::util::is_ci; -use uutests::util::{TestScenario, run_ucmd_as_root}; +use uutests::util::{TestScenario, run_ucmd_as_root, run_ucmd_as_root_to_migrate}; use uutests::util_name; #[test] @@ -57,12 +57,13 @@ fn test_no_such_directory() { } #[test] +#[cfg(not(target_os = "macos"))] fn test_multiple_group_args() { let ts = TestScenario::new(util_name!()); let at = &ts.fixtures; at.mkdir("id"); - if let Ok(result) = run_ucmd_as_root( + if let Ok(result) = run_ucmd_as_root_to_migrate( &ts, &["--groups='invalid ignored'", "--groups=''", "/", "id", "-G"], ) { @@ -80,7 +81,7 @@ fn test_invalid_user_spec() { result .failure() .code_is(125) - .stderr_is("chroot: invalid user"); + .stderr_is("chroot: invalid user\n"); } else { print!("Test skipped; requires root user"); } @@ -89,7 +90,7 @@ fn test_invalid_user_spec() { result .failure() .code_is(125) - .stderr_is("chroot: invalid user"); + .stderr_is("chroot: invalid user\n"); } else { print!("Test skipped; requires root user"); } @@ -98,7 +99,7 @@ fn test_invalid_user_spec() { result .failure() .code_is(125) - .stderr_is("chroot: invalid group"); + .stderr_is("chroot: invalid group\n"); } else { print!("Test skipped; requires root user"); } @@ -111,14 +112,14 @@ fn test_invalid_user() { let dir = "CHROOT_DIR"; at.mkdir(dir); - if let Ok(result) = run_ucmd_as_root(&ts, &[dir, "whoami"]) { + if let Ok(result) = run_ucmd_as_root_to_migrate(&ts, &[dir, "whoami"]) { result.success().no_stderr().stdout_is("root"); } else { print!("Test skipped; requires root user"); } // `--user` is an abbreviation of `--userspec`. - if let Ok(result) = run_ucmd_as_root(&ts, &["--user=nobody:+65535", dir, "pwd"]) { + if let Ok(result) = run_ucmd_as_root_to_migrate(&ts, &["--user=nobody:+65535", dir, "pwd"]) { result.failure().stderr_is("chroot: invalid user"); } else { print!("Test skipped; requires root user"); @@ -181,7 +182,7 @@ fn test_default_shell() { let shell = std::env::var("SHELL").unwrap_or_else(|_| "/bin/sh".to_string()); let expected = format!("chroot: failed to run command '{shell}': No such file or directory"); - if let Ok(result) = run_ucmd_as_root(&ts, &[dir]) { + if let Ok(result) = run_ucmd_as_root_to_migrate(&ts, &[dir]) { result.stderr_contains(expected); } else { print!("Test skipped; requires root user"); @@ -195,13 +196,13 @@ fn test_chroot() { let dir = "CHROOT_DIR"; at.mkdir(dir); - if let Ok(result) = run_ucmd_as_root(&ts, &[dir, "whoami"]) { + if let Ok(result) = run_ucmd_as_root_to_migrate(&ts, &[dir, "whoami"]) { result.success().no_stderr().stdout_is("root"); } else { print!("Test skipped; requires root user"); } - if let Ok(result) = run_ucmd_as_root(&ts, &[dir, "pwd"]) { + if let Ok(result) = run_ucmd_as_root_to_migrate(&ts, &[dir, "pwd"]) { result.success().no_stderr().stdout_is("/"); } else { print!("Test skipped; requires root user"); @@ -229,7 +230,7 @@ fn test_chroot_skip_chdir() { at.symlink_file("/", "isroot"); for dir in dirs { let env_cd = std::env::current_dir().unwrap(); - if let Ok(result) = run_ucmd_as_root(&ts, &[dir, "--skip-chdir"]) { + if let Ok(result) = run_ucmd_as_root_to_migrate(&ts, &[dir, "--skip-chdir"]) { // Should return the same path assert_eq!( result.success().no_stderr().stdout_str(), @@ -250,7 +251,7 @@ fn test_chroot_extra_arg() { at.mkdir(dir); let env_cd = std::env::current_dir().unwrap(); // Verify that -P is pwd's and not chroot - if let Ok(result) = run_ucmd_as_root(&ts, &[dir, "pwd", "-P"]) { + if let Ok(result) = run_ucmd_as_root_to_migrate(&ts, &[dir, "pwd", "-P"]) { assert_eq!( result.success().no_stderr().stdout_str(), env_cd.to_str().unwrap() diff --git a/tests/by-util/test_install.rs b/tests/by-util/test_install.rs index 2a2e7d67039..fe5d3dead87 100644 --- a/tests/by-util/test_install.rs +++ b/tests/by-util/test_install.rs @@ -6,10 +6,9 @@ #[cfg(not(target_os = "openbsd"))] use filetime::FileTime; -use std::fs; #[cfg(target_os = "linux")] use std::os::unix::ffi::OsStringExt; -use std::os::unix::fs::{MetadataExt, PermissionsExt}; +use std::os::unix::fs::PermissionsExt; #[cfg(not(windows))] use std::process::Command; #[cfg(any(target_os = "linux", target_os = "android"))] @@ -19,7 +18,7 @@ use uucore::process::{getegid, geteuid}; use uucore::selinux::get_getfattr_output; use uutests::at_and_ucmd; use uutests::new_ucmd; -use uutests::util::{TestScenario, is_ci, run_ucmd_as_root}; +use uutests::util::{TestScenario, is_ci}; use uutests::util_name; #[test] @@ -2017,7 +2016,12 @@ fn test_target_file_ends_with_slash() { } #[test] +#[cfg(not(target_os = "macos"))] fn test_install_root_combined() { + use std::fs; + use std::os::unix::fs::MetadataExt; + use uutests::util::run_ucmd_as_root; + let ts = TestScenario::new(util_name!()); let at = &ts.fixtures; at.touch("a"); diff --git a/tests/uutests/src/lib/util.rs b/tests/uutests/src/lib/util.rs index 4ede04c6eaa..e25460854ee 100644 --- a/tests/uutests/src/lib/util.rs +++ b/tests/uutests/src/lib/util.rs @@ -3143,6 +3143,21 @@ pub fn run_ucmd_as_root( run_ucmd_as_root_with_stdin_stdout(ts, args, None, None) } +// Using this function as a temporary measure to keep the tests that are not passing +// either locally or on the build fleet to be able to allow all of the tests that require root +// to pass. This will be removed one all of the tests that use this are fixed. +#[cfg(unix)] +pub fn run_ucmd_as_root_to_migrate( + ts: &TestScenario, + args: &[&str], +) -> std::result::Result { + if is_ci() { + Err(format!("{UUTILS_INFO}: {}", "cannot run inside CI")) + } else { + run_ucmd_as_root_with_stdin_stdout(ts, args, None, None) + } +} + #[cfg(unix)] pub fn run_ucmd_as_root_with_stdin_stdout( ts: &TestScenario,