Skip to content

Commit fda2b44

Browse files
committed
test(benchmarks-website): adopt Result<()> + ? in tests, drop unwraps
CLAUDE.md asks tests to return `Result<()>` and use `?` instead of `unwrap`. Two test functions and one helper still leaned on `unwrap()`: - `migrate::tests::flush_all_does_not_overcount_on_failure` plus its `open_db_without` helper. - `classifier::tests::random_access_bins_dataset_pattern`. - `read_routes_serve_after_ingest` in `vortex-bench-server/tests/ingest.rs`. The `axum::serve(listener, app).await.unwrap()` calls inside the spawned background-server closures stay — they're inside `tokio::spawn`'s unit-returning future, so `?` cannot propagate, and panicking on a setup failure is the right shape there. `cargo test -p vortex-bench-server -p vortex-bench-migrate` is green; no snapshot rewrites needed. Signed-off-by: Claude <noreply@anthropic.com>
1 parent 573c6b3 commit fda2b44

3 files changed

Lines changed: 21 additions & 10 deletions

File tree

benchmarks-website/migrate/src/classifier.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -793,6 +793,8 @@ fn split_engine_format(series: &str) -> Option<(String, String)> {
793793

794794
#[cfg(test)]
795795
mod tests {
796+
use anyhow::Context as _;
797+
796798
use super::*;
797799

798800
fn record(name: &str) -> V2Record {
@@ -836,14 +838,16 @@ mod tests {
836838
}
837839

838840
#[test]
839-
fn random_access_bins_dataset_pattern() {
840-
let bin = classify(&record("random-access/taxi/take/parquet")).unwrap();
841+
fn random_access_bins_dataset_pattern() -> anyhow::Result<()> {
842+
let bin = classify(&record("random-access/taxi/take/parquet"))
843+
.context("classify returned None for a known-good 4-part random-access name")?;
841844
assert_eq!(
842845
bin,
843846
V3Bin::RandomAccess {
844847
dataset: "taxi/take".into(),
845848
format: "parquet".into(),
846849
}
847850
);
851+
Ok(())
848852
}
849853
}

benchmarks-website/migrate/src/migrate.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -869,12 +869,12 @@ mod tests {
869869

870870
use super::*;
871871

872-
fn open_db_without(table: &str) -> (tempfile::TempDir, Connection) {
873-
let dir = tempfile::TempDir::new().unwrap();
872+
fn open_db_without(table: &str) -> Result<(tempfile::TempDir, Connection)> {
873+
let dir = tempfile::TempDir::new()?;
874874
let path = dir.path().join("v3.duckdb");
875-
let conn = open_target_db(&path).unwrap();
876-
conn.execute_batch(&format!("DROP TABLE {table}")).unwrap();
877-
(dir, conn)
875+
let conn = open_target_db(&path)?;
876+
conn.execute_batch(&format!("DROP TABLE {table}"))?;
877+
Ok((dir, conn))
878878
}
879879

880880
fn one_query_row() -> QueryMeasurement {
@@ -898,12 +898,12 @@ mod tests {
898898
}
899899

900900
#[test]
901-
fn flush_all_does_not_overcount_on_failure() {
901+
fn flush_all_does_not_overcount_on_failure() -> Result<()> {
902902
// Drop `compression_times` before flushing so the second
903903
// flush in `flush_all` fails. The first (queries) succeeded,
904904
// so its counter must be set; the failed table's counter and
905905
// every later table's counter must stay at zero.
906-
let (_dir, conn) = open_db_without("compression_times");
906+
let (_dir, conn) = open_db_without("compression_times")?;
907907

908908
let mut summary = MigrationSummary::default();
909909
let mut q = QueryAccum::default();
@@ -931,5 +931,6 @@ mod tests {
931931
summary.compression_size_inserted, 0,
932932
"later flushes never ran"
933933
);
934+
Ok(())
934935
}
935936
}

benchmarks-website/server/tests/ingest.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,13 @@ async fn read_routes_serve_after_ingest() -> Result<()> {
283283
assert!(body["display_name"].is_string());
284284
assert!(body["unit"].is_string());
285285
assert!(body["commits"].is_array());
286-
assert_eq!(body["commits"].as_array().unwrap().len(), 1);
286+
assert_eq!(
287+
body["commits"]
288+
.as_array()
289+
.context("commits is array")?
290+
.len(),
291+
1
292+
);
287293
assert!(body["series"].is_object());
288294
Ok(())
289295
}

0 commit comments

Comments
 (0)