From eee0a521afe0492f04bee58c9a236683fb23601b Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Fri, 28 Nov 2025 04:27:07 +0000 Subject: audit: fix shared test context to minimise events sent to production relays --- grasp-audit/src/bin/grasp-audit.rs | 42 +++++++------- grasp-audit/src/client.rs | 26 +++++++++ grasp-audit/src/fixtures.rs | 109 ++++++++++++++++++++++++------------- 3 files changed, 119 insertions(+), 58 deletions(-) (limited to 'grasp-audit/src') diff --git a/grasp-audit/src/bin/grasp-audit.rs b/grasp-audit/src/bin/grasp-audit.rs index 29f9c9a..2aabefe 100644 --- a/grasp-audit/src/bin/grasp-audit.rs +++ b/grasp-audit/src/bin/grasp-audit.rs @@ -120,7 +120,27 @@ async fn main() -> Result<()> { "all" => { println!("Running all tests...\n"); let mut all_results = AuditResult::new("All GRASP-01 Tests"); - + + // Repository creation tests + println!(" → Repository creation tests..."); + let repo_results = specs::RepositoryCreationTests::run_all(&client, &relay_domain).await; + all_results.merge(repo_results); + + // Git clone tests + println!(" → Git clone tests..."); + let clone_results = specs::GitCloneTests::run_all(&client, &relay_domain).await; + all_results.merge(clone_results); + + // Push authorization tests + println!(" → Push authorization tests..."); + let push_results = specs::PushAuthorizationTests::run_all(&client, &relay_domain).await; + all_results.merge(push_results); + + // Event acceptance policy tests + println!(" → Event acceptance policy tests..."); + let event_results = specs::EventAcceptancePolicyTests::run_all(&client).await; + all_results.merge(event_results); + // NIP-01 smoke tests println!(" → NIP-01 smoke tests..."); let nip01_results = specs::Nip01SmokeTests::run_all(&client).await; @@ -131,30 +151,10 @@ async fn main() -> Result<()> { let nip11_results = specs::Nip11DocumentTests::run_all(&client).await; all_results.merge(nip11_results); - // Event acceptance policy tests - println!(" → Event acceptance policy tests..."); - let event_results = specs::EventAcceptancePolicyTests::run_all(&client).await; - all_results.merge(event_results); - // CORS tests println!(" → CORS tests..."); let cors_results = specs::CorsTests::run_all(&client, &relay_domain).await; all_results.merge(cors_results); - - // Git clone tests - println!(" → Git clone tests..."); - let clone_results = specs::GitCloneTests::run_all(&client, &relay_domain).await; - all_results.merge(clone_results); - - // Push authorization tests - println!(" → Push authorization tests..."); - let push_results = specs::PushAuthorizationTests::run_all(&client, &relay_domain).await; - all_results.merge(push_results); - - // Repository creation tests - println!(" → Repository creation tests..."); - let repo_results = specs::RepositoryCreationTests::run_all(&client, &relay_domain).await; - all_results.merge(repo_results); println!(); all_results diff --git a/grasp-audit/src/client.rs b/grasp-audit/src/client.rs index 8b96f4f..4f3401c 100644 --- a/grasp-audit/src/client.rs +++ b/grasp-audit/src/client.rs @@ -1,11 +1,22 @@ //! Audit client for testing GRASP implementations use crate::audit::{AuditConfig, AuditEventBuilder, AuditMode}; +use crate::fixtures::FixtureKind; use anyhow::{anyhow, Result}; use nostr_sdk::prelude::*; +use std::collections::HashMap; +use std::sync::{Arc, Mutex}; use std::time::Duration; +/// Type alias for the fixture cache - shared across TestContext instances +pub type FixtureCache = Arc>>; + /// Client for auditing GRASP implementations +/// +/// The AuditClient owns a fixture cache that is shared across all TestContext +/// instances created from this client. This provides natural cache sharing: +/// - CLI creates one AuditClient → fixtures shared across all tests +/// - cargo test creates one AuditClient per test → fixtures isolated per test pub struct AuditClient { client: Client, pub config: AuditConfig, @@ -14,6 +25,8 @@ pub struct AuditClient { maintainer_keys: Keys, /// Recursive maintainer keys for testing recursive authorization scenarios recursive_maintainer_keys: Keys, + /// Fixture cache for TestContext instances - shared across all contexts using this client + fixture_cache: FixtureCache, } impl AuditClient { @@ -30,6 +43,7 @@ impl AuditClient { keys, maintainer_keys, recursive_maintainer_keys, + fixture_cache: Arc::new(Mutex::new(HashMap::new())), } } @@ -88,9 +102,19 @@ impl AuditClient { keys, maintainer_keys, recursive_maintainer_keys, + fixture_cache: Arc::new(Mutex::new(HashMap::new())), }) } + /// Get the fixture cache for TestContext usage + /// + /// This cache is shared across all TestContext instances created from this client. + /// In CLI mode (one client for all tests), fixtures are reused. + /// In test mode (one client per test), fixtures are isolated. + pub fn fixture_cache(&self) -> &FixtureCache { + &self.fixture_cache + } + /// Get the public key for this audit client pub fn public_key(&self) -> PublicKey { self.keys.public_key() @@ -488,6 +512,7 @@ mod tests { keys: keys.clone(), maintainer_keys, recursive_maintainer_keys, + fixture_cache: Arc::new(Mutex::new(HashMap::new())), }; let _builder = client.event_builder(Kind::TextNote, "test content"); @@ -508,6 +533,7 @@ mod tests { keys: keys.clone(), maintainer_keys, recursive_maintainer_keys, + fixture_cache: Arc::new(Mutex::new(HashMap::new())), }; // Create an event with a custom tag diff --git a/grasp-audit/src/fixtures.rs b/grasp-audit/src/fixtures.rs index 89531c6..81620f6 100644 --- a/grasp-audit/src/fixtures.rs +++ b/grasp-audit/src/fixtures.rs @@ -6,6 +6,17 @@ //! - **CI Mode (Isolated)**: Creates fresh events for each test, ensuring complete isolation //! - **Production Mode (Shared)**: Reuses shared fixtures to minimize event publication //! +//! # Cache Sharing Strategy +//! +//! The fixture cache lives on the `AuditClient`, not on `TestContext`. This provides +//! natural cache sharing semantics: +//! +//! - **CLI mode**: Creates one `AuditClient` → fixtures shared across all tests +//! - **cargo test**: Creates one `AuditClient` per test → fixtures isolated per test +//! +//! This eliminates the need for global state while still enabling fixture reuse +//! when appropriate. +//! //! # Example //! //! ```no_run @@ -25,8 +36,6 @@ use crate::{AuditClient, AuditMode}; use anyhow::{Context, Result}; use nostr_sdk::prelude::Event; -use std::collections::HashMap; -use std::sync::{Arc, Mutex}; /// Deterministic commit hash used in RepoState fixtures (Owner variant) /// This is the hash produced by creating a commit with: @@ -161,6 +170,13 @@ impl From for ContextMode { /// - In Isolated mode: Creates fresh events for each test /// - In Shared mode: Caches and reuses events across tests /// +/// # Cache Location +/// +/// The fixture cache lives on `AuditClient`, not on `TestContext`. This means: +/// - Multiple `TestContext` instances from the same client share the cache +/// - CLI mode (one client) naturally shares fixtures across all tests +/// - Test mode (one client per test) naturally isolates fixtures +/// /// # Example /// /// ```no_run @@ -181,20 +197,18 @@ impl From for ContextMode { pub struct TestContext<'a> { client: &'a AuditClient, mode: ContextMode, - cache: Arc>>, } impl<'a> TestContext<'a> { /// Create a new test context /// /// The context mode is automatically determined from the client's audit config. + /// The fixture cache is borrowed from the client, enabling natural sharing: + /// - Same client = shared cache (CLI mode behavior) + /// - Different clients = isolated caches (test mode behavior) pub fn new(client: &'a AuditClient) -> Self { let mode = ContextMode::from(client.config.mode); - Self { - client, - mode, - cache: Arc::new(Mutex::new(HashMap::new())), - } + Self { client, mode } } /// Create a test context with explicit mode override @@ -202,11 +216,7 @@ impl<'a> TestContext<'a> { /// This is useful for testing the context itself or for advanced use cases /// where you want to override the default mode behavior. pub fn with_mode(client: &'a AuditClient, mode: ContextMode) -> Self { - Self { - client, - mode, - cache: Arc::new(Mutex::new(HashMap::new())), - } + Self { client, mode } } /// Get a fixture, creating it if needed based on mode @@ -261,15 +271,28 @@ impl<'a> TestContext<'a> { } /// Get or create a shared fixture (caches for reuse) + /// + /// Uses the client's fixture cache to ensure fixtures are reused across + /// all TestContext instances in Production mode. async fn get_or_create_shared(&self, kind: FixtureKind) -> Result { - // Check cache first + // Check client's cache first (shared across all TestContext instances using same client) { - let cache = self.cache.lock().unwrap(); + let cache = self.client.fixture_cache().lock().unwrap(); if let Some(event) = cache.get(&kind) { + tracing::debug!("get_or_create_shared({:?}) found in client cache", kind); return Ok(event.clone()); } } + // Check relay connection before attempting to build + let is_connected = self.client.is_connected().await; + if !is_connected { + return Err(anyhow::anyhow!( + "Relay connection lost before building {:?} fixture (shared cache mode)", + kind + )); + } + // Not in cache, create it let event = self .build_fixture(kind) @@ -286,64 +309,76 @@ impl<'a> TestContext<'a> { ) })?; - // Store in cache + // Store in client's cache (shared across all TestContext instances using same client) { - let mut cache = self.cache.lock().unwrap(); + let mut cache = self.client.fixture_cache().lock().unwrap(); cache.insert(kind, event.clone()); + tracing::debug!("get_or_create_shared({:?}) stored in client cache ({} entries)", kind, cache.len()); } Ok(event) } - /// Get or create a ValidRepo, with caching within the TestContext. + /// Get or create a ValidRepo, with caching. /// This is a helper method that avoids async recursion by not going /// through get_fixture. It handles the repo specifically. /// - /// IMPORTANT: We always cache within a TestContext instance to ensure - /// fixture dependencies work correctly. The isolation between tests - /// comes from each test having its own TestContext with a fresh cache. + /// Uses client's fixture_cache for caching - in Shared mode this enables + /// cross-test reuse when the same client is used. async fn get_or_create_repo(&self) -> Result { - // Always check cache first - this ensures fixture dependencies work - // (e.g., MaintainerRepoAndState needs the SAME repo_id as RepoState) + // Check client's cache first (shared across all TestContext instances using same client) { - let cache = self.cache.lock().unwrap(); + let cache = self.client.fixture_cache().lock().unwrap(); if let Some(event) = cache.get(&FixtureKind::ValidRepo) { + tracing::debug!("get_or_create_repo() found in client cache"); return Ok(event.clone()); } } + // Check relay connection before creating repo + let is_connected = self.client.is_connected().await; + if !is_connected { + return Err(anyhow::anyhow!( + "Relay connection lost before creating ValidRepo fixture" + )); + } + // Create a new repo let test_name = format!( "fixture-{:?}-{}", FixtureKind::ValidRepo, &uuid::Uuid::new_v4().to_string()[..8] ); - let repo = self.client.create_repo_announcement(&test_name).await?; + + let repo = self.client.create_repo_announcement(&test_name).await + .with_context(|| format!("create_repo_announcement failed for {}", test_name))?; // Send it - self.client.send_event(repo.clone()).await?; + self.client.send_event(repo.clone()).await + .with_context(|| "Failed to send repo announcement to relay")?; - // Always cache it - isolation comes from each test having its own TestContext + // Store in client's cache (shared across all TestContext instances using same client) { - let mut cache = self.cache.lock().unwrap(); + let mut cache = self.client.fixture_cache().lock().unwrap(); cache.insert(FixtureKind::ValidRepo, repo.clone()); + tracing::debug!("get_or_create_repo() stored in client cache ({} entries)", cache.len()); } Ok(repo) } - /// Get or create a RepoWithIssue, with caching within the TestContext. + /// Get or create a RepoWithIssue, with caching via the client. /// Returns the issue event (repo is already sent/cached via get_or_create_repo). async fn get_or_create_issue(&self) -> Result { - // Always check cache first - ensures fixture dependencies work + // Check client's cache first { - let cache = self.cache.lock().unwrap(); + let cache = self.client.fixture_cache().lock().unwrap(); if let Some(event) = cache.get(&FixtureKind::RepoWithIssue) { return Ok(event.clone()); } } - // Get or create repo (reuses cached within this TestContext) + // Get or create repo (reuses cached via client) let repo = self.get_or_create_repo().await?; // Create the issue @@ -357,9 +392,9 @@ impl<'a> TestContext<'a> { // Send it self.client.send_event(issue.clone()).await?; - // Always cache it - isolation comes from each test having its own TestContext + // Store in client's cache { - let mut cache = self.cache.lock().unwrap(); + let mut cache = self.client.fixture_cache().lock().unwrap(); cache.insert(FixtureKind::RepoWithIssue, issue.clone()); } @@ -707,10 +742,10 @@ impl<'a> TestContext<'a> { /// Clear the fixture cache /// - /// This is useful for tests that want to ensure fresh fixtures - /// even in shared mode. + /// This clears the client's fixture cache, affecting all TestContext + /// instances using the same client. pub fn clear_cache(&self) { - let mut cache = self.cache.lock().unwrap(); + let mut cache = self.client.fixture_cache().lock().unwrap(); cache.clear(); } } -- cgit v1.2.3