diff options
| author | DanConwayDev <DanConwayDev@protonmail.com> | 2025-11-28 04:57:20 +0000 |
|---|---|---|
| committer | DanConwayDev <DanConwayDev@protonmail.com> | 2025-11-28 04:57:20 +0000 |
| commit | c45a425606776e937f4429402030aed8f2ab7436 (patch) | |
| tree | 5693fa14df57734d8bc910659fc6ce17876eb7e4 | |
| parent | c283c38add8d2b0e8359d8b03c064a188a43d6e0 (diff) | |
fix: respect Isolated mode in TestContext fixture helpers
Previously get_or_create_repo() and get_or_create_issue() always checked
the client cache first, bypassing the mode-based caching logic. This
caused fixture leaking across test suites when using the same AuditClient.
With this fix:
- In Isolated mode: helpers skip the cache, creating fresh fixtures
- In Shared mode: helpers use the cache for fixture reuse (unchanged)
This restores proper test isolation for push authorization tests that
were failing because they shared the same ValidRepo fixture.
| -rw-r--r-- | grasp-audit/src/fixtures.rs | 25 |
1 files changed, 14 insertions, 11 deletions
diff --git a/grasp-audit/src/fixtures.rs b/grasp-audit/src/fixtures.rs index 81620f6..6014343 100644 --- a/grasp-audit/src/fixtures.rs +++ b/grasp-audit/src/fixtures.rs | |||
| @@ -319,18 +319,20 @@ impl<'a> TestContext<'a> { | |||
| 319 | Ok(event) | 319 | Ok(event) |
| 320 | } | 320 | } |
| 321 | 321 | ||
| 322 | /// Get or create a ValidRepo, with caching. | 322 | /// Get or create a ValidRepo, with mode-aware caching. |
| 323 | /// This is a helper method that avoids async recursion by not going | 323 | /// This is a helper method that avoids async recursion by not going |
| 324 | /// through get_fixture. It handles the repo specifically. | 324 | /// through get_fixture. It handles the repo specifically. |
| 325 | /// | 325 | /// |
| 326 | /// Uses client's fixture_cache for caching - in Shared mode this enables | 326 | /// Uses client's fixture_cache for caching - in Shared mode this enables |
| 327 | /// cross-test reuse when the same client is used. | 327 | /// cross-test reuse when the same client is used. |
| 328 | /// In Isolated mode, the cache is bypassed to ensure fresh fixtures. | ||
| 328 | async fn get_or_create_repo(&self) -> Result<Event> { | 329 | async fn get_or_create_repo(&self) -> Result<Event> { |
| 329 | // Check client's cache first (shared across all TestContext instances using same client) | 330 | // Only check client's cache in Shared mode |
| 330 | { | 331 | // In Isolated mode, we always create fresh fixtures |
| 332 | if self.mode == ContextMode::Shared { | ||
| 331 | let cache = self.client.fixture_cache().lock().unwrap(); | 333 | let cache = self.client.fixture_cache().lock().unwrap(); |
| 332 | if let Some(event) = cache.get(&FixtureKind::ValidRepo) { | 334 | if let Some(event) = cache.get(&FixtureKind::ValidRepo) { |
| 333 | tracing::debug!("get_or_create_repo() found in client cache"); | 335 | tracing::debug!("get_or_create_repo() found in client cache (Shared mode)"); |
| 334 | return Ok(event.clone()); | 336 | return Ok(event.clone()); |
| 335 | } | 337 | } |
| 336 | } | 338 | } |
| @@ -357,8 +359,9 @@ impl<'a> TestContext<'a> { | |||
| 357 | self.client.send_event(repo.clone()).await | 359 | self.client.send_event(repo.clone()).await |
| 358 | .with_context(|| "Failed to send repo announcement to relay")?; | 360 | .with_context(|| "Failed to send repo announcement to relay")?; |
| 359 | 361 | ||
| 360 | // Store in client's cache (shared across all TestContext instances using same client) | 362 | // Store in client's cache only in Shared mode |
| 361 | { | 363 | // In Isolated mode, we don't cache to ensure test isolation |
| 364 | if self.mode == ContextMode::Shared { | ||
| 362 | let mut cache = self.client.fixture_cache().lock().unwrap(); | 365 | let mut cache = self.client.fixture_cache().lock().unwrap(); |
| 363 | cache.insert(FixtureKind::ValidRepo, repo.clone()); | 366 | cache.insert(FixtureKind::ValidRepo, repo.clone()); |
| 364 | tracing::debug!("get_or_create_repo() stored in client cache ({} entries)", cache.len()); | 367 | tracing::debug!("get_or_create_repo() stored in client cache ({} entries)", cache.len()); |
| @@ -367,11 +370,11 @@ impl<'a> TestContext<'a> { | |||
| 367 | Ok(repo) | 370 | Ok(repo) |
| 368 | } | 371 | } |
| 369 | 372 | ||
| 370 | /// Get or create a RepoWithIssue, with caching via the client. | 373 | /// Get or create a RepoWithIssue, with mode-aware caching via the client. |
| 371 | /// Returns the issue event (repo is already sent/cached via get_or_create_repo). | 374 | /// Returns the issue event (repo is already sent/cached via get_or_create_repo). |
| 372 | async fn get_or_create_issue(&self) -> Result<Event> { | 375 | async fn get_or_create_issue(&self) -> Result<Event> { |
| 373 | // Check client's cache first | 376 | // Only check client's cache in Shared mode |
| 374 | { | 377 | if self.mode == ContextMode::Shared { |
| 375 | let cache = self.client.fixture_cache().lock().unwrap(); | 378 | let cache = self.client.fixture_cache().lock().unwrap(); |
| 376 | if let Some(event) = cache.get(&FixtureKind::RepoWithIssue) { | 379 | if let Some(event) = cache.get(&FixtureKind::RepoWithIssue) { |
| 377 | return Ok(event.clone()); | 380 | return Ok(event.clone()); |
| @@ -392,8 +395,8 @@ impl<'a> TestContext<'a> { | |||
| 392 | // Send it | 395 | // Send it |
| 393 | self.client.send_event(issue.clone()).await?; | 396 | self.client.send_event(issue.clone()).await?; |
| 394 | 397 | ||
| 395 | // Store in client's cache | 398 | // Store in client's cache only in Shared mode |
| 396 | { | 399 | if self.mode == ContextMode::Shared { |
| 397 | let mut cache = self.client.fixture_cache().lock().unwrap(); | 400 | let mut cache = self.client.fixture_cache().lock().unwrap(); |
| 398 | cache.insert(FixtureKind::RepoWithIssue, issue.clone()); | 401 | cache.insert(FixtureKind::RepoWithIssue, issue.clone()); |
| 399 | } | 402 | } |