Skip to content

Commit b81c552

Browse files
committed
Refactor command execution in bundler and gemset modules
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 b81c552

File tree

4 files changed

+633
-57
lines changed

4 files changed

+633
-57
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)