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 /grasp-audit/src | |
| 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.
Diffstat (limited to 'grasp-audit/src')
| -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 | } |