From df9e8826a17e24b1f31e155189da9c4822067e80 Mon Sep 17 00:00:00 2001 From: StudentWeis Date: Sat, 24 Oct 2026 18:01:40 +0800 Subject: [PATCH] fix(core): propagate history count failures Refs #308 --- crates/ropy-core/src/repository/backend.rs | 12 +- .../src/repository/backend/memory.rs | 4 +- .../ropy-core/src/repository/backend/redb.rs | 23 ++- crates/ropy-core/src/repository/cleanup.rs | 54 ++++-- crates/ropy-core/src/repository/repo.rs | 5 +- .../src/repository/tests/dedup_tests.rs | 34 ++-- .../src/repository/tests/display_tests.rs | 20 +-- .../src/repository/tests/pin_tests.rs | 6 +- .../src/repository/tests/recovery_tests.rs | 38 ++++- .../src/repository/tests/save_tests.rs | 12 +- crates/ropy-core/src/repository/time_index.rs | 12 +- src/benchmark.rs | 8 +- src/clipboard/listener.rs | 16 +- src/gui/board/ui_tests.rs | 22 ++- src/promo/tests.rs | 2 +- src/updater/checker.rs | 159 ++++-------------- 16 files changed, 211 insertions(+), 216 deletions(-) diff --git a/crates/ropy-core/src/repository/backend.rs b/crates/ropy-core/src/repository/backend.rs index 28bd5c3..9430300 100644 --- a/crates/ropy-core/src/repository/backend.rs +++ b/crates/ropy-core/src/repository/backend.rs @@ -71,11 +71,17 @@ pub trait KvTree: Send + Sync { fn remove(&self, key: &[u8]) -> Result; /// Return the number of entries currently visible in the tree. - fn len(&self) -> usize; + /// + /// # Errors + /// Returns an error if the backend cannot read the entry count. + fn len(&self) -> Result; /// Whether this tree currently contains no entries. - fn is_empty(&self) -> bool { - self.len() == 0 + /// + /// # Errors + /// Returns an error if the backend cannot read the entry count. + fn is_empty(&self) -> Result { + self.len().map(|count| count == 0) } /// Remove all entries from this test tree. diff --git a/crates/ropy-core/src/repository/backend/memory.rs b/crates/ropy-core/src/repository/backend/memory.rs index dfdad4e..4db9766 100644 --- a/crates/ropy-core/src/repository/backend/memory.rs +++ b/crates/ropy-core/src/repository/backend/memory.rs @@ -180,10 +180,10 @@ impl KvTree for MemoryTreeHandle { Ok(entries.remove(key).is_some()) } - fn len(&self) -> usize { + fn len(&self) -> Result { let entries_lock = self.tree.entries.read(); let entries = recover_lock(entries_lock); - entries.len() + Ok(entries.len()) } #[cfg(test)] diff --git a/crates/ropy-core/src/repository/backend/redb.rs b/crates/ropy-core/src/repository/backend/redb.rs index 802794b..7dcc28d 100644 --- a/crates/ropy-core/src/repository/backend/redb.rs +++ b/crates/ropy-core/src/repository/backend/redb.rs @@ -176,6 +176,8 @@ pub struct RedbTree { db: Arc, name: String, write_lock: Mutex<()>, + #[cfg(test)] + fail_next_len: std::sync::atomic::AtomicBool, } impl RedbTree { @@ -199,9 +201,17 @@ impl RedbTree { db, name, write_lock: Mutex::new(()), + #[cfg(test)] + fail_next_len: std::sync::atomic::AtomicBool::new(false), }) } + #[cfg(test)] + pub(crate) fn fail_next_len(&self) { + self.fail_next_len + .store(true, std::sync::atomic::Ordering::SeqCst); + } + const fn table_definition(name: &str) -> RedbTableDefinition<'_> { TableDefinition::new(name) } @@ -281,17 +291,20 @@ impl KvTree for RedbTree { Ok(removed) } - fn len(&self) -> usize { + fn len(&self) -> Result { self.with_read_table(|table| { + #[cfg(test)] + if self + .fail_next_len + .swap(false, std::sync::atomic::Ordering::SeqCst) + { + return Err(RepositoryError::Query("injected count read failure".into())); + } table .len() .map(|len| usize::try_from(len).unwrap_or(usize::MAX)) .map_err(|error| RepositoryError::Query(error.to_string())) }) - .unwrap_or_else(|error| { - tracing::warn!(error = %error, tree = %self.name, "failed to read tree length"); - 0 - }) } #[cfg(test)] diff --git a/crates/ropy-core/src/repository/cleanup.rs b/crates/ropy-core/src/repository/cleanup.rs index 04faba6..af8853f 100644 --- a/crates/ropy-core/src/repository/cleanup.rs +++ b/crates/ropy-core/src/repository/cleanup.rs @@ -27,7 +27,7 @@ impl ClipboardRepository { /// Returns an error if selecting or deleting ordinary records fails. pub fn clear_ordinary_records(&self) -> Result { let _operation = self.lock_operation(); - let total = self.count(); + let total = self.count()?; let favorite_ids = self.favorite_id_set()?; let ordinary_total = self.ordinary_record_count(total, &favorite_ids)?; @@ -41,7 +41,7 @@ impl ClipboardRepository { /// Returns an error if querying retention exemptions or deleting old records fails. pub fn cleanup_old_records(&self, keep_count: usize) -> Result { let _operation = self.lock_operation(); - let total = self.count(); + let total = self.count()?; let favorite_ids = self.favorite_id_set()?; let ordinary_total = self.ordinary_record_count(total, &favorite_ids)?; self.cleanup_old_records_with_ordinary_total( @@ -64,7 +64,7 @@ impl ClipboardRepository { keep_count: usize, ) -> Result { let _operation = self.lock_operation(); - let total = self.count(); + let total = self.count()?; let trigger = ClipboardRepository::::cleanup_trigger_record_count(keep_count); // Ordinary records cannot outnumber all records. Avoid reading retention // exemptions until even the total exceeds the buffered limit. @@ -181,6 +181,40 @@ mod tests { use super::*; use crate::repository::test_helpers::{create_test_repo, save_numbered_records, save_text_at}; + #[rstest] + #[case::clear(0)] + #[case::trim(1)] + #[case::buffered_trim(2)] + fn test_cleanup_count_failure_preserves_history_and_allows_retry(#[case] operation: u8) { + let (_directory, repo) = crate::repository::test_helpers::create_test_repo_with( + crate::repository::backend::redb::redb_backend_factory, + ); + let record = repo + .save_rich_text("keep".into(), Some("keep"), None) + .unwrap(); + let cleanup = || match operation { + 0 => repo.clear_ordinary_records(), + 1 => repo.cleanup_old_records(0), + _ => repo.cleanup_old_records_if_needed(0), + }; + // Two records exceed the buffered limit, so a successful retry must delete both. + repo.save_text("other".into()).unwrap(); + repo.records.fail_next_len(); + assert!(matches!(cleanup(), Err(RepositoryError::Query(_)))); + assert_eq!(repo.get_display_records(10).unwrap().len(), 2); + assert_eq!( + crate::repository::assets::load_rich_text_html(record.rich_text_meta.as_ref().unwrap()) + .as_deref(), + Some("keep"), + ); + assert_eq!(cleanup().unwrap(), 2); + assert_eq!(repo.get_display_records(10).unwrap(), []); + assert!( + crate::repository::assets::load_rich_text_html(record.rich_text_meta.as_ref().unwrap()) + .is_none() + ); + } + #[rstest] #[case(0, 1)] #[case(1, 1)] @@ -235,7 +269,7 @@ mod tests { let removed = repo.cleanup_old_records(1).expect("Failed to cleanup"); assert_eq!(removed, 0); - assert_eq!(repo.count(), 3); + assert_eq!(repo.count().expect("count"), 3); } #[test] @@ -253,7 +287,7 @@ mod tests { let removed = repo.cleanup_old_records(2).expect("Failed to cleanup"); assert_eq!(removed, 3); - assert_eq!(repo.count(), 4); + assert_eq!(repo.count().expect("count"), 4); assert!(repo.get_by_id(pinned.id).expect("query").is_some()); assert!(repo.get_by_id(favorited.id).expect("query").is_some()); } @@ -283,7 +317,7 @@ mod tests { .expect("Failed to cleanup"); assert_eq!(removed, 0); - assert_eq!(repo.count(), 11); + assert_eq!(repo.count().expect("count"), 11); } #[test] @@ -298,7 +332,7 @@ mod tests { .expect("Failed to cleanup"); assert_eq!(removed, 2); - assert_eq!(repo.count(), 10); + assert_eq!(repo.count().expect("count"), 10); } #[test] @@ -320,7 +354,7 @@ mod tests { .expect("Failed to cleanup"); assert_eq!(removed, 0); - assert_eq!(repo.count(), 12); + assert_eq!(repo.count().expect("count"), 12); } #[test] @@ -330,7 +364,7 @@ mod tests { let removed = repo.clear_ordinary_records().expect("Failed to clear"); assert_eq!(removed, 0); - assert_eq!(repo.count(), 0); + assert_eq!(repo.count().expect("count"), 0); } #[test] @@ -341,6 +375,6 @@ mod tests { let removed = repo.clear_ordinary_records().expect("Failed to clear"); assert_eq!(removed, 5); - assert_eq!(repo.count(), 0); + assert_eq!(repo.count().expect("count"), 0); } } diff --git a/crates/ropy-core/src/repository/repo.rs b/crates/ropy-core/src/repository/repo.rs index 675f889..0dc059f 100644 --- a/crates/ropy-core/src/repository/repo.rs +++ b/crates/ropy-core/src/repository/repo.rs @@ -462,7 +462,10 @@ impl ClipboardRepository { } /// Return the number of stored records. - pub fn count(&self) -> usize { + /// + /// # Errors + /// Returns an error if the backend cannot read the record count. + pub fn count(&self) -> Result { self.records.len() } } diff --git a/crates/ropy-core/src/repository/tests/dedup_tests.rs b/crates/ropy-core/src/repository/tests/dedup_tests.rs index f04d925..bb449ca 100644 --- a/crates/ropy-core/src/repository/tests/dedup_tests.rs +++ b/crates/ropy-core/src/repository/tests/dedup_tests.rs @@ -36,7 +36,7 @@ fn test_dedup_same_content() { let r2 = save_text_at(&repo, "duplicate", 2); assert_eq!(r1.id, r2.id); - assert_eq!(repo.count(), 1); + assert_eq!(repo.count().expect("count"), 1); assert!(r2.created_at > r1.created_at); } @@ -48,7 +48,7 @@ fn test_dedup_aba_pattern() { save_text_at(&repo, "B", 2); let a2 = save_text_at(&repo, "A", 3); - assert_eq!(repo.count(), 2); + assert_eq!(repo.count().expect("count"), 2); let recent = load_display_records(&repo, 2); assert_eq!(recent[0].content, "A"); assert_eq!(recent[0].created_at, a2.created_at); @@ -74,13 +74,13 @@ fn assert_cleanup_old_records_with(factory: BackendFactory let (_temp_dir, repo) = create_test_repo_with(factory); save_numbered_records(&repo, 10); - assert_eq!(repo.count(), 10); + assert_eq!(repo.count().expect("count"), 10); let removed = repo .cleanup_old_records(5) .unwrap_or_else(|error| panic!("Failed to clean up: {error}")); assert_eq!(removed, 5); - assert_eq!(repo.count(), 5); + assert_eq!(repo.count().expect("count"), 5); let recent = load_display_records(&repo, 5); assert_eq!(recent[0].content, "Record 10"); @@ -105,7 +105,7 @@ fn test_cleanup_old_records_zero_keep() { let removed = repo.cleanup_old_records(0).expect("Failed to clean up"); assert_eq!(removed, 1); - assert_eq!(repo.count(), 0); + assert_eq!(repo.count().expect("count"), 0); } #[test] @@ -116,7 +116,7 @@ fn test_cleanup_old_records_greater_than_total() { let removed = repo.cleanup_old_records(100).expect("Failed to clean up"); assert_eq!(removed, 0); - assert_eq!(repo.count(), 1); + assert_eq!(repo.count().expect("count"), 1); } #[test] @@ -127,7 +127,7 @@ fn test_cleanup_old_records_equal_to_total() { let removed = repo.cleanup_old_records(1).expect("Failed to clean up"); assert_eq!(removed, 0); - assert_eq!(repo.count(), 1); + assert_eq!(repo.count().expect("count"), 1); } #[rstest] @@ -144,7 +144,7 @@ fn test_cleanup_old_records_if_needed_at_threshold_preserves_records(#[case] kee .unwrap_or_else(|err| panic!("Failed to conditionally clean up: {err}")); assert_eq!(removed, 0); - assert_eq!(repo.count(), threshold); + assert_eq!(repo.count().expect("count"), threshold); } #[rstest] @@ -163,7 +163,7 @@ fn test_cleanup_old_records_if_needed_above_threshold_trims_to_keep_count( .unwrap_or_else(|err| panic!("Failed to conditionally clean up: {err}")); assert_eq!(removed, threshold + 1 - keep_count); - assert_eq!(repo.count(), keep_count); + assert_eq!(repo.count().expect("count"), keep_count); let recent = repo .get_display_records(keep_count) @@ -186,7 +186,7 @@ fn test_cleanup_old_records_when_favorited_record_present_counts_only_ordinary_r let removed = repo.cleanup_old_records(3).expect("Failed to clean up"); assert_eq!(removed, 2); - assert_eq!(repo.count(), 4); + assert_eq!(repo.count().expect("count"), 4); assert!( repo.get_by_id(favorite.id) .expect("Failed to get by id") @@ -253,11 +253,11 @@ fn test_delete() { let record = repo .save_text("To be deleted".to_string()) .expect("Failed to save"); - assert_eq!(repo.count(), 1); + assert_eq!(repo.count().expect("count"), 1); let deleted = repo.delete(record.id).expect("Failed to delete"); assert!(deleted); - assert_eq!(repo.count(), 0); + assert_eq!(repo.count().expect("count"), 0); let deleted_again = repo.delete(record.id).expect("Failed to delete"); assert!(!deleted_again); @@ -353,10 +353,10 @@ fn test_clear() { repo.save_text("One".to_string()).expect("Failed to save"); repo.save_text("Two".to_string()).expect("Failed to save"); repo.save_text("Three".to_string()).expect("Failed to save"); - assert_eq!(repo.count(), 3); + assert_eq!(repo.count().expect("count"), 3); repo.clear().expect("Failed to clear"); - assert_eq!(repo.count(), 0); + assert_eq!(repo.count().expect("count"), 0); } #[test] @@ -407,7 +407,7 @@ fn test_clear_ordinary_records_when_special_records_present_preserves_them() { .expect("Failed to clear ordinary records"); assert_eq!(removed, 2); - assert_eq!(repo.count(), 3); + assert_eq!(repo.count().expect("count"), 3); assert!( repo.get_by_id(pinned.id) .expect("Failed to load pinned") @@ -455,7 +455,7 @@ fn test_clear_ordinary_records_when_no_ordinary_records_returns_zero() { .expect("Failed to clear ordinary records"); assert_eq!(removed, 0); - assert_eq!(repo.count(), 2); + assert_eq!(repo.count().expect("count"), 2); assert!( repo.get_by_id(pinned.id) .expect("Failed to load pinned") @@ -473,5 +473,5 @@ fn test_clear_empty_repo() { let (_temp_dir, repo) = create_test_repo(); repo.clear().expect("Failed to clear empty repo"); - assert_eq!(repo.count(), 0); + assert_eq!(repo.count().expect("count"), 0); } diff --git a/crates/ropy-core/src/repository/tests/display_tests.rs b/crates/ropy-core/src/repository/tests/display_tests.rs index be693dd..8920c7f 100644 --- a/crates/ropy-core/src/repository/tests/display_tests.rs +++ b/crates/ropy-core/src/repository/tests/display_tests.rs @@ -35,7 +35,7 @@ fn test_same_millisecond_limit_and_cleanup_keep_newest(#[case] cleanup: bool) { let repo = ClipboardRepository::from_backend(backend, images).unwrap(); if cleanup { assert_eq!(repo.cleanup_old_records(1).unwrap(), 1); - assert_eq!(repo.count(), 1); + assert_eq!(repo.count().expect("count"), 1); assert!(repo.get_by_id(10).unwrap().is_none()); } let selected = repo.get_display_records(1).unwrap(); @@ -161,7 +161,7 @@ fn test_get_display_records_when_total_is_41_with_two_favorites_and_one_pinned_r .get_display_records(40) .expect("Failed to get display records"); - assert_eq!(repo.count(), 41); + assert_eq!(repo.count().expect("count"), 41); assert_eq!(display_records.len(), 41); } @@ -187,7 +187,7 @@ fn test_get_display_records_when_limit_is_40_with_two_favorites_and_one_pinned_r .get_display_records(40) .expect("Failed to get display records"); - assert_eq!(repo.count(), 43); + assert_eq!(repo.count().expect("count"), 43); assert_eq!(display_records.len(), 43); } @@ -228,7 +228,7 @@ fn test_get_display_records_when_newest_records_are_favorited_still_returns_43() .get_display_records(40) .expect("Failed to get display records"); - assert_eq!(repo.count(), 43); + assert_eq!(repo.count().expect("count"), 43); assert_eq!(display_records.len(), 43); } @@ -310,7 +310,7 @@ fn test_get_by_id_u64_max() { fn test_count_empty() { let (_temp_dir, repo) = create_test_repo(); - assert_eq!(repo.count(), 0); + assert_eq!(repo.count().expect("count"), 0); } #[test] @@ -358,13 +358,13 @@ fn test_multiple_operations_sequence() { let r2 = save_text_at(&repo, "Second", 2); let _r3 = save_text_at(&repo, "Third", 3); - assert_eq!(repo.count(), 3); + assert_eq!(repo.count().expect("count"), 3); repo.toggle_pin(r1.id).expect("Failed to pin"); let deleted = repo.delete(r2.id).expect("Failed to delete"); assert!(deleted); - assert_eq!(repo.count(), 2); + assert_eq!(repo.count().expect("count"), 2); let recent = load_display_records(&repo, 10); assert_eq!(recent.len(), 2); @@ -372,7 +372,7 @@ fn test_multiple_operations_sequence() { assert_eq!(recent[1].content, "Third"); repo.clear().expect("Failed to clear"); - assert_eq!(repo.count(), 0); + assert_eq!(repo.count().expect("count"), 0); } #[test] @@ -394,7 +394,7 @@ fn test_concurrent_saves_then_deletes_remove_selected_records() { ids.push(record.id); } - assert_eq!(repo.count(), 10); + assert_eq!(repo.count().expect("count"), 10); let repo2 = repo.clone(); let ids_to_delete = ids[..5].to_vec(); @@ -405,5 +405,5 @@ fn test_concurrent_saves_then_deletes_remove_selected_records() { }); handle.join().expect("Delete thread panicked"); - assert_eq!(repo.count(), 5); + assert_eq!(repo.count().expect("count"), 5); } diff --git a/crates/ropy-core/src/repository/tests/pin_tests.rs b/crates/ropy-core/src/repository/tests/pin_tests.rs index 76c150e..9a664d5 100644 --- a/crates/ropy-core/src/repository/tests/pin_tests.rs +++ b/crates/ropy-core/src/repository/tests/pin_tests.rs @@ -46,11 +46,11 @@ fn test_cleanup_skips_pinned() { } repo.toggle_pin(r1.id).expect("Failed to toggle pin"); - assert_eq!(repo.count(), 6); + assert_eq!(repo.count().expect("count"), 6); let removed = repo.cleanup_old_records(3).expect("Failed to clean up"); assert_eq!(removed, 2); - assert_eq!(repo.count(), 4); + assert_eq!(repo.count().expect("count"), 4); let pinned = repo .get_by_id(r1.id) .expect("Failed to get") @@ -73,7 +73,7 @@ fn test_cleanup_keeps_pinned_when_not_enough_unpinned() { let removed = repo.cleanup_old_records(2).expect("Failed to clean up"); assert_eq!(removed, 0); - assert_eq!(repo.count(), 4); + assert_eq!(repo.count().expect("count"), 4); } #[test] diff --git a/crates/ropy-core/src/repository/tests/recovery_tests.rs b/crates/ropy-core/src/repository/tests/recovery_tests.rs index b5496dc..ada6b4d 100644 --- a/crates/ropy-core/src/repository/tests/recovery_tests.rs +++ b/crates/ropy-core/src/repository/tests/recovery_tests.rs @@ -26,7 +26,7 @@ fn test_display_record_read_failure_returns_error_and_preserves_history() { assert_eq!(records[0].id, record.id); assert!(records[0].pinned); assert!(favorites.is_empty()); - assert_eq!(repo.count(), 1); + assert_eq!(repo.count().expect("count"), 1); } #[rstest::rstest] @@ -64,7 +64,7 @@ fn test_display_corrupt_record_fills_limit_with_readable_history(#[case] reopen: [pinned.id, good.id, favorite.id] ); assert_eq!(favorites, std::collections::HashSet::from([favorite.id])); - assert_eq!(repo.count(), 4); + assert_eq!(repo.count().expect("count"), 4); } #[test] @@ -157,9 +157,16 @@ fn test_redb_new_record_aborted_write_leaves_no_record_or_index(#[case] fail_aft let (dir, repo) = crate::repository::test_helpers::create_test_repo_with(redb_backend_factory); repo.backend.fail_write_after(fail_after); assert!(repo.save_text("new".into()).is_err()); - assert_eq!(repo.count(), 0); + assert_eq!(repo.count().expect("count"), 0); for tree in [TIME_INDEX_TREE, TIME_INDEX_LOOKUP_TREE] { - assert_eq!(repo.backend.open_tree(tree).expect("tree").len(), 0); + assert_eq!( + repo.backend + .open_tree(tree) + .expect("tree") + .len() + .expect("count"), + 0 + ); } drop(repo); let reopened = ClipboardRepository::init( @@ -168,7 +175,7 @@ fn test_redb_new_record_aborted_write_leaves_no_record_or_index(#[case] fail_aft redb_backend_factory, ) .expect("reopen"); - assert_eq!(reopened.count(), 0); + assert_eq!(reopened.count().expect("count"), 0); } #[test] @@ -210,10 +217,17 @@ fn test_repository_concurrent_recopy_pin_delete_keeps_indexes_consistent() { barrier.wait(); repo.delete(record.id).expect("delete"); saver.join().expect("saver"); - let count = repo.count(); + let count = repo.count().expect("count"); assert_eq!(repo.get_display_records(10).expect("display").len(), count); for tree in [TIME_INDEX_TREE, TIME_INDEX_LOOKUP_TREE] { - assert_eq!(repo.backend.open_tree(tree).expect("tree").len(), count); + assert_eq!( + repo.backend + .open_tree(tree) + .expect("tree") + .len() + .expect("count"), + count + ); } repo.delete(record.id).expect("reset fixture"); } @@ -390,6 +404,14 @@ fn test_redb_reopen_inconsistent_indexes_repairs_without_losing_pin_or_favorite( assert!(display[0].pinned); assert_eq!(reopened.favorite_ids().expect("favorites"), vec![record.id]); for tree in [TIME_INDEX_TREE, TIME_INDEX_LOOKUP_TREE] { - assert_eq!(reopened.backend.open_tree(tree).expect("tree").len(), 1); + assert_eq!( + reopened + .backend + .open_tree(tree) + .expect("tree") + .len() + .expect("count"), + 1 + ); } } diff --git a/crates/ropy-core/src/repository/tests/save_tests.rs b/crates/ropy-core/src/repository/tests/save_tests.rs index 31c744c..da95206 100644 --- a/crates/ropy-core/src/repository/tests/save_tests.rs +++ b/crates/ropy-core/src/repository/tests/save_tests.rs @@ -47,7 +47,7 @@ fn test_pending_image_partial_install_failure_removes_new_payload() { let thumbnail = repo.images_dir().join("42_thumb.png"); std::fs::create_dir(&thumbnail).unwrap(); assert!(repo.save_pending_image(image).is_err()); - assert_eq!(repo.count(), 0); + assert_eq!(repo.count().expect("count"), 0); assert!(!repo.images_dir().join("42.png").exists()); std::fs::remove_dir(thumbnail).unwrap(); assert_eq!(std::fs::read_dir(repo.images_dir()).unwrap().count(), 0); @@ -72,7 +72,7 @@ fn test_save_rich_text_sidecar_write_failure_preserves_committed_record(#[case] repo.save_rich_text("same".into(), Some("new"), None) .is_err() ); - assert_eq!(repo.count(), usize::from(existing)); + assert_eq!(repo.count().expect("count"), usize::from(existing)); std::fs::remove_file(&sidecars).unwrap(); if let Some(original) = original { std::fs::rename(backup, sidecars).unwrap(); @@ -223,7 +223,7 @@ fn test_save_different_content_types() { .expect("Failed to save path"); assert_eq!(path_record.content_type, ContentType::FilePath); - assert_eq!(repo.count(), 2); + assert_eq!(repo.count().expect("count"), 2); } #[test] @@ -256,7 +256,7 @@ fn test_repository_save_files_when_equivalent_inputs_reuses_existing_record() { .expect("Failed to save second file record"); assert_eq!(first.id, second.id); - assert_eq!(repo.count(), 1); + assert_eq!(repo.count().expect("count"), 1); } #[test] @@ -307,7 +307,7 @@ fn test_save_image_from_path_dedup() { .expect("Failed to save second"); assert_eq!(r1.id, r2.id); - assert_eq!(repo.count(), 1); + assert_eq!(repo.count().expect("count"), 1); assert_eq!(r2.created_at, capture_time(2)); assert_eq!(r2.content, r1.content); assert!(path1.exists()); @@ -327,7 +327,7 @@ fn test_save_after_clear() { let record = repo .save_text("After clear".to_string()) .expect("Failed to save after clear"); - assert_eq!(repo.count(), 1); + assert_eq!(repo.count().expect("count"), 1); assert_eq!(record.content, "After clear"); } diff --git a/crates/ropy-core/src/repository/time_index.rs b/crates/ropy-core/src/repository/time_index.rs index f4f5ee6..f268a1b 100644 --- a/crates/ropy-core/src/repository/time_index.rs +++ b/crates/ropy-core/src/repository/time_index.rs @@ -462,13 +462,13 @@ mod tests { index.insert_raw(2000, 2, true, &ContentType::Image); index.insert_raw(3000, 3, false, &ContentType::FilePath); - assert_eq!(index.entries.len(), 3); - assert_eq!(index.id_lookup.len(), 3); + assert_eq!(index.entries.len().expect("count"), 3); + assert_eq!(index.id_lookup.len().expect("count"), 3); index.clear().expect("Failed to clear"); - assert_eq!(index.entries.len(), 0); - assert_eq!(index.id_lookup.len(), 0); + assert_eq!(index.entries.len().expect("count"), 0); + assert_eq!(index.id_lookup.len().expect("count"), 0); let ids = select_display_ids_without_favorites(&index, 10); assert_eq!(ids, Vec::::new()); } @@ -743,8 +743,8 @@ mod tests { let key = MemoryTimeIndex::encode_key(timestamp, 1); index.remove_raw(&key).expect("Failed to remove raw"); - assert_eq!(index.entries.len(), 0); - assert_eq!(index.id_lookup.len(), 0); + assert_eq!(index.entries.len().expect("count"), 0); + assert_eq!(index.id_lookup.len().expect("count"), 0); let ids = select_display_ids_without_favorites(&index, 10); assert_eq!(ids, Vec::::new()); } diff --git a/src/benchmark.rs b/src/benchmark.rs index 991bf49..8233fed 100644 --- a/src/benchmark.rs +++ b/src/benchmark.rs @@ -51,7 +51,7 @@ pub(crate) fn run(ready: PathBuf) -> Result<(), BenchmarkError> { let records = repository.get_display_records(100)?; let readiness = format!( "{{\"stored_records\":{},\"loaded_records\":{}}}", - repository.count(), + repository.count()?, records.len() ); let repository = Arc::new(repository); @@ -109,14 +109,14 @@ mod tests { ) .unwrap(); fixture::seed(&repository).unwrap(); - assert_eq!(repository.count(), 200); + assert_eq!(repository.count().unwrap(), 200); assert_eq!(repository.get_display_records(100).unwrap().len(), 100); for index in 0..200 { assert_eq!(fixture::text(index).len(), 1024); } repository.save_text(fixture::text(0)).unwrap(); - assert_eq!(repository.count(), 200); + assert_eq!(repository.count().unwrap(), 200); repository.save_text(fixture::text(200)).unwrap(); - assert_eq!(repository.count(), 201); + assert_eq!(repository.count().unwrap(), 201); } } diff --git a/src/clipboard/listener.rs b/src/clipboard/listener.rs index 6265667..6d44334 100644 --- a/src/clipboard/listener.rs +++ b/src/clipboard/listener.rs @@ -488,7 +488,7 @@ mod tests { .persist(&fixture.repo) .expect("discard stale image"); assert_eq!( - fixture.repo.count(), + fixture.repo.count().expect("count"), 0, "old captures must not restore history" ); @@ -506,7 +506,7 @@ mod tests { .expect("new capture") .persist(&fixture.repo) .expect("new copy"); - assert_eq!(fixture.repo.count(), 1); + assert_eq!(fixture.repo.count().expect("count"), 1); } #[rstest] @@ -535,7 +535,7 @@ mod tests { .persist(&fixture.repo) .expect("persist unrelated capture"); assert!(fixture.repo.get_by_id(record.id).expect("lookup").is_none()); - assert_eq!(fixture.repo.count(), 1); + assert_eq!(fixture.repo.count().expect("count"), 1); fixture.text("deleted"); fixture .rx @@ -543,7 +543,7 @@ mod tests { .expect("recaptured") .persist(&fixture.repo) .expect("new copy"); - assert_eq!(fixture.repo.count(), 2); + assert_eq!(fixture.repo.count().expect("count"), 2); } #[test] @@ -554,7 +554,7 @@ mod tests { fixture.backend.fail_next_batch(); assert!(super::super::clear_tracked_history(&fixture.repo, &fixture.tracker).is_err()); assert!(capture.persist(&fixture.repo).expect("persist").is_some()); - assert_eq!(fixture.repo.count(), 1); + assert_eq!(fixture.repo.count().expect("count"), 1); fixture.text("kept after failure"); assert!( fixture.rx.is_empty(), @@ -620,7 +620,7 @@ mod tests { .expect("recaptured") .persist(&fixture.repo) .expect("persist retry"); - assert_eq!(fixture.repo.count(), 1); + assert_eq!(fixture.repo.count().expect("count"), 1); } #[test] @@ -643,7 +643,7 @@ mod tests { .expect("retry") .persist(&fixture.repo) .expect("persist retry"); - assert_eq!(fixture.repo.count(), 1); + assert_eq!(fixture.repo.count().expect("count"), 1); } #[test] @@ -717,7 +717,7 @@ mod tests { .expect("encode image"); fixture.backend.fail_next_batch(); assert!(capture.persist(&fixture.repo).is_err()); - assert_eq!(fixture.repo.count(), 0); + assert_eq!(fixture.repo.count().expect("count"), 0); assert_eq!( std::fs::read_dir(fixture.repo.images_dir()) .expect("read images directory") diff --git a/src/gui/board/ui_tests.rs b/src/gui/board/ui_tests.rs index d9bb322..6f92f83 100644 --- a/src/gui/board/ui_tests.rs +++ b/src/gui/board/ui_tests.rs @@ -147,7 +147,11 @@ fn test_history_clear_confirmation_reports_failure_and_retry_clears(cx: &mut Tes }); window.render_frame(cx); window.click("ok", cx); - assert_eq!(repo.count(), 1, "clear must not run on the event thread"); + assert_eq!( + repo.count().unwrap(), + 1, + "clear must not run on the event thread" + ); assert!(window.find("history-operation-pending").visible()); let id = board.read(cx).filtered_record_id_at(0).unwrap(); assert_eq!(window.find(("delete-btn", id)).disabled(), Some(true)); @@ -156,7 +160,7 @@ fn test_history_clear_confirmation_reports_failure_and_retry_clears(cx: &mut Tes }) .unwrap(); cx.run_until_parked(); - assert_eq!(repo.count(), 0); + assert_eq!(repo.count().unwrap(), 0); cx.update_window(handle, |_, window, cx| { window.render_frame(cx); assert_eq!(board.read(cx).filtered_record_len(), 0); @@ -1343,7 +1347,7 @@ fn test_refresh_notification_preserves_pending_storage_limit_trim(cx: &TestAppCo }); cx.run_until_parked(); assert_eq!( - repo.count(), + repo.count().unwrap(), 10, "notification must not downgrade a requested immediate trim to buffered cleanup" ); @@ -1366,7 +1370,7 @@ fn test_refresh_settings_recovery_preserves_history_until_repaired(cx: &TestAppC }); cx.run_until_parked(); assert_eq!( - repo.count(), + repo.count().unwrap(), 231, "recovery defaults must not delete history" ); @@ -1376,7 +1380,7 @@ fn test_refresh_settings_recovery_preserves_history_until_repaired(cx: &TestAppC }); cx.run_until_parked(); assert_eq!( - repo.count(), + repo.count().unwrap(), 231, "explicit trims must also respect recovery" ); @@ -1385,7 +1389,11 @@ fn test_refresh_settings_recovery_preserves_history_until_repaired(cx: &TestAppC board.update(cx, |board, cx| board.refresh_records_from_repository(cx)); }); cx.run_until_parked(); - assert_eq!(repo.count(), 200, "valid settings restore normal retention"); + assert_eq!( + repo.count().unwrap(), + 200, + "valid settings restore normal retention" + ); } #[gpui_kit::test] @@ -1456,7 +1464,7 @@ fn test_refresh_clear_while_pending_does_not_restore_deleted_history(cx: &TestAp }); cx.run_until_parked(); cx.update(|cx| assert!(crate::utils::read_or_recover(&board.read(cx).records).is_empty())); - assert_eq!(repo.count(), 0); + assert_eq!(repo.count().unwrap(), 0); } #[gpui_kit::test] diff --git a/src/promo/tests.rs b/src/promo/tests.rs index 1fe1056..a30f079 100644 --- a/src/promo/tests.rs +++ b/src/promo/tests.rs @@ -78,7 +78,7 @@ fn test_promo_input_shield_prevents_settings_and_copy(cx: &mut TestAppContext) { assert!(board.copy_tx.is_closed()); let repository = GlobalRepository::global(cx).cloned().unwrap(); assert_eq!(repository.images_dir(), fixture.path().join("images")); - assert_eq!(repository.count(), 0); + assert_eq!(repository.count().unwrap(), 0); assert_eq!(crate::utils::read_or_recover(&board.records).len(), 4); assert_eq!(board.layout_mode, LayoutMode::List); assert!(board.pinned); diff --git a/src/updater/checker.rs b/src/updater/checker.rs index 39e86eb..a36b8e7 100644 --- a/src/updater/checker.rs +++ b/src/updater/checker.rs @@ -260,27 +260,6 @@ mod tests { ); } - #[test] - #[expect(clippy::unwrap_used)] - fn test_parse_version_plain() { - let v = parse_version("1.2.3").unwrap(); - assert_eq!(v, Version::new(1, 2, 3)); - } - - #[test] - #[expect(clippy::unwrap_used)] - fn test_parse_version_with_v_prefix() { - let v = parse_version("v0.2.1").unwrap(); - assert_eq!(v, Version::new(0, 2, 1)); - } - - #[test] - #[expect(clippy::unwrap_used)] - fn test_parse_version_prerelease() { - let v = parse_version("v0.3.0-beta").unwrap(); - assert!(!v.pre.is_empty()); - } - #[test] fn test_expected_asset_name_macos() { let name = expected_asset_name("aarch64-apple-darwin"); @@ -363,110 +342,40 @@ mod tests { assert_matches!(result, Err(UpdateError::MissingChecksumAsset(_))); } - // ── parse_version Error Cases ───────────────────────────────── - - #[test] - fn test_parse_version_empty_string() { - let result = parse_version(""); - assert!(result.is_err()); - } - - #[test] - fn test_parse_version_whitespace_only() { - let result = parse_version(" "); - assert!(result.is_err()); - } - - #[test] - fn test_parse_version_invalid_chars() { - let result = parse_version("not-a-version"); - assert!(result.is_err()); - } - - #[test] - fn test_parse_version_missing_components() { - // Only major version - let result = parse_version("1"); - assert!(result.is_err()); - - // Only major.minor - let result = parse_version("1.2"); - assert!(result.is_err()); - } - - #[test] - fn test_parse_version_too_many_components() { - let result = parse_version("1.2.3.4.5"); - assert!(result.is_err()); - } - - #[test] - fn test_parse_version_negative_numbers() { - let result = parse_version("-1.2.3"); - assert!(result.is_err()); - } - - #[test] - fn test_parse_version_gibberish() { - let invalid_versions = vec![ - "@@@", - "v", - "V", - "version", - "1.2.3-rc.😀", // Invalid unicode in pre-release - "1.2.3+build🔧", // Invalid unicode in build metadata - ]; - - for version in invalid_versions { - let result = parse_version(version); - assert!(result.is_err(), "Expected error for: {version}"); - } - } - - #[test] - fn test_parse_version_invalid_prerelease_format() { - // Invalid prerelease identifier - let result = parse_version("1.2.3-"); - assert!(result.is_err()); - } - - #[test] - fn test_parse_version_invalid_build_metadata() { - // Valid version with build metadata should parse - let result = parse_version("1.2.3+build123"); - assert!(result.is_ok()); - - // But this is invalid - let result = parse_version("1.2.3+"); - assert!(result.is_err()); - } - - #[test] - #[expect(clippy::unwrap_used)] - fn test_parse_version_case_insensitive_v_prefix() { - // Both 'v' and 'V' should work - let v1 = parse_version("v1.2.3").unwrap(); - let v2 = parse_version("V1.2.3").unwrap(); - assert_eq!(v1, v2); - } - - #[test] - #[expect(clippy::unwrap_used)] - fn test_parse_version_with_build_metadata() { - let v = parse_version("1.2.3+build.123").unwrap(); - assert_eq!(v.major, 1); - assert_eq!(v.minor, 2); - assert_eq!(v.patch, 3); - } - - #[test] - #[expect(clippy::unwrap_used)] - fn test_parse_version_complex_prerelease() { - let v = parse_version("1.0.0-alpha.1+build.123").unwrap(); - assert_eq!(v.major, 1); - assert_eq!(v.minor, 0); - assert_eq!(v.patch, 0); - assert!(!v.pre.is_empty()); + #[rstest::rstest] + #[case("")] + #[case(" ")] + #[case("not-a-version")] + #[case("1")] + #[case("1.2")] + #[case("1.2.3.4.5")] + #[case("-1.2.3")] + #[case("@@@")] + #[case("v")] + #[case("V")] + #[case("version")] + #[case("1.2.3-rc.😀")] + #[case("1.2.3+build🔧")] + #[case("1.2.3-")] + #[case("1.2.3+")] + fn test_parse_version_invalid_tag_returns_parse_error(#[case] tag: &str) { + assert_matches!(parse_version(tag), Err(UpdateError::Parse(_))); + } + + #[rstest::rstest] + #[case("1.2.3", "1.2.3")] + #[case("v1.2.3", "1.2.3")] + #[case("V1.2.3", "1.2.3")] + #[case("v0.3.0-beta", "0.3.0-beta")] + #[case("1.2.3+build123", "1.2.3+build123")] + #[case("1.2.3+build.123", "1.2.3+build.123")] + #[case("1.0.0-alpha.1+build.123", "1.0.0-alpha.1+build.123")] + fn test_parse_version_valid_tag_preserves_version( + #[case] tag: &str, + #[case] expected: &str, + ) -> Result<(), UpdateError> { + assert_eq!(parse_version(tag)?.to_string(), expected); + Ok(()) } // ── expected_asset_name Edge Cases ────────────────────────────