Skip to content

Commit 8d22e8e

Browse files
pmaxhoganclaude
andcommitted
test: close the coverage gap on the bottleneck classifier's Rust surface
PR #311's coverage gate flagged the new backend surface as under-covered (84.34% vs main's 84.56%). Adds targeted unit tests for every area the gate report highlighted: - bottleneck_hub.rs: extracted the AppHandle-independent decision logic out of tick_once into pure, directly-testable helpers (account_signal, aggregate_accounts, build_snapshot, should_emit), each covering a branch the monolithic async function previously hid from a unit test. Also covers BottleneckHub::push/latest directly. - SyncOrchestrator::pacer_backoff_remaining_ms / backend_label: default (unwired), wired-but-clear (FakePacer), and wired-and-throttled (a real AimdPacer that has actually rate-limited) cases; plus a driven-app test proving FakeOrchestrator inherits the Orchestrator trait's default bodies. - AimdPacer::backoff_remaining_ms: already covered by the previous commit's test, still verified against a FakeClock in and after the backoff window. - IoCounters::add_hashed/snapshot: already covered by the previous commit's test. - Executor credit paths: new inline_upload (buffered, <4MiB) and stream_upload (streaming cpu_stage, >=4MiB) tests asserting the hash counter is credited the full plaintext size in both pipelines. - Scanner credit path: a direct scan_with_priority call (bypassing the scan()/scan_with_progress() wrappers, which always pass None) proving a deep-verify re-hash credits the hash counter. - assembly.rs: bottleneck_backend_label covers all four BackendKind variants with distinct labels. - app_state.rs: bottleneck_hub()/set_bottleneck_task()/shutdown_bottleneck_task() round-trip (register, signal, join, idempotent no-op). Residual uncovered lines are structural: tick_once's AppHandle/AppState plumbing, the bottleneck_status Tauri command (needs a real tauri::State, which this codebase has no harness to construct - matches the pre-existing untested pattern for io_throughput_series/get_sync_status), the emit_sync_bottleneck event (needs a real AppHandle, matches emit_sync_io_throughput's own untested baseline), and the lib.rs sampler spawn/drain wiring (needs full app boot). None of these are new patterns - each mirrors an already-uncovered sibling that predates this PR. Local cargo llvm-cov (same command as .github/workflows/coverage.yml) moved 83.73% -> 83.96% on this diff; cargo test / clippy -D warnings / fmt all clean (1032 driven-core + driven-app tests passing). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019xKUm9vH4ifb5LHR5szy1v
1 parent 264c4ca commit 8d22e8e

6 files changed

Lines changed: 538 additions & 53 deletions

File tree

crates/driven-core/src/executor.rs

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12295,6 +12295,65 @@ mod tests {
1229512295
);
1229612296
}
1229712297

12298+
/// issue #308 bottleneck classifier: a fresh small-file upload (the
12299+
/// BUFFERED `inline_upload` path, below [`PIPELINE_THRESHOLD`]) credits
12300+
/// the hash counter with the whole plaintext, alongside the existing
12301+
/// disk-read credit - both happen in the same `read_hash_encrypt` pass.
12302+
#[tokio::test]
12303+
async fn inline_upload_credits_hashed_bytes() {
12304+
let h = harness().await;
12305+
let body = vec![7u8; 4096];
12306+
let (rel, size) = h.write_file("small.bin", &body);
12307+
12308+
let io = Arc::new(crate::iostat::IoCounters::default());
12309+
let exec = h.executor().with_io_counters(io.clone());
12310+
let out = exec
12311+
.execute(
12312+
&h.source,
12313+
&h.upload_plan(&rel, size),
12314+
&noop_progress,
12315+
&noop_outcome,
12316+
)
12317+
.await
12318+
.unwrap();
12319+
assert!(matches!(out[0], OpOutcome::Done { .. }), "got {:?}", out[0]);
12320+
12321+
let snap = io.snapshot();
12322+
assert_eq!(snap.hashed_bytes, size, "the whole plaintext was hashed");
12323+
assert_eq!(snap.disk_read_bytes, size, "and read from disk");
12324+
}
12325+
12326+
/// issue #308 bottleneck classifier: a fresh large-file upload (the
12327+
/// STREAMING `cpu_stage` path, at/above [`PIPELINE_THRESHOLD`]) credits
12328+
/// the hash counter chunk-by-chunk as it streams, summing to the whole
12329+
/// plaintext by the time the upload completes.
12330+
#[tokio::test]
12331+
async fn stream_upload_credits_hashed_bytes() {
12332+
let h = harness().await;
12333+
let size_bytes = (PIPELINE_THRESHOLD + 64 * 1024) as usize;
12334+
let body: Vec<u8> = (0..size_bytes).map(|i| (i % 251) as u8).collect();
12335+
let (rel, size) = h.write_file("streamed.bin", &body);
12336+
12337+
let io = Arc::new(crate::iostat::IoCounters::default());
12338+
let exec = h.executor().with_io_counters(io.clone());
12339+
let out = exec
12340+
.execute(
12341+
&h.source,
12342+
&h.upload_plan(&rel, size),
12343+
&noop_progress,
12344+
&noop_outcome,
12345+
)
12346+
.await
12347+
.unwrap();
12348+
assert!(matches!(out[0], OpOutcome::Done { .. }), "got {:?}", out[0]);
12349+
12350+
let snap = io.snapshot();
12351+
assert_eq!(
12352+
snap.hashed_bytes, size,
12353+
"every streamed chunk's bytes were credited to the hash counter"
12354+
);
12355+
}
12356+
1229812357
/// [`ResumeAcc`]'s gauge accounting is symmetric across push / partial
1229912358
/// drain / clear, the drain clamps at the buffered length, and DROP
1230012359
/// refunds whatever is left - the guarantee the error-unwind test relies

crates/driven-core/src/orchestrator.rs

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4458,6 +4458,86 @@ mod tests {
44584458
}
44594459
}
44604460

4461+
#[test]
4462+
fn pacer_backoff_and_backend_label_default_when_unwired() {
4463+
// issue #308: a trait object built without `with_pacer` /
4464+
// `with_backend_label` (the historical construction, and every test
4465+
// fake elsewhere in this crate) must read as "clear" with a generic
4466+
// label, never panic or silently misreport a backoff.
4467+
let account = AccountId::new_v4();
4468+
let dir = tempfile::tempdir().unwrap();
4469+
let src = source_in(account, dir.path());
4470+
let exec = Arc::new(RecordingExecutor::default());
4471+
let (orch, _clock) = build(
4472+
account,
4473+
vec![src],
4474+
exec,
4475+
power_on_ac(),
4476+
Arc::new(FakeNet::online()),
4477+
OrchestratorConfig::default(),
4478+
);
4479+
assert_eq!(orch.pacer_backoff_remaining_ms(), None);
4480+
assert_eq!(orch.backend_label(), "your destination");
4481+
}
4482+
4483+
#[test]
4484+
fn pacer_backoff_delegates_to_the_wired_pacer_when_clear() {
4485+
// issue #308: a wired-but-not-throttling pacer (`FakePacer`'s default
4486+
// `backoff_remaining_ms`, which it does not override) still reads as
4487+
// clear through the orchestrator - `with_pacer` alone must not
4488+
// fabricate a backoff.
4489+
let account = AccountId::new_v4();
4490+
let dir = tempfile::tempdir().unwrap();
4491+
let src = source_in(account, dir.path());
4492+
let exec = Arc::new(RecordingExecutor::default());
4493+
let (orch, _clock) = build(
4494+
account,
4495+
vec![src],
4496+
exec,
4497+
power_on_ac(),
4498+
Arc::new(FakeNet::online()),
4499+
OrchestratorConfig::default(),
4500+
);
4501+
let orch = orch
4502+
.with_pacer(Arc::new(FakePacer::default()))
4503+
.with_backend_label("Drive");
4504+
assert_eq!(orch.pacer_backoff_remaining_ms(), None);
4505+
assert_eq!(orch.backend_label(), "Drive");
4506+
}
4507+
4508+
#[test]
4509+
fn pacer_backoff_reports_the_live_deadline_from_a_throttled_pacer() {
4510+
// issue #308: a REAL AimdPacer that has actually throttled reports its
4511+
// live remaining-ms through the orchestrator, non-blocking (no
4512+
// `.await`, no clock advance needed to observe it).
4513+
let account = AccountId::new_v4();
4514+
let dir = tempfile::tempdir().unwrap();
4515+
let src = source_in(account, dir.path());
4516+
let exec = Arc::new(RecordingExecutor::default());
4517+
let (orch, _clock) = build(
4518+
account,
4519+
vec![src],
4520+
exec,
4521+
power_on_ac(),
4522+
Arc::new(FakeNet::online()),
4523+
OrchestratorConfig::default(),
4524+
);
4525+
4526+
let pacer_clock: Arc<dyn Clock> = Arc::new(FakeClock::new());
4527+
let pacer = crate::pacer::AimdPacer::new(pacer_clock, None);
4528+
pacer.note_response(crate::pacer::ResponseClass::RateLimited {
4529+
retry_after: std::time::Duration::from_secs(5),
4530+
});
4531+
let pacer: Arc<dyn Pacer> = Arc::new(pacer);
4532+
let orch = orch.with_pacer(pacer).with_backend_label("S3");
4533+
4534+
let remaining = orch
4535+
.pacer_backoff_remaining_ms()
4536+
.expect("the pacer is mid-backoff");
4537+
assert!(remaining >= 5_000, "remaining_ms = {remaining}");
4538+
assert_eq!(orch.backend_label(), "S3");
4539+
}
4540+
44614541
#[test]
44624542
fn effective_cap_throttles_only_when_metered_and_throttle_mode() {
44634543
let base = OrchestratorConfig {

crates/driven-core/src/scanner.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2080,6 +2080,51 @@ mod tests {
20802080
assert!(res.new_or_changed.is_empty(), "{:?}", res.new_or_changed);
20812081
}
20822082

2083+
/// issue #308 bottleneck classifier: a deep-verify re-hash credits the
2084+
/// app-global hash counter with the WHOLE file's bytes, whether the
2085+
/// content turns out unchanged (this test) or changed (the mismatch
2086+
/// path re-hashes exactly the same way before it can tell the
2087+
/// difference). `scan_with_priority` is called directly (rather than the
2088+
/// `scan`/`scan_with_progress` wrappers, which always pass `None`) since
2089+
/// only it threads the counters through.
2090+
#[tokio::test]
2091+
async fn deep_verify_credits_the_hash_counter() {
2092+
let dir = tempfile::tempdir().unwrap();
2093+
let root = dir.path();
2094+
let p = root.join("a.txt");
2095+
write(&p, b"hello");
2096+
2097+
let src = source_at(root);
2098+
let state = FakeState::default();
2099+
let (size, mtime) = stat_of(&p);
2100+
state.put(row(
2101+
src.id,
2102+
"a.txt",
2103+
size,
2104+
mtime,
2105+
*blake3::hash(b"hello").as_bytes(),
2106+
));
2107+
2108+
let io = Arc::new(crate::iostat::IoCounters::default());
2109+
let res = scan_with_priority(
2110+
&src,
2111+
&state,
2112+
ScanMode::DeepVerify,
2113+
None,
2114+
None,
2115+
WorkPriority::Normal,
2116+
Some(io.clone()),
2117+
)
2118+
.await
2119+
.unwrap();
2120+
assert!(res.new_or_changed.is_empty(), "{:?}", res.new_or_changed);
2121+
assert_eq!(
2122+
io.snapshot().hashed_bytes,
2123+
size,
2124+
"the whole file was hashed"
2125+
);
2126+
}
2127+
20832128
/// Issue #35 item e: a BUNDLED member - a `file_state` row with
20842129
/// `drive_file_id = NULL` (its bytes live inside a `.tar.gz` bundle), status
20852130
/// Synced, and a matching stored hash - must NOT be re-emitted as changed by a

src-tauri/src/app_state.rs

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2062,6 +2062,63 @@ pub(crate) mod tests {
20622062
let _ = std::fs::remove_dir_all(dir);
20632063
}
20642064

2065+
#[test]
2066+
fn fake_orchestrator_inherits_the_orchestrator_traits_default_bottleneck_methods() {
2067+
// issue #308: `FakeOrchestrator` deliberately does not override
2068+
// `pacer_backoff_remaining_ms` / `backend_label` - it exists to
2069+
// exercise the `Orchestrator` trait's DEFAULT bodies (a pacer-less /
2070+
// label-less trait object must read as clear with a generic label,
2071+
// never panic), the same contract the bottleneck sampler leans on
2072+
// for any account whose orchestrator was built without those seams.
2073+
let orch = FakeOrchestrator::new();
2074+
assert_eq!(orch.pacer_backoff_remaining_ms(), None);
2075+
assert_eq!(orch.backend_label(), "your destination");
2076+
}
2077+
2078+
#[tokio::test]
2079+
async fn bottleneck_runtime_hub_task_and_shutdown_round_trip() {
2080+
// issue #308: the bottleneck sampler's runtime bookkeeping. Unlike
2081+
// `IostatRuntime` there is no "install" step - the hub is always
2082+
// available, defaulting to `NotBackingUp` - so this only needs to
2083+
// cover the getter plus the set/shutdown task pair (mirrors
2084+
// `set_iostat_task`/`shutdown_iostat_task`'s no-orphan drain).
2085+
let (state, dir) = temp_state().await;
2086+
let app_state = AppState::new(
2087+
state,
2088+
HashMap::new(),
2089+
RemoteMode::Fake,
2090+
default_fake_registry(),
2091+
);
2092+
2093+
let hub = app_state.bottleneck_hub();
2094+
assert_eq!(
2095+
hub.latest().state,
2096+
crate::bottleneck_hub::BottleneckState::NotBackingUp
2097+
);
2098+
2099+
// No task registered yet: shutdown is a safe no-op.
2100+
assert!(app_state.shutdown_bottleneck_task().is_none());
2101+
2102+
// Register a task that exits promptly on the shutdown signal (the
2103+
// real sampler's own shape), then confirm shutdown signals + hands
2104+
// back the handle so the quit drain can join it.
2105+
let (shutdown_tx, mut shutdown_rx) = watch::channel(false);
2106+
let task = tokio::spawn(async move {
2107+
let _ = shutdown_rx.changed().await;
2108+
});
2109+
app_state.set_bottleneck_task(task, shutdown_tx);
2110+
2111+
let handle = app_state
2112+
.shutdown_bottleneck_task()
2113+
.expect("the just-registered task round-trips");
2114+
handle.await.unwrap();
2115+
2116+
// Taken: a second shutdown call is again a safe no-op.
2117+
assert!(app_state.shutdown_bottleneck_task().is_none());
2118+
2119+
let _ = std::fs::remove_dir_all(dir);
2120+
}
2121+
20652122
#[tokio::test]
20662123
async fn vss_helper_manager_installs_and_shutdown_is_noop() {
20672124
// Issue #25: AppState owns the least-privilege VSS helper broker manager -

src-tauri/src/assembly.rs

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1397,13 +1397,42 @@ struct SourceProgressEvent {
13971397

13981398
#[cfg(test)]
13991399
mod tests {
1400-
use super::{classify_bridge_event, BridgeAction};
1400+
use super::{bottleneck_backend_label, classify_bridge_event, BridgeAction};
14011401
use driven_core::orchestrator::OrchestratorConfig;
14021402
use driven_core::state::sqlite::SqliteStateRepo;
14031403
use driven_core::state::StateRepo;
14041404
use driven_core::types::{AccountId, ActivityEntry, ExecProgress, OrchestratorEvent, SourceId};
14051405
use tokio::sync::broadcast::error::RecvError;
14061406

1407+
/// issue #308: every `BackendKind` variant maps to a distinct, non-empty
1408+
/// display label for the bottleneck classifier's "X rate-limited"
1409+
/// sub-line, matching the mockup's short-form wording ("Drive", not
1410+
/// "Google Drive").
1411+
#[test]
1412+
fn bottleneck_backend_label_covers_every_backend_kind_distinctly() {
1413+
use driven_remote::BackendKind;
1414+
let labels: Vec<&'static str> = BackendKind::ALL
1415+
.iter()
1416+
.map(|&kind| bottleneck_backend_label(kind))
1417+
.collect();
1418+
assert_eq!(bottleneck_backend_label(BackendKind::GoogleDrive), "Drive");
1419+
assert_eq!(bottleneck_backend_label(BackendKind::S3), "S3");
1420+
assert_eq!(
1421+
bottleneck_backend_label(BackendKind::LocalFolder),
1422+
"your local folder"
1423+
);
1424+
assert_eq!(bottleneck_backend_label(BackendKind::Sftp), "SFTP");
1425+
for label in &labels {
1426+
assert!(!label.is_empty());
1427+
}
1428+
let unique: std::collections::HashSet<_> = labels.iter().collect();
1429+
assert_eq!(
1430+
unique.len(),
1431+
labels.len(),
1432+
"every backend gets its own label"
1433+
);
1434+
}
1435+
14071436
/// M7-P1-1: a broadcast `Lagged` MUST classify as an `ActivityReconcile`
14081437
/// (carrying the dropped count) so the bridge emits `activity:lagged` and the
14091438
/// webview reconciles the dropped rows from the durable `activity_log` -

0 commit comments

Comments
 (0)