Skip to content

Commit a226c68

Browse files
committed
augment process wrapping helper to any program; add unit tests
1 parent 992f04a commit a226c68

File tree

10 files changed

+131
-22
lines changed

10 files changed

+131
-22
lines changed

git_xet/src/app/install.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::path::PathBuf;
33
use crate::app::Command::Transfer;
44
use crate::constants::{GIT_LFS_CUSTOM_TRANSFER_AGENT_NAME, GIT_LFS_CUSTOM_TRANSFER_AGENT_PROGRAM};
55
use crate::errors::{GitXetError, Result};
6-
use crate::git_process_wrapping::run_git_captured;
6+
use crate::utils::process_wrapping::run_git_captured;
77

88
#[derive(Default)]
99
pub(crate) enum ConfigLocation {
@@ -102,9 +102,9 @@ pub mod tests {
102102
use serial_test::serial;
103103

104104
use super::{global, local};
105-
use crate::git_process_wrapping::run_git_captured_with_input_and_output;
106105
use crate::git_repo::GitRepo;
107106
use crate::test_utils::TestRepo;
107+
use crate::utils::process_wrapping::run_git_captured_with_input_and_output;
108108

109109
pub fn get_lfs_env(env_list: &[u8], key: &str) -> Result<Option<String>> {
110110
let reader = BufReader::new(Cursor::new(env_list));

git_xet/src/app/uninstall.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::path::PathBuf;
33
use super::install::ConfigLocation;
44
use crate::constants::GIT_LFS_CUSTOM_TRANSFER_AGENT_NAME;
55
use crate::errors::Result;
6-
use crate::git_process_wrapping::run_git_captured;
6+
use crate::utils::process_wrapping::run_git_captured;
77

88
// Remove git-xet registration from the system Git config.
99
pub fn system() -> Result<()> {
@@ -64,8 +64,8 @@ mod tests {
6464
use super::{all, local};
6565
use crate::app::install;
6666
use crate::app::install::tests::get_lfs_env;
67-
use crate::git_process_wrapping::run_git_captured_with_input_and_output;
6867
use crate::test_utils::TestRepo;
68+
use crate::utils::process_wrapping::run_git_captured_with_input_and_output;
6969

7070
#[test]
7171
#[serial(env_var_write_read)]

git_xet/src/auth.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,9 +186,9 @@ mod test_cred_helpers {
186186

187187
use super::get_credential;
188188
use crate::constants::HF_TOKEN_ENV;
189-
use crate::git_process_wrapping::run_git_captured_with_input_and_output;
190189
use crate::git_repo::GitRepo;
191190
use crate::test_utils::TestRepo;
191+
use crate::utils::process_wrapping::run_git_captured_with_input_and_output;
192192

193193
#[test]
194194
#[serial(env_var_write_read)]

git_xet/src/auth/git.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::sync::Arc;
55
use hub_client::BearerCredentialHelper;
66

77
use crate::errors::{GitXetError, Result};
8-
use crate::git_process_wrapping::run_git_captured_with_input_and_output;
8+
use crate::utils::process_wrapping::run_git_captured_with_input_and_output;
99

1010
// This implements the mechanism to get credential stored in configured git credential helpers, including
1111
// git-credential-cache, git-credential-store, git-credential-libsecret (Linux), git-credential-osxkeychain (macOS),
@@ -88,9 +88,9 @@ mod test_cred_helpers {
8888
use serial_test::serial;
8989

9090
use super::GitCredentialHelper;
91-
use crate::git_process_wrapping::run_git_captured_with_input_and_output;
9291
use crate::git_url::GitUrl;
9392
use crate::test_utils::TestRepo;
93+
use crate::utils::process_wrapping::run_git_captured_with_input_and_output;
9494

9595
#[test]
9696
#[serial(env_var_write_read)]

git_xet/src/errors.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ use crate::lfs_agent_protocol::GitLFSProtocolError;
99

1010
#[derive(Error, Debug)]
1111
pub enum GitXetError {
12-
#[error("Git command failed: {reason}, {source:?}")]
13-
GitCommandFailed {
12+
#[error("Command failed: {reason}, {source:?}")]
13+
CommandFailed {
1414
reason: String,
1515
source: Option<std::io::Error>,
1616
},
@@ -49,8 +49,8 @@ pub enum GitXetError {
4949
pub type Result<T> = std::result::Result<T, GitXetError>;
5050

5151
impl GitXetError {
52-
pub(crate) fn git_cmd_failed(e: impl Display, source: Option<std::io::Error>) -> GitXetError {
53-
GitXetError::GitCommandFailed {
52+
pub(crate) fn cmd_failed(e: impl Display, source: Option<std::io::Error>) -> GitXetError {
53+
GitXetError::CommandFailed {
5454
reason: e.to_string(),
5555
source,
5656
}

git_xet/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ pub mod app;
22
mod auth;
33
mod constants;
44
mod errors;
5-
mod git_process_wrapping;
65
mod git_repo;
76
mod git_url;
87
mod lfs_agent_protocol;
98
mod test_utils;
109
mod token_refresher;
10+
mod utils;

git_xet/src/test_utils/temp.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use anyhow::Result;
66
use tempfile::{TempDir, tempdir};
77
use utils::EnvVarGuard;
88

9-
use crate::git_process_wrapping::run_git_captured;
9+
use crate::utils::process_wrapping::run_git_captured;
1010

1111
// A test utility to create a temporary HOME environment, this sets up a clean environment for
1212
// git operations which depend heavily on a global config file.

git_xet/src/test_utils/test_repo.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ use std::path::{Path, PathBuf};
44
use anyhow::Result;
55
use tempfile::TempDir;
66

7-
use crate::git_process_wrapping::run_git_captured;
87
use crate::test_utils::TempHome;
8+
use crate::utils::process_wrapping::run_git_captured;
99

1010
// A test utility to create a repo at a temporary directory with a clean global config.
1111
// It allows performing a varies of git operations in this repo.

git_xet/src/utils/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
pub mod process_wrapping;

git_xet/src/git_process_wrapping.rs renamed to git_xet/src/utils/process_wrapping.rs

Lines changed: 116 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,15 @@ use std::process::{Child, ChildStdin, Command, Stdio};
55
use crate::constants::GIT_EXECUTABLE;
66
use crate::errors::{GitXetError, Result};
77

8-
// This mod implements utilities to invoke Git commands through child processes from the `git` program.
8+
// This mod implements utilities to invoke commands through child processes from any program, and
9+
// exposes special helpers to invoke git commands.
910

1011
/// Run a Git command as a child process by setting the current working directory to `working_dir`.
1112
/// This function doesn't allow the parent process to send data to the child.
1213
///
1314
/// Return `Ok(())` if the Git command finishes correctly and the child's stdout and stderr are ignored;
1415
/// Return the underlying I/O error if the child process spawning or waiting fails; otherwise, the captured
15-
/// stdout and stderr of the child are wrapped in an `Err(GitXetError::GitCommandFailed(_))` and returned.
16+
/// stdout and stderr of the child are wrapped in an `Err(GitXetError::CommandFailed(_))` and returned.
1617
pub fn run_git_captured<P, S1, I, S2>(working_dir: P, git_command: S1, args: I) -> Result<()>
1718
where
1819
P: AsRef<Path>,
@@ -21,7 +22,27 @@ where
2122
S2: AsRef<OsStr>,
2223
{
2324
let mut command = Command::new(GIT_EXECUTABLE);
24-
command.current_dir(working_dir).arg(git_command.as_ref()).args(args);
25+
command.current_dir(working_dir).arg(git_command).args(args);
26+
27+
CapturedCommand::new(command)?.wait()
28+
}
29+
30+
/// Run a command as a child process by setting the current working directory to `working_dir`.
31+
/// This function doesn't allow the parent process to send data to the child.
32+
///
33+
/// Return `Ok(())` if the command finishes correctly and the child's stdout and stderr are ignored;
34+
/// Return the underlying I/O error if the child process spawning or waiting fails; otherwise, the captured
35+
/// stdout and stderr of the child are wrapped in an `Err(GitXetError::CommandFailed(_))` and returned.
36+
#[allow(dead_code)]
37+
pub fn run_program_captured<S1, P, I, S2>(program: S1, working_dir: P, args: I) -> Result<()>
38+
where
39+
S1: AsRef<OsStr>,
40+
P: AsRef<Path>,
41+
I: IntoIterator<Item = S2>,
42+
S2: AsRef<OsStr>,
43+
{
44+
let mut command = Command::new(program);
45+
command.current_dir(working_dir).args(args);
2546

2647
CapturedCommand::new(command)?.wait()
2748
}
@@ -60,6 +81,41 @@ where
6081
CapturedCommand::new_with_piped_stdin(command)
6182
}
6283

84+
/// Run a command as a child process by setting the current working directory to `working_dir`.
85+
/// This function allows the parent process to send data to the child through the piped stdin.
86+
///
87+
/// Return `Ok(CapturedCommand)` if the command process spawns correctly; otherwise, the underlying I/O
88+
/// error is returned.
89+
///
90+
/// # Examples
91+
/// ```ignore
92+
/// let mut cmd = run_program_captured_with_input_and_output("tee", path, &["dump.txt"])?;
93+
///
94+
/// {
95+
/// let mut writer = cmd.stdin()?;
96+
/// write!(writer, "some_data")?;
97+
/// }
98+
///
99+
/// let (response, _err) = cmd.wait_with_output()?;
100+
/// ```
101+
#[allow(dead_code)]
102+
pub fn run_program_captured_with_input_and_output<S1, P, I, S2>(
103+
program: S1,
104+
working_dir: P,
105+
args: I,
106+
) -> Result<CapturedCommand>
107+
where
108+
S1: AsRef<OsStr>,
109+
P: AsRef<Path>,
110+
I: IntoIterator<Item = S2>,
111+
S2: AsRef<OsStr>,
112+
{
113+
let mut command = Command::new(program);
114+
command.current_dir(working_dir).args(args);
115+
116+
CapturedCommand::new_with_piped_stdin(command)
117+
}
118+
63119
// This struct wraps inside a spawned child process, whose stdout and stderr is piped
64120
// to the parent process instead of pointing at the terminal.
65121
pub struct CapturedCommand {
@@ -74,8 +130,11 @@ impl CapturedCommand {
74130
// From past experience, if the "git" program is not found the underlying error
75131
// only says "Not Found" and is not very helpful to identify the cause. We thus
76132
// capture this error and make the message more explicit.
77-
std::io::ErrorKind::NotFound => GitXetError::git_cmd_failed(r#"program "git" not found"#, Some(e)),
78-
_ => GitXetError::git_cmd_failed("internal", Some(e)),
133+
std::io::ErrorKind::NotFound => GitXetError::cmd_failed(
134+
format!(r#"program "{}" not found"#, command.get_program().display()),
135+
Some(e),
136+
),
137+
_ => GitXetError::cmd_failed("internal", Some(e)),
79138
})?,
80139
})
81140
}
@@ -98,7 +157,7 @@ impl CapturedCommand {
98157
}
99158

100159
/// Synchronously wait for the child to exit completely, returning `Ok(())` if the child exits with status code 0;
101-
/// otherwise, return the captured output wrapped in an `Err(GitXetError::GitCommandFailed(_))`.
160+
/// otherwise, return the captured output wrapped in an `Err(GitXetError::CommandFailed(_))`.
102161
pub fn wait(self) -> Result<()> {
103162
// ignores output
104163
let _ = self.wait_with_output()?;
@@ -108,7 +167,7 @@ impl CapturedCommand {
108167

109168
/// Synchronously wait for the child to exit and collect all remaining output on the stdout/stderr handles,
110169
/// returning a tuple of captured output if the child exits with status code 0; otherwise, return the captured
111-
/// output wrapped in an `Err(GitXetError::GitCommandFailed(_))`.
170+
/// output wrapped in an `Err(GitXetError::CommandFailed(_))`.
112171
pub fn wait_with_output(self) -> Result<(Vec<u8>, Vec<u8>)> {
113172
let ret = self.child_process.wait_with_output()?;
114173

@@ -117,11 +176,60 @@ impl CapturedCommand {
117176
_ => {
118177
let stdout = std::str::from_utf8(&ret.stdout).unwrap_or("<Binary Data>").trim();
119178
let stderr = std::str::from_utf8(&ret.stderr).unwrap_or("<Binary Data>").trim();
120-
Err(GitXetError::git_cmd_failed(
179+
Err(GitXetError::cmd_failed(
121180
format!("err_code = {:?}, stdout = \"{}\", stderr = \"{}\"", ret.status.code(), stdout, stderr),
122181
None,
123182
))
124183
},
125184
}
126185
}
127186
}
187+
188+
#[cfg(test)]
189+
mod tests {
190+
use std::io::Write;
191+
192+
use anyhow::Result;
193+
194+
use super::*;
195+
196+
#[test]
197+
fn test_run_program_captured() -> Result<()> {
198+
#[cfg(unix)]
199+
run_program_captured("sh", std::env::current_dir()?, &["-c", "echo hello"])?;
200+
#[cfg(windows)]
201+
run_program_captured("cmd", std::env::current_dir()?, &["/C", "echo hello"])?;
202+
203+
Ok(())
204+
}
205+
206+
#[test]
207+
fn test_program_captured_with_input_and_output() -> Result<()> {
208+
let mut cmd = if cfg!(windows) {
209+
run_program_captured_with_input_and_output("cmd", std::env::current_dir()?, &["/C", "more"])?
210+
} else {
211+
run_program_captured_with_input_and_output("sh", std::env::current_dir()?, &["-c", "cat"])?
212+
};
213+
214+
{
215+
let mut writer = cmd.stdin()?;
216+
write!(writer, "hello")?;
217+
}
218+
219+
let (response, _err) = cmd.wait_with_output()?;
220+
assert_eq!(response, "hello".as_bytes());
221+
222+
Ok(())
223+
}
224+
225+
#[test]
226+
fn test_error_on_get_stdin_without_captured() -> Result<()> {
227+
let mut command = Command::new("more");
228+
command.current_dir(std::env::current_dir()?);
229+
230+
let mut command = CapturedCommand::new(command)?;
231+
assert!(command.stdin().is_err());
232+
233+
Ok(())
234+
}
235+
}

0 commit comments

Comments
 (0)