Skip to content

Commit f7da1e6

Browse files
authored
Refactor command execution in bundler and gemset modules (#87)
This refactoring improves the architecture and testability of the Ruby extension by introducing abstractions for command execution. Key changes: - Add `CommandExecutor` trait for bundle commands with default impl - Add `GemCommandExecutor` trait for gem commands with default impl - Refactor Bundler and Gemset to accept executors via DI The refactoring enables better testing through mock implementations while keeping the original functionality intact.
1 parent b319276 commit f7da1e6

File tree

4 files changed

+641
-61
lines changed

4 files changed

+641
-61
lines changed

src/bundler.rs

Lines changed: 231 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,67 @@
11
use std::path::Path;
2-
use zed_extension_api::{Command, Result};
2+
use zed_extension_api::{process::Output, Command, Result};
3+
4+
pub trait CommandExecutor {
5+
fn execute_bundle(
6+
&self,
7+
sub_command: String,
8+
args: Vec<String>,
9+
envs: Vec<(String, String)>,
10+
bundle_gemfile_path: &str,
11+
) -> Result<Output>;
12+
}
13+
14+
pub struct RealCommandExecutor;
15+
16+
impl CommandExecutor for RealCommandExecutor {
17+
fn execute_bundle(
18+
&self,
19+
sub_command: String,
20+
args: Vec<String>,
21+
envs: Vec<(String, String)>,
22+
bundle_gemfile_path: &str,
23+
) -> Result<Output> {
24+
Command::new("bundle")
25+
.arg(sub_command)
26+
.args(args)
27+
.envs(envs)
28+
.env("BUNDLE_GEMFILE", bundle_gemfile_path)
29+
.output()
30+
}
31+
}
332

433
/// A simple wrapper around the `bundle` command.
534
pub struct Bundler {
635
pub working_dir: String,
736
envs: Vec<(String, String)>,
37+
command_executor: Box<dyn CommandExecutor>,
838
}
939

1040
impl Bundler {
11-
pub fn new(working_dir: String, envs: Vec<(String, String)>) -> Self {
12-
Bundler { working_dir, envs }
41+
/// Creates a new `Bundler` instance.
42+
///
43+
/// # Arguments
44+
/// * `working_dir` - The working directory where `bundle` commands should be executed.
45+
/// * `command_executor` - An executor for `bundle` commands.
46+
pub fn new(
47+
working_dir: String,
48+
envs: Vec<(String, String)>,
49+
command_executor: Box<dyn CommandExecutor>,
50+
) -> Self {
51+
Bundler {
52+
working_dir,
53+
envs,
54+
command_executor,
55+
}
1356
}
1457

58+
/// Retrieves the installed version of a gem using `bundle info --version <name>`.
59+
///
60+
/// # Arguments
61+
/// * `name` - The name of the gem.
62+
///
63+
/// # Returns
64+
/// A `Result` containing the version string if successful, or an error message.
1565
pub fn installed_gem_version(&self, name: &str) -> Result<String> {
1666
let args = vec!["--version".into(), name.into()];
1767

@@ -22,14 +72,10 @@ impl Bundler {
2272
let bundle_gemfile_path = Path::new(&self.working_dir).join("Gemfile");
2373
let bundle_gemfile = bundle_gemfile_path
2474
.to_str()
25-
.ok_or("Invalid path to Gemfile")?;
75+
.ok_or_else(|| "Invalid path to Gemfile".to_string())?;
2676

27-
Command::new("bundle")
28-
.arg(cmd)
29-
.args(args)
30-
.envs(self.envs.clone())
31-
.env("BUNDLE_GEMFILE", bundle_gemfile)
32-
.output()
77+
self.command_executor
78+
.execute_bundle(cmd, args, self.envs.clone(), bundle_gemfile)
3379
.and_then(|output| match output.status {
3480
Some(0) => Ok(String::from_utf8_lossy(&output.stdout).to_string()),
3581
Some(status) => {
@@ -46,3 +92,178 @@ impl Bundler {
4692
})
4793
}
4894
}
95+
96+
#[cfg(test)]
97+
mod tests {
98+
use super::*;
99+
use std::cell::RefCell;
100+
101+
struct MockExecutorConfig {
102+
output_to_return: Option<Result<Output>>,
103+
expected_sub_command: Option<String>,
104+
expected_args: Option<Vec<String>>,
105+
expected_envs: Option<Vec<(String, String)>>,
106+
expected_bundle_gemfile_path: Option<String>,
107+
}
108+
109+
struct MockCommandExecutor {
110+
config: RefCell<MockExecutorConfig>,
111+
}
112+
113+
impl MockCommandExecutor {
114+
fn new() -> Self {
115+
MockCommandExecutor {
116+
config: RefCell::new(MockExecutorConfig {
117+
output_to_return: None,
118+
expected_sub_command: None,
119+
expected_args: None,
120+
expected_envs: None,
121+
expected_bundle_gemfile_path: None,
122+
}),
123+
}
124+
}
125+
126+
fn expect(
127+
&self,
128+
sub_command: &str,
129+
args: &[&str],
130+
envs: &[(&str, &str)],
131+
bundle_gemfile_path: &str,
132+
output: super::Result<Output>,
133+
) {
134+
let mut config = self.config.borrow_mut();
135+
config.expected_sub_command = Some(sub_command.to_string());
136+
config.expected_args = Some(args.iter().map(|s| s.to_string()).collect());
137+
config.expected_envs = Some(
138+
envs.iter()
139+
.map(|&(k, v)| (k.to_string(), v.to_string()))
140+
.collect(),
141+
);
142+
config.expected_bundle_gemfile_path = Some(bundle_gemfile_path.to_string());
143+
config.output_to_return = Some(output);
144+
}
145+
}
146+
147+
impl CommandExecutor for MockCommandExecutor {
148+
fn execute_bundle(
149+
&self,
150+
sub_command: String,
151+
args: Vec<String>,
152+
envs: Vec<(String, String)>,
153+
bundle_gemfile_path: &str,
154+
) -> super::Result<Output> {
155+
let mut config = self.config.borrow_mut();
156+
157+
if let Some(expected_cmd) = &config.expected_sub_command {
158+
assert_eq!(&sub_command, expected_cmd, "Mock: Sub-command mismatch");
159+
}
160+
if let Some(expected_args) = &config.expected_args {
161+
assert_eq!(&args, expected_args, "Mock: Args mismatch");
162+
}
163+
if let Some(expected_envs) = &config.expected_envs {
164+
assert_eq!(&envs, expected_envs, "Mock: Env mismatch");
165+
}
166+
if let Some(expected_path) = &config.expected_bundle_gemfile_path {
167+
assert_eq!(
168+
bundle_gemfile_path, expected_path,
169+
"Mock: Gemfile path mismatch"
170+
);
171+
}
172+
173+
config.output_to_return.take().expect(
174+
"MockCommandExecutor: output_to_return was not set or already consumed for the test",
175+
)
176+
}
177+
}
178+
179+
fn create_mock_executor_for_success(
180+
version: &str,
181+
dir: &str,
182+
gem: &str,
183+
) -> MockCommandExecutor {
184+
let mock = MockCommandExecutor::new();
185+
mock.expect(
186+
"info",
187+
&["--version", gem],
188+
&[],
189+
&format!("{}/Gemfile", dir),
190+
Ok(Output {
191+
status: Some(0),
192+
stdout: version.as_bytes().to_vec(),
193+
stderr: Vec::new(),
194+
}),
195+
);
196+
mock
197+
}
198+
199+
#[test]
200+
fn test_installed_gem_version_success() {
201+
let mock_executor = create_mock_executor_for_success("8.0.0", "test_dir", "rails");
202+
let bundler = Bundler::new("test_dir".into(), vec![], Box::new(mock_executor));
203+
let version = bundler
204+
.installed_gem_version("rails")
205+
.expect("Expected successful version");
206+
assert_eq!(version, "8.0.0", "Installed gem version should match");
207+
}
208+
209+
#[test]
210+
fn test_installed_gem_version_command_error() {
211+
let mock_executor = MockCommandExecutor::new();
212+
let gem_name = "unknown_gem";
213+
let error_output = "Could not find gem 'unknown_gem'.";
214+
215+
mock_executor.expect(
216+
"info",
217+
&["--version", gem_name],
218+
&[],
219+
"test_dir/Gemfile",
220+
Ok(Output {
221+
status: Some(1),
222+
stdout: Vec::new(),
223+
stderr: error_output.as_bytes().to_vec(),
224+
}),
225+
);
226+
227+
let bundler = Bundler::new("test_dir".into(), vec![], Box::new(mock_executor));
228+
let result = bundler.installed_gem_version(gem_name);
229+
230+
assert!(
231+
result.is_err(),
232+
"Expected error for failed gem version check"
233+
);
234+
let err_msg = result.unwrap_err();
235+
assert!(
236+
err_msg.contains("'bundle' command failed (status: 1)"),
237+
"Error message should contain status"
238+
);
239+
assert!(
240+
err_msg.contains(error_output),
241+
"Error message should contain stderr output"
242+
);
243+
}
244+
245+
#[test]
246+
fn test_installed_gem_version_execution_failure_from_executor() {
247+
let mock_executor = MockCommandExecutor::new();
248+
let gem_name = "critical_gem";
249+
let specific_error_msg = "Mocked execution failure";
250+
251+
mock_executor.expect(
252+
"info",
253+
&["--version", gem_name],
254+
&[],
255+
"test_dir/Gemfile",
256+
Err(specific_error_msg.to_string()),
257+
);
258+
259+
let bundler = Bundler::new("test_dir".into(), vec![], Box::new(mock_executor));
260+
let result = bundler.installed_gem_version(gem_name);
261+
262+
assert!(result.is_err(), "Expected error from executor failure");
263+
assert_eq!(
264+
result.unwrap_err(),
265+
specific_error_msg,
266+
"Error message should match executor error"
267+
);
268+
}
269+
}

0 commit comments

Comments
 (0)