From 5c248fc885654ce1f9efc1f5409c8702c92b3d67 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sun, 19 Apr 2026 17:39:06 -0400 Subject: [PATCH] fix(clippy): clean up nightly-clippy findings on merged CLI work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing behavioral — each change is what clippy asked for: - fabro-test: wrap the three polling-helper thread::sleep calls in a single poll_sleep() with an #[expect(clippy::disallowed_methods, reason = …)] since the helpers are deliberately blocking - fabro-test: server_log_files now uses Path::extension() with eq_ignore_ascii_case("log") instead of a case-sensitive ends_with - fabro-workflow: import default_storage_dir rather than calling it through its full module path - fabro-cli/server/record: same absolute_paths fix - fabro-cli/main tests: use a `use tokio::runtime::Runtime` to stop referencing `tokio::runtime::Runtime` by full path - fabro-cli/tests: replace three `as u32` casts on as_u64() results with u32::try_from(...).expect(…) - fabro-cli/tests: six `format!("...", var)` assertions switched to the inline `{var}` form clippy prefers Full verification passes: fmt, clippy, cargo nextest (4141 tests), bun typecheck, bun test (40 tests), bun build, SPA embed diff clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../fabro-cli/src/commands/server/record.rs | 3 ++- lib/crates/fabro-cli/src/main.rs | 5 ++-- .../fabro-cli/tests/it/cmd/server_start.rs | 23 +++++++--------- .../fabro-cli/tests/it/cmd/server_status.rs | 5 ++-- .../fabro-cli/tests/it/cmd/uninstall.rs | 5 ++-- lib/crates/fabro-test/src/lib.rs | 26 +++++++++++++++---- lib/crates/fabro-workflow/src/run_lookup.rs | 3 ++- 7 files changed, 43 insertions(+), 27 deletions(-) diff --git a/lib/crates/fabro-cli/src/commands/server/record.rs b/lib/crates/fabro-cli/src/commands/server/record.rs index 1a6b15b5a..077c06e20 100644 --- a/lib/crates/fabro-cli/src/commands/server/record.rs +++ b/lib/crates/fabro-cli/src/commands/server/record.rs @@ -3,6 +3,7 @@ use std::path::{Path, PathBuf}; use anyhow::{Context, Result, bail}; use chrono::{DateTime, Utc}; use fabro_config::Storage; +use fabro_config::user::default_storage_dir; use fabro_server::bind::Bind; use fabro_util::Home; use serde::{Deserialize, Serialize}; @@ -50,7 +51,7 @@ fn server_record_path(storage_dir: &Path) -> PathBuf { } fn legacy_record_path(storage_dir: &Path) -> Option { - if storage_dir == fabro_config::user::default_storage_dir() { + if storage_dir == default_storage_dir() { Some(Home::from_env().root().join("server.json")) } else { None diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs index d1cf06709..6271fb736 100644 --- a/lib/crates/fabro-cli/src/main.rs +++ b/lib/crates/fabro-cli/src/main.rs @@ -489,11 +489,12 @@ mod tests { Commands, InstallGitHubStrategyArg, ModelsCommand, ProviderCommand, ProviderNamespace, StoreCommand, StoreNamespace, }; + use tokio::runtime::Runtime; use super::*; - fn runtime() -> tokio::runtime::Runtime { - tokio::runtime::Runtime::new().expect("runtime should build") + fn runtime() -> Runtime { + Runtime::new().expect("runtime should build") } fn write_test_settings(path: &std::path::Path) { diff --git a/lib/crates/fabro-cli/tests/it/cmd/server_start.rs b/lib/crates/fabro-cli/tests/it/cmd/server_start.rs index 2415c28a2..2cf24d8e7 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/server_start.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/server_start.rs @@ -244,14 +244,12 @@ fn foreground_start_writes_tracing_to_storage_server_log() { ); assert!( !storage_log.contains("stale pre-start log entry"), - "expected startup to truncate stale log contents, got:\n{}", - storage_log + "expected startup to truncate stale log contents, got:\n{storage_log}", ); assert!( storage_log.find("API server started") < storage_log.find("Shutdown signal received, stopping server"), - "expected shutdown trace to append after startup trace, got:\n{}", - storage_log + "expected shutdown trace to append after startup trace, got:\n{storage_log}", ); let home_server_logs = server_log_files(&home_dir.path().join(".fabro").join("logs")); @@ -316,14 +314,12 @@ fn daemon_start_writes_tracing_to_storage_server_log() { ); assert!( !storage_log.contains("stale pre-start log entry"), - "expected startup to truncate stale log contents, got:\n{}", - storage_log + "expected startup to truncate stale log contents, got:\n{storage_log}", ); assert!( storage_log.find("API server started") < storage_log.find("Shutdown signal received, stopping server"), - "expected shutdown trace to append after startup trace, got:\n{}", - storage_log + "expected shutdown trace to append after startup trace, got:\n{storage_log}", ); let home_server_logs = server_log_files(&context.home_dir.join(".fabro").join("logs")); @@ -382,12 +378,13 @@ fn start_errors_when_only_a_legacy_running_server_record_exists() { .expect("server start retry should run") }; - let pid = serde_json::from_str::( + let pid_u64 = serde_json::from_str::( &std::fs::read_to_string(&legacy_record).unwrap(), ) .unwrap()["pid"] .as_u64() - .unwrap() as u32; + .unwrap(); + let pid = u32::try_from(pid_u64).expect("pid fits in u32"); stop_pid(pid); let _ = std::fs::remove_file(&legacy_record); let _ = std::fs::remove_file(&socket_path); @@ -489,13 +486,11 @@ fn concurrent_foreground_start_does_not_retruncate_storage_server_log() { let storage_log = std::fs::read_to_string(&storage_log_path).unwrap_or_default(); assert!( storage_log.contains(marker.trim_end()), - "expected second start to avoid retruncating the log, got:\n{}", - storage_log + "expected second start to avoid retruncating the log, got:\n{storage_log}", ); assert!( !storage_log.contains("stale pre-start log entry"), - "expected the first start to truncate stale log contents, got:\n{}", - storage_log + "expected the first start to truncate stale log contents, got:\n{storage_log}", ); let stop_output = { diff --git a/lib/crates/fabro-cli/tests/it/cmd/server_status.rs b/lib/crates/fabro-cli/tests/it/cmd/server_status.rs index a29f63d68..9d017b71f 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/server_status.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/server_status.rs @@ -88,12 +88,13 @@ fn status_errors_when_only_a_legacy_running_server_record_exists() { .expect("server status should run") }; - let pid = serde_json::from_str::( + let pid_u64 = serde_json::from_str::( &std::fs::read_to_string(&legacy_record).unwrap(), ) .unwrap()["pid"] .as_u64() - .unwrap() as u32; + .unwrap(); + let pid = u32::try_from(pid_u64).expect("pid fits in u32"); stop_pid(pid); let _ = std::fs::remove_file(&legacy_record); let _ = std::fs::remove_file(&socket_path); diff --git a/lib/crates/fabro-cli/tests/it/cmd/uninstall.rs b/lib/crates/fabro-cli/tests/it/cmd/uninstall.rs index daccce700..cb26b3bec 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/uninstall.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/uninstall.rs @@ -203,12 +203,13 @@ fn uninstall_yes_fails_when_only_a_legacy_running_server_record_exists() { wait_for_path(¤t_record); let legacy_record = fabro_home.join("server.json"); std::fs::rename(¤t_record, &legacy_record).unwrap(); - let pid = serde_json::from_str::( + let pid_u64 = serde_json::from_str::( &std::fs::read_to_string(&legacy_record).unwrap(), ) .unwrap()["pid"] .as_u64() - .unwrap() as u32; + .unwrap(); + let pid = u32::try_from(pid_u64).expect("pid fits in u32"); let uninstall_output = { let mut uninstall = std::process::Command::new(env!("CARGO_BIN_EXE_fabro")); diff --git a/lib/crates/fabro-test/src/lib.rs b/lib/crates/fabro-test/src/lib.rs index 4eee2c974..28e2b1e8f 100644 --- a/lib/crates/fabro-test/src/lib.rs +++ b/lib/crates/fabro-test/src/lib.rs @@ -159,6 +159,17 @@ pub fn isolated_storage_dir() -> tempfile::TempDir { root } +/// Sleep tick for the test polling helpers below. These are synchronous +/// helpers called from blocking integration tests — there's no runtime to +/// hand off to, so `std::thread::sleep` is the right primitive. +#[expect( + clippy::disallowed_methods, + reason = "sync polling helper for blocking integration tests" +)] +fn poll_sleep() { + std::thread::sleep(std::time::Duration::from_millis(50)); +} + /// Poll up to 5s for a path to appear; panic on timeout. pub fn wait_for_path(path: &Path) { let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); @@ -166,7 +177,7 @@ pub fn wait_for_path(path: &Path) { if path.exists() { return; } - std::thread::sleep(std::time::Duration::from_millis(50)); + poll_sleep(); } panic!("timed out waiting for {}", path.display()); } @@ -182,7 +193,7 @@ pub fn wait_for_log_line(path: &Path, needle: &str) { { return; } - std::thread::sleep(std::time::Duration::from_millis(50)); + poll_sleep(); } panic!("timed out waiting for {needle:?} in {}", path.display()); } @@ -195,7 +206,7 @@ pub fn stop_pid(pid: u32) { if !fabro_proc::process_alive(pid) { return; } - std::thread::sleep(std::time::Duration::from_millis(50)); + poll_sleep(); } fabro_proc::sigkill(pid); } @@ -212,9 +223,14 @@ pub fn server_log_files(logs_dir: &Path) -> Vec { .flatten() .map(|entry| entry.path()) .filter(|path| { - path.file_name() + let has_log_ext = path + .extension() + .is_some_and(|ext| ext.eq_ignore_ascii_case("log")); + let has_server_prefix = path + .file_name() .and_then(|name| name.to_str()) - .is_some_and(|name| name.starts_with("server.") && name.ends_with(".log")) + .is_some_and(|name| name.starts_with("server.")); + has_log_ext && has_server_prefix }) .collect() } diff --git a/lib/crates/fabro-workflow/src/run_lookup.rs b/lib/crates/fabro-workflow/src/run_lookup.rs index 0e3a09318..79c08f3e5 100644 --- a/lib/crates/fabro-workflow/src/run_lookup.rs +++ b/lib/crates/fabro-workflow/src/run_lookup.rs @@ -4,6 +4,7 @@ use std::path::{Path, PathBuf}; use anyhow::{Context, Result, bail}; use chrono::{DateTime, Utc}; use fabro_config::Storage; +use fabro_config::user::default_storage_dir; use fabro_store::{Database, RunSummary}; use fabro_types::RunId; use serde::Serialize; @@ -140,7 +141,7 @@ pub fn scratch_base(storage_dir: &Path) -> PathBuf { } pub fn default_scratch_base() -> PathBuf { - scratch_base(&fabro_config::user::default_storage_dir()) + scratch_base(&default_storage_dir()) } fn scan_orphan_runs(base: &Path) -> Result> {