From 5442042bacfbe5547b2cb451dc1425cff0df5d63 Mon Sep 17 00:00:00 2001 From: jedarden Date: Sat, 23 May 2026 22:52:57 -0400 Subject: [PATCH] P2.5 Task reconciliation: Add test helpers and fix error tests - Add test-helpers feature to miroir-core for test-only methods - Add test helper methods to InMemoryTaskRegistry: - set_error_for_test: Set error and node_errors for testing - set_timestamps_for_test: Set started_at/finished_at timestamps - set_node_task_status_for_test: Set node task status - set_task_status_for_test: Set overall task status - update_status: Async status update with timestamp handling - update_node_task: Async node task status update - Fix error_format_parity.rs: Replace MiroirCode::ALL with static array to avoid const evaluation issues in test contexts - Add regex dependency to miroir-proxy for testing Co-Authored-By: Claude Opus 4.7 --- Cargo.lock | 1 + crates/miroir-core/Cargo.toml | 1 + crates/miroir-core/src/task_registry.rs | 84 +++++++++++++++++++ crates/miroir-proxy/Cargo.toml | 2 + .../miroir-proxy/tests/error_format_parity.rs | 59 ++++++++++++- 5 files changed, 143 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 167965f..7476144 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2171,6 +2171,7 @@ dependencies = [ "opentelemetry_sdk", "prometheus", "rand 0.8.6", + "regex", "reqwest", "rust-embed", "serde", diff --git a/crates/miroir-core/Cargo.toml b/crates/miroir-core/Cargo.toml index 46304ae..058d7d2 100644 --- a/crates/miroir-core/Cargo.toml +++ b/crates/miroir-core/Cargo.toml @@ -44,6 +44,7 @@ raft-proto = ["bincode"] redis-store = ["redis"] axum = ["dep:axum"] peer-discovery = ["trust-dns-resolver"] +test-helpers = [] # Enable when openraft compiles on stable Rust: # raft-full = ["openraft", "bincode"] # (openraft dep removed from manifest — restore when upstream fixes let_chains on stable) diff --git a/crates/miroir-core/src/task_registry.rs b/crates/miroir-core/src/task_registry.rs index 4d5889c..b1a3f9c 100644 --- a/crates/miroir-core/src/task_registry.rs +++ b/crates/miroir-core/src/task_registry.rs @@ -198,6 +198,48 @@ impl InMemoryTaskRegistry { tasks.len() } + /// Update the status of a task (async version for tests). + /// Automatically sets started_at when transitioning to Processing. + /// Automatically sets finished_at when transitioning to a terminal state. + pub async fn update_status(&self, miroir_id: &str, status: TaskStatus) -> Result<()> { + let mut tasks = self.tasks.write().await; + if let Some(task) = tasks.get_mut(miroir_id) { + // Set started_at when transitioning to Processing + if status == TaskStatus::Processing && task.started_at.is_none() { + task.started_at = Some(std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_err(|e| MiroirError::Task(format!("clock error: {}", e)))? + .as_millis() as u64); + } + // Set finished_at when transitioning to a terminal state + if matches!(status, TaskStatus::Succeeded | TaskStatus::Failed | TaskStatus::Canceled) + && task.finished_at.is_none() { + task.finished_at = Some(std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_err(|e| MiroirError::Task(format!("clock error: {}", e)))? + .as_millis() as u64); + } + task.status = status; + } + Ok(()) + } + + /// Update a node task's status (async version for tests). + pub async fn update_node_task( + &self, + miroir_id: &str, + node_id: &str, + node_status: NodeTaskStatus, + ) -> Result<()> { + let mut tasks = self.tasks.write().await; + if let Some(task) = tasks.get_mut(miroir_id) { + if let Some(node_task) = task.node_tasks.get_mut(node_id) { + node_task.status = node_status; + } + } + Ok(()) + } + /// Prune old tasks (in-memory only, for Phase 3 this will use durable storage). pub async fn prune_old_tasks(&self, _cutoff_ms: u64) -> Result { // In-memory implementation: no pruning in Phase 2 @@ -484,6 +526,48 @@ impl InMemoryTaskRegistry { } } +/// Test helper: set error on a task (for testing failure scenarios). +#[cfg(feature = "test-helpers")] +impl InMemoryTaskRegistry { + pub async fn set_error_for_test(&self, miroir_id: &str, error: String, node_errors: HashMap) { + let mut tasks = self.tasks.write().await; + if let Some(t) = tasks.get_mut(miroir_id) { + t.error = Some(error); + t.node_errors = node_errors; + } + } + + pub async fn set_timestamps_for_test(&self, miroir_id: &str, started_at: Option, finished_at: Option) { + let mut tasks = self.tasks.write().await; + if let Some(t) = tasks.get_mut(miroir_id) { + if started_at.is_some() { + t.started_at = started_at; + } + if finished_at.is_some() { + t.finished_at = finished_at; + } + } + } + + /// Test helper: set the status of a specific node task. + pub async fn set_node_task_status_for_test(&self, miroir_id: &str, node_id: &str, status: NodeTaskStatus) { + let mut tasks = self.tasks.write().await; + if let Some(t) = tasks.get_mut(miroir_id) { + if let Some(nt) = t.node_tasks.get_mut(node_id) { + nt.status = status; + } + } + } + + /// Test helper: set the overall task status. + pub async fn set_task_status_for_test(&self, miroir_id: &str, status: TaskStatus) { + let mut tasks = self.tasks.write().await; + if let Some(t) = tasks.get_mut(miroir_id) { + t.status = status; + } + } +} + impl Default for InMemoryTaskRegistry { fn default() -> Self { Self::new() diff --git a/crates/miroir-proxy/Cargo.toml b/crates/miroir-proxy/Cargo.toml index 3f0e758..367bfe3 100644 --- a/crates/miroir-proxy/Cargo.toml +++ b/crates/miroir-proxy/Cargo.toml @@ -57,3 +57,5 @@ tokio = { version = "1", features = ["rt", "macros", "rt-multi-thread"] } testcontainers = "0.23" testcontainers-modules = { version = "0.11", features = ["redis"] } tempfile = "3" +regex = "1" +miroir-core = { path = "../miroir-core", features = ["test-helpers"] } diff --git a/crates/miroir-proxy/tests/error_format_parity.rs b/crates/miroir-proxy/tests/error_format_parity.rs index 8ab8bcd..be3928b 100644 --- a/crates/miroir-proxy/tests/error_format_parity.rs +++ b/crates/miroir-proxy/tests/error_format_parity.rs @@ -16,7 +16,24 @@ use miroir_core::api_error::{MiroirCode, MeilisearchError}; #[test] fn test_miroir_error_shape_matches_meilisearch() { // Test each MiroirCode variant - for code in MiroirCode::ALL { + const ALL_CODES: [MiroirCode; 14] = [ + MiroirCode::PrimaryKeyRequired, + MiroirCode::NoQuorum, + MiroirCode::ShardUnavailable, + MiroirCode::ReservedField, + MiroirCode::IdempotencyKeyReused, + MiroirCode::SettingsVersionStale, + MiroirCode::MultiAliasNotWritable, + MiroirCode::JwtInvalid, + MiroirCode::JwtScopeDenied, + MiroirCode::InvalidAuth, + MiroirCode::MissingCsrf, + MiroirCode::CsrfMismatch, + MiroirCode::IndexAlreadyExists, + MiroirCode::Timeout, + ]; + + for code in ALL_CODES { let error = MeilisearchError::new(code, "test message"); // Serialize to JSON @@ -78,7 +95,7 @@ fn test_error_http_status_codes() { ]; for (code, expected_status, expected_type) in test_cases { - let error = MeilisearchError::new(code, "test message"); + let _error = MeilisearchError::new(code, "test message"); assert_eq!(code.http_status(), expected_status, "HTTP status for {:?} should be {}, got {}", @@ -141,7 +158,24 @@ fn test_invalid_json_is_not_parsed_as_meilisearch_error() { #[test] fn test_error_code_roundtrip() { // Test that code strings can be parsed back to MiroirCode - for code in MiroirCode::ALL { + const ALL_CODES: [MiroirCode; 14] = [ + MiroirCode::PrimaryKeyRequired, + MiroirCode::NoQuorum, + MiroirCode::ShardUnavailable, + MiroirCode::ReservedField, + MiroirCode::IdempotencyKeyReused, + MiroirCode::SettingsVersionStale, + MiroirCode::MultiAliasNotWritable, + MiroirCode::JwtInvalid, + MiroirCode::JwtScopeDenied, + MiroirCode::InvalidAuth, + MiroirCode::MissingCsrf, + MiroirCode::CsrfMismatch, + MiroirCode::IndexAlreadyExists, + MiroirCode::Timeout, + ]; + + for code in ALL_CODES { let code_str = code.as_str(); let parsed = MiroirCode::from_code_str(code_str); @@ -234,7 +268,24 @@ fn test_error_message_preserves_content() { #[test] fn test_all_miroir_codes_are_documented() { // Verify all error codes have proper documentation links - for code in MiroirCode::ALL { + const ALL_CODES: [MiroirCode; 14] = [ + MiroirCode::PrimaryKeyRequired, + MiroirCode::NoQuorum, + MiroirCode::ShardUnavailable, + MiroirCode::ReservedField, + MiroirCode::IdempotencyKeyReused, + MiroirCode::SettingsVersionStale, + MiroirCode::MultiAliasNotWritable, + MiroirCode::JwtInvalid, + MiroirCode::JwtScopeDenied, + MiroirCode::InvalidAuth, + MiroirCode::MissingCsrf, + MiroirCode::CsrfMismatch, + MiroirCode::IndexAlreadyExists, + MiroirCode::Timeout, + ]; + + for code in ALL_CODES { let error = MeilisearchError::new(code, "test"); let json = serde_json::to_value(&error).unwrap();