admin/OpenJarvis
1
0
Fork 0
mirror of https://github.com/open-jarvis/OpenJarvis.git synced 2026-08-14 00:47:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(desktop): extract testable uv-sync error helpers + unit tests (#331)

Browse Source 
The uv-sync failure handling added in #398 was inline in the async
`boot_backend` GUI path, so its logic — exit-code rendering and the
stderr "tail" extraction — could only be verified by running the
desktop app. This extracts the pure logic into three free functions
and adds unit tests, so the message formatting is now covered by
`cargo test` with no GUI / webview runtime needed.

Extracted:
- `uv_sync_stderr_tail(stderr, max_chars)` — last N chars of stderr,
  trimmed, on char boundaries. The original inline version used
  `.rev().take(N).collect().chars().rev().collect()`; the new version
  is `skip(count - N)` which is clearer and equally UTF-8-safe (matters
  because Windows consoles emit non-ASCII cp9xx bytes — a byte slice
  could split a codepoint).
- `format_uv_sync_failure(root, exit_code, stderr)` — the non-zero-exit
  message. Now renders a missing exit code (signal-terminated process)
  as "unknown" instead of a misleading "-1".
- `format_uv_sync_spawn_error(root, uv_bin, err)` — the can't-spawn
  message.

`boot_backend` now calls these instead of formatting inline.

Tests (7, in `#[cfg(test)] mod tests`):
- tail returns whole string when shorter than the limit
- tail keeps the END (the actionable line), not the spinner-noise start
- tail trims surrounding whitespace
- tail never splits a multi-byte codepoint (500×"é", limit 100 → exactly
  100 chars, all "é")
- failure message includes exit code + stderr tail + the actionable
  "run uv sync manually" hint
- missing exit code renders as "exit unknown", never "exit -1"
- spawn-error names the uv binary path and repo root

Verified the logic standalone via `rustc --test` (7/7 pass). In CI they
run via `cargo test` in desktop.yml's `validate` job, which already
builds the Tauri crate with the webkit deps and runs on every PR that
touches `frontend/**` — so this changes the existing `cargo check` step
to `cargo test` (a superset: same build coverage, plus the tests).

This doesn't verify the GUI *behavior* (that still needs a human running
the app, or the windows-latest empirical path) — but the error-message
logic that was previously untestable now has automated coverage.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
krypticmouse
2026-05-24 15:26:07 +00:00
co-authored by Claude Opus 4.7
parent 0c34817ecd
commit 658effb964
2 changed files with 131 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/workflows/desktop.yml
+5 -2
View File
@@ -66,9 +66,12 @@ jobs:
- name: Create frontend dist stub
run: mkdir -p frontend/dist && echo '<html><body></body></html>' > frontend/dist/index.html
- name: Cargo check
# `cargo test` builds the crate (same coverage as the old `cargo check`)
# and runs the unit tests, including the #331 uv-sync error-formatting
# helpers. Static command, no untrusted input — no injection surface.
- name: Cargo test
working-directory: frontend/src-tauri
run: cargo check
run: cargo test
# Remove stale artifacts from the desktop-latest pre-release so that
# only the current build's files are available for download.
frontend/src-tauri/src/lib.rs
+126 -25
View File
@@ -405,6 +405,57 @@ async fn pull_model(model: &str) -> Result<(), String> {
Ok(())
}
// ---------------------------------------------------------------------------
// uv sync error formatting (pure helpers — unit-tested, see #331)
// ---------------------------------------------------------------------------
/// Last `max_chars` characters of a `uv sync` stderr stream, trimmed.
///
/// uv's actionable diagnostic almost always lands at the tail of the
/// stream, so when surfacing a failure to the user we show the end, not
/// the (usually noisy progress-spinner) beginning. Operates on `char`
/// boundaries so it never splits a multi-byte UTF-8 codepoint — important
/// because Windows consoles emit non-ASCII (cp9xx) bytes.
fn uv_sync_stderr_tail(stderr: &str, max_chars: usize) -> String {
let total = stderr.chars().count();
let skip = total.saturating_sub(max_chars);
stderr.chars().skip(skip).collect::<String>().trim().to_string()
}
/// Error message shown when `uv sync` runs but exits non-zero (#331).
///
/// `exit_code` is `None` when the process was terminated by a signal with
/// no exit code (rendered as "unknown" rather than a misleading -1).
fn format_uv_sync_failure(
root: &std::path::Path,
exit_code: Option<i32>,
stderr: &str,
) -> String {
let code = exit_code
.map(|c| c.to_string())
.unwrap_or_else(|| "unknown".to_string());
format!(
"`uv sync` failed in {} (exit {}). Last output:\n\n{}\n\n\
Try opening a terminal in that directory and running \
`uv sync --extra server` manually for the full output.",
root.display(),
code,
uv_sync_stderr_tail(stderr, 800),
)
}
/// Error message shown when `uv sync` can't even be spawned (#331) —
/// e.g. the resolved `uv` binary doesn't exist or isn't executable.
fn format_uv_sync_spawn_error(root: &std::path::Path, uv_bin: &str, err: &str) -> String {
format!(
"Could not run `uv sync`: {}. Verify uv is installed at \
`{}` and the OpenJarvis repo is at `{}`.",
err,
uv_bin,
root.display(),
)
}
// ---------------------------------------------------------------------------
// Backend boot sequence (runs in background after app launch)
// ---------------------------------------------------------------------------
@@ -695,36 +746,13 @@ async fn boot_backend(backend: SharedBackend, status: SharedStatus) {
match sync_output {
Ok(out) if !out.status.success() => {
let stderr = String::from_utf8_lossy(&out.stderr);
// Take the last ~800 chars — uv's most useful diagnostic line
// is usually at the tail of the stream.
let tail: String = stderr
.chars()
.rev()
.take(800)
.collect::<String>()
.chars()
.rev()
.collect();
let mut s = status.lock().await;
s.error = Some(format!(
"`uv sync` failed in {} (exit {}). Last output:\n\n{}\n\n\
Try opening a terminal in that directory and running \
`uv sync --extra server` manually for the full output.",
root.display(),
out.status.code().unwrap_or(-1),
tail.trim(),
));
s.error = Some(format_uv_sync_failure(root, out.status.code(), &stderr));
return;
}
Err(e) => {
let mut s = status.lock().await;
s.error = Some(format!(
"Could not run `uv sync`: {}. Verify uv is installed at \
`{}` and the OpenJarvis repo is at `{}`.",
e,
uv_bin,
root.display(),
));
s.error = Some(format_uv_sync_spawn_error(root, &uv_bin, &e.to_string()));
return;
}
Ok(_) => {} // success — fall through
@@ -1788,3 +1816,76 @@ pub fn run() {
}
});
}
// ---------------------------------------------------------------------------
// Tests
// ---------------------------------------------------------------------------
#[cfg(test)]
mod tests {
use super::{format_uv_sync_failure, format_uv_sync_spawn_error, uv_sync_stderr_tail};
use std::path::Path;
#[test]
fn tail_returns_whole_string_when_shorter_than_limit() {
assert_eq!(uv_sync_stderr_tail("short error", 800), "short error");
}
#[test]
fn tail_keeps_the_end_not_the_beginning() {
// uv's actionable line is at the end; the spinner noise is at the start.
let s = format!("{}ACTUAL ERROR HERE", "spinner-noise ".repeat(200));
let tail = uv_sync_stderr_tail(&s, 40);
assert!(tail.ends_with("ACTUAL ERROR HERE"), "tail was: {tail:?}");
assert!(!tail.contains("spinner-noise spinner-noise spinner-noise"));
assert!(tail.chars().count() <= 40);
}
#[test]
fn tail_trims_surrounding_whitespace() {
assert_eq!(uv_sync_stderr_tail(" \n padded \n ", 800), "padded");
}
#[test]
fn tail_never_splits_a_multibyte_codepoint() {
// Each "é" is 2 bytes / 1 char. A byte-based slice could panic or
// produce invalid UTF-8; the char-based tail must not.
let s = "é".repeat(500);
let tail = uv_sync_stderr_tail(&s, 100);
assert_eq!(tail.chars().count(), 100);
assert!(tail.chars().all(|c| c == 'é'));
}
#[test]
fn failure_message_includes_exit_code_and_tail_and_hint() {
let msg = format_uv_sync_failure(
Path::new("/home/u/.openjarvis/src"),
Some(2),
"error: failed to resolve numpy==2.1.3",
);
assert!(msg.contains("exit 2"));
assert!(msg.contains("/home/u/.openjarvis/src"));
assert!(msg.contains("failed to resolve numpy==2.1.3"));
assert!(msg.contains("uv sync --extra server")); // actionable next step
}
#[test]
fn failure_message_renders_missing_exit_code_as_unknown() {
// Process killed by signal → no exit code. Must not show a misleading -1.
let msg = format_uv_sync_failure(Path::new("/x"), None, "boom");
assert!(msg.contains("exit unknown"));
assert!(!msg.contains("exit -1"));
}
#[test]
fn spawn_error_names_the_binary_and_root() {
let msg = format_uv_sync_spawn_error(
Path::new("/repo"),
"C:\\Users\\me\\.local\\bin\\uv.exe",
"No such file or directory (os error 2)",
);
assert!(msg.contains("C:\\Users\\me\\.local\\bin\\uv.exe"));
assert!(msg.contains("/repo"));
assert!(msg.contains("No such file or directory"));
}
}