mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-10 03:30:59 +00:00
fix(clippy): clean up nightly-clippy findings on merged CLI work
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) <noreply@anthropic.com>
This commit is contained in:
parent
948d59062a
commit
5c248fc885
7 changed files with 43 additions and 27 deletions
|
|
@ -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<PathBuf> {
|
||||
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
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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::<serde_json::Value>(
|
||||
let pid_u64 = serde_json::from_str::<serde_json::Value>(
|
||||
&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 = {
|
||||
|
|
|
|||
|
|
@ -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::<serde_json::Value>(
|
||||
let pid_u64 = serde_json::from_str::<serde_json::Value>(
|
||||
&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);
|
||||
|
|
|
|||
|
|
@ -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::<serde_json::Value>(
|
||||
let pid_u64 = serde_json::from_str::<serde_json::Value>(
|
||||
&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"));
|
||||
|
|
|
|||
|
|
@ -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<PathBuf> {
|
|||
.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()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<Vec<RunInfo>> {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue