Skip to content

Commit 1f327f7

Browse files
committed
Add tests for toolchain functionality and enhance Python tool setup with UV integration
- Introduced `toolchain_tests.rs` for validating Python tool setup, installation, and directory structure. - Enhanced Python tool package installation by integrating `uv` for performance; implemented fallback to `pip`. - Improved error handling, logging, and installation path validation in Python tool setup. - Updated hook execution to parse and manage commands with arguments more robustly. - Extended documentation and planning with new tasks and options for Python environment setup.
1 parent fac6c84 commit 1f327f7

File tree

4 files changed

+283
-12
lines changed

4 files changed

+283
-12
lines changed

docs/plan.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,10 @@ Tasks:
1313
- [x] We need a fundamental change in the way we run hooks. The configuration file should be used to create the context for each task we want to run and the context should contain the entry we are needing to run as a process. We need to be able to specify hooks that are to be ran from the built-in hooks versus hooks that are external. We could potentially run them in the same process or in a separate process. If we choose to run them in a separate process then we need to ensure that there is a cli command that the entry can point to. This will allow us to run the hooks in parallel and also allow us to run hooks in a different process than the main application. Should help debugging specific hooks.
1414
- [x] Make sure that the file in `docs/.pre-commit-config.yaml` can be used and converted to the rustyhook configuration file format. Importantly it should set up its own python environment and install the required packages without needing the system to have anything installed.
1515
- [x] Instead of having a separate binary called "rustyhooks-hooks" I think we should continue to use our primary "rustyhook" binary and have a sub-command that can be used to run the hooks. This will allow us to run the hooks in parallel and also allow us to run the hooks in a different process than the main application. Should help debugging specific hooks.
16-
- [ ] replace the use of pip with uv - potentially link to the source code so as to get the benefit without needing a local binary. This should help speed up the installation of a python interpreter as well as improve the performance ot package installation. We will likely be able to remove a substantial amount of the code we have in our source code for installing python interpreters and installing packages.
16+
- [ ] Use either https://gregoryszorc.com/docs/python-build-standalone/main/ or https://pyoxidizer.readthedocs.io/en/latest/pyoxy.html to download the specified python version and install it locally.
17+
- [ ] Have uv use the downloaded python interpreter that is specified in the .python-version file. use uv for package installation. We will likely be able to remove a substantial amount of the code we have in our source code for installing python interpreters and installing packages.
1718
- [ ] emulate all known pre-commit hooks for use with native execution - all known hooks at https://github.com/pre-commit/pre-commit-hooks
19+
- [ ] Break up the implementations of the hooks into separate files and create a test suite for each file.
1820
- [ ] create tests for each hook and complete coverage
1921
- [ ] replace the npm and nvm install process with fnm if it makes sense to do so
2022
- [ ] expand all built in hooks to include all known hooks from https://pre-commit.com/hooks.html

src/runner/hook_context.rs

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,10 +148,27 @@ impl HookContext {
148148
pub fn run_in_separate_process(&self) -> Result<(), HookContextError> {
149149
println!("Running hook {} in separate process", self.id);
150150

151+
// Parse the entry to separate the command from any arguments
152+
let parts: Vec<&str> = self.entry.split_whitespace().collect();
153+
if parts.is_empty() {
154+
return Err(HookContextError::ProcessError(format!(
155+
"Empty entry for hook {}", self.id
156+
)));
157+
}
158+
159+
// The first part is the command, the rest are arguments
160+
let command_name = parts[0];
161+
let command_args = &parts[1..];
162+
151163
// Create a command to run the hook
152-
let mut command = Command::new(&self.entry);
164+
let mut command = Command::new(command_name);
165+
166+
// Add any arguments from the entry
167+
for arg in command_args {
168+
command.arg(arg);
169+
}
153170

154-
// Add arguments
171+
// Add arguments from the hook configuration
155172
for arg in &self.args {
156173
command.arg(arg);
157174
}

src/toolchains/python.rs

Lines changed: 154 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -302,27 +302,158 @@ impl PythonTool {
302302
Ok(())
303303
}
304304

305-
/// Install packages in the virtualenv
305+
/// Install packages in the virtualenv using pip (traditional approach)
306306
fn install_packages(&self, ctx: &SetupContext) -> Result<(), ToolError> {
307+
// Find the python executable in the virtualenv
308+
let python = if cfg!(windows) {
309+
ctx.install_dir.join("Scripts").join("python.exe")
310+
} else {
311+
ctx.install_dir.join("bin").join("python")
312+
};
313+
307314
// Find the pip executable in the virtualenv
308315
let pip = if cfg!(windows) {
309316
ctx.install_dir.join("Scripts").join("pip.exe")
310317
} else {
311318
ctx.install_dir.join("bin").join("pip")
312319
};
313320

314-
// Install each package
315-
for package in &self.packages {
321+
// Check if the python executable exists in the virtualenv
322+
if !python.exists() {
323+
return Err(ToolError::ExecutionError(
324+
format!("Python executable not found at {:?}", python),
325+
));
326+
}
327+
328+
log::debug!("Python executable found at {:?}", python);
329+
330+
// Install all packages at once for better performance
331+
if !self.packages.is_empty() {
332+
// First, try to install uv directly using pip
316333
let status = Command::new(&pip)
317334
.arg("install")
318-
.arg(package)
335+
.arg("uv")
319336
.status()
320-
.map_err(|e| ToolError::ExecutionError(format!("Failed to install {}: {}", package, e)))?;
337+
.map_err(|e| ToolError::ExecutionError(format!("Failed to install uv: {}", e)))?;
321338

322339
if !status.success() {
323-
return Err(ToolError::ExecutionError(
324-
format!("Failed to install {}", package),
325-
));
340+
log::warn!("Failed to install uv, falling back to regular pip for package installation");
341+
342+
// If uv installation fails, fall back to regular pip for package installation
343+
let mut cmd = Command::new(&pip);
344+
cmd.arg("install");
345+
346+
// Add all packages as arguments
347+
for package in &self.packages {
348+
cmd.arg(package);
349+
}
350+
351+
log::debug!("Running pip command: {:?}", cmd);
352+
353+
let output = cmd.output()
354+
.map_err(|e| ToolError::ExecutionError(format!("Failed to install packages with pip: {}", e)))?;
355+
356+
if !output.status.success() {
357+
let stderr = String::from_utf8_lossy(&output.stderr);
358+
let stdout = String::from_utf8_lossy(&output.stdout);
359+
log::error!("pip stderr: {}", stderr);
360+
log::error!("pip stdout: {}", stdout);
361+
return Err(ToolError::ExecutionError(
362+
format!("Failed to install packages with pip: {}", stderr),
363+
));
364+
}
365+
366+
log::debug!("Successfully installed packages with pip");
367+
} else {
368+
// If uv installation succeeds, use it to install packages
369+
let uv = if cfg!(windows) {
370+
ctx.install_dir.join("Scripts").join("uv.exe")
371+
} else {
372+
ctx.install_dir.join("bin").join("uv")
373+
};
374+
375+
// Check if the uv executable exists
376+
if !uv.exists() {
377+
log::warn!("uv executable not found at {:?}, falling back to regular pip", uv);
378+
379+
// If uv is not found, fall back to regular pip
380+
let mut cmd = Command::new(&pip);
381+
cmd.arg("install");
382+
383+
// Add all packages as arguments
384+
for package in &self.packages {
385+
cmd.arg(package);
386+
}
387+
388+
log::debug!("Running pip command: {:?}", cmd);
389+
390+
let output = cmd.output()
391+
.map_err(|e| ToolError::ExecutionError(format!("Failed to install packages with pip: {}", e)))?;
392+
393+
if !output.status.success() {
394+
let stderr = String::from_utf8_lossy(&output.stderr);
395+
let stdout = String::from_utf8_lossy(&output.stdout);
396+
log::error!("pip stderr: {}", stderr);
397+
log::error!("pip stdout: {}", stdout);
398+
return Err(ToolError::ExecutionError(
399+
format!("Failed to install packages with pip: {}", stderr),
400+
));
401+
}
402+
403+
log::debug!("Successfully installed packages with pip");
404+
} else {
405+
// Use uv to install packages
406+
let mut cmd = Command::new(&uv);
407+
cmd.arg("pip")
408+
.arg("install");
409+
410+
// Add all packages as arguments
411+
for package in &self.packages {
412+
cmd.arg(package);
413+
}
414+
415+
log::debug!("Running uv command: {:?}", cmd);
416+
417+
let output = cmd.output()
418+
.map_err(|e| ToolError::ExecutionError(format!("Failed to install packages with uv: {}", e)))?;
419+
420+
if !output.status.success() {
421+
let stderr = String::from_utf8_lossy(&output.stderr);
422+
let stdout = String::from_utf8_lossy(&output.stdout);
423+
log::error!("uv stderr: {}", stderr);
424+
log::error!("uv stdout: {}", stdout);
425+
426+
log::warn!("Failed to install packages with uv, falling back to regular pip");
427+
428+
// If uv fails, fall back to regular pip
429+
let mut cmd = Command::new(&pip);
430+
cmd.arg("install");
431+
432+
// Add all packages as arguments
433+
for package in &self.packages {
434+
cmd.arg(package);
435+
}
436+
437+
log::debug!("Running pip command: {:?}", cmd);
438+
439+
let output = cmd.output()
440+
.map_err(|e| ToolError::ExecutionError(format!("Failed to install packages with pip: {}", e)))?;
441+
442+
if !output.status.success() {
443+
let stderr = String::from_utf8_lossy(&output.stderr);
444+
let stdout = String::from_utf8_lossy(&output.stdout);
445+
log::error!("pip stderr: {}", stderr);
446+
log::error!("pip stdout: {}", stdout);
447+
return Err(ToolError::ExecutionError(
448+
format!("Failed to install packages with pip: {}", stderr),
449+
));
450+
}
451+
452+
log::debug!("Successfully installed packages with pip");
453+
} else {
454+
log::debug!("Successfully installed packages with uv");
455+
}
456+
}
326457
}
327458
}
328459

@@ -399,14 +530,28 @@ impl Tool for PythonTool {
399530
}
400531

401532
fn is_installed(&self) -> bool {
533+
// For Python tools, we need to check if the Python executable and the tool executable exist
534+
let python_path = if cfg!(windows) {
535+
self.install_dir.join("Scripts").join("python.exe")
536+
} else {
537+
self.install_dir.join("bin").join("python")
538+
};
539+
402540
// Check if the tool executable exists in the virtualenv
403541
let tool_path = if cfg!(windows) {
404542
self.install_dir.join("Scripts").join(format!("{}.exe", self.name))
405543
} else {
406544
self.install_dir.join("bin").join(&self.name)
407545
};
408546

409-
tool_path.exists()
547+
// Log the paths for debugging
548+
log::debug!("Checking if Python tool is installed:");
549+
log::debug!(" Python path: {:?}, exists: {}", python_path, python_path.exists());
550+
log::debug!(" Tool path: {:?}, exists: {}", tool_path, tool_path.exists());
551+
552+
// For Python tools, we consider them installed if both the Python executable
553+
// and the tool executable exist
554+
python_path.exists() && tool_path.exists()
410555
}
411556

412557
fn install_dir(&self) -> &PathBuf {

tests/toolchain_tests.rs

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
//! Tests for toolchain functionality
2+
3+
use rustyhook::toolchains::{PythonTool, Tool, SetupContext};
4+
5+
#[test]
6+
fn test_python_tool_with_uv() {
7+
// Create a temporary directory for the test
8+
let temp_dir = tempfile::tempdir().unwrap();
9+
let cache_dir = temp_dir.path().join(".rustyhook").join("cache");
10+
11+
// Create a Python tool with a test package
12+
let python_tool = PythonTool::new("pytest", "1.0.0", vec!["pytest".to_string()]);
13+
14+
// Get the installation directory from the Python tool
15+
let install_dir = python_tool.install_dir().clone();
16+
17+
// Create the cache directory
18+
std::fs::create_dir_all(&cache_dir).unwrap();
19+
20+
// Create a setup context
21+
let ctx = SetupContext {
22+
cache_dir: cache_dir.clone(),
23+
install_dir: install_dir.clone(),
24+
force: false,
25+
version: Some("1.0.0".to_string()),
26+
};
27+
28+
// Set up the Python tool (this will install uv and use it to install pytest)
29+
let result = python_tool.setup(&ctx);
30+
31+
// Check that the setup was successful
32+
assert!(result.is_ok(), "Failed to set up Python tool: {:?}", result);
33+
34+
// Check the installation directory structure
35+
println!("Installation directory: {:?}", install_dir);
36+
if install_dir.exists() {
37+
println!("Installation directory exists");
38+
39+
// List the contents of the installation directory
40+
if let Ok(entries) = std::fs::read_dir(&install_dir) {
41+
println!("Contents of installation directory:");
42+
for entry in entries {
43+
if let Ok(entry) = entry {
44+
println!(" {:?}", entry.path());
45+
}
46+
}
47+
} else {
48+
println!("Failed to read installation directory");
49+
}
50+
51+
// Check bin directory
52+
let bin_dir = if cfg!(windows) {
53+
install_dir.join("Scripts")
54+
} else {
55+
install_dir.join("bin")
56+
};
57+
58+
if bin_dir.exists() {
59+
println!("Bin directory exists: {:?}", bin_dir);
60+
61+
// List the contents of the bin directory
62+
if let Ok(entries) = std::fs::read_dir(&bin_dir) {
63+
println!("Contents of bin directory:");
64+
for entry in entries {
65+
if let Ok(entry) = entry {
66+
println!(" {:?}", entry.path());
67+
}
68+
}
69+
} else {
70+
println!("Failed to read bin directory");
71+
}
72+
} else {
73+
println!("Bin directory does not exist: {:?}", bin_dir);
74+
}
75+
} else {
76+
println!("Installation directory does not exist");
77+
}
78+
79+
// Check that the Python tool is installed
80+
println!("Checking if Python tool is installed...");
81+
let is_installed = python_tool.is_installed();
82+
println!("Python tool is installed: {}", is_installed);
83+
84+
// Get the tool path that is being checked
85+
let tool_path = if cfg!(windows) {
86+
install_dir.join("Scripts").join(format!("{}.exe", python_tool.name()))
87+
} else {
88+
install_dir.join("bin").join(python_tool.name())
89+
};
90+
println!("Tool path being checked: {:?}", tool_path);
91+
println!("Tool path exists: {}", tool_path.exists());
92+
93+
// Check that the pytest package is installed
94+
let pytest_path = if cfg!(windows) {
95+
install_dir.join("Scripts").join("pytest.exe")
96+
} else {
97+
install_dir.join("bin").join("pytest")
98+
};
99+
println!("Pytest path: {:?}", pytest_path);
100+
println!("Pytest path exists: {}", pytest_path.exists());
101+
102+
// Assert that the Python tool is installed
103+
assert!(is_installed, "Python tool is not installed");
104+
105+
// Assert that the pytest package is installed
106+
assert!(pytest_path.exists(), "pytest package is not installed");
107+
}

0 commit comments

Comments
 (0)