Fix test interference from shared notification IDs

[yewreeka]

Test fixtures in IncomingMessageWriterExplodeTests and
ExpiredConversationsWorkerTests both used the hardcoded conversation ID
"test-conversation-id". When Swift Testing runs suites in parallel,
a .conversationExpired notification posted by one suite's test could be
captured by another suite's NotificationCapture, causing
testPastDatePostsExpiredNotification to see count == 2 instead of 1.

Fix: use unique UUIDs for conversation IDs in test fixtures and filter
captured notifications by conversationId.

Note

Use per-instance UUID-prefixed conversationId in ExpiredWorkerTestFixtures and ExplodeTestFixtures to fix test interference from shared notification IDs

Tests generate unique conversationId values via UUID and update .conversationExpired notification assertions to filter by matching conversationId and expect a count of 1.

📍Where to Start

Start with the notification filtering assertion in IncomingMessageWriterExplodeTests.testFromCreatorSchedulesExplosion in IncomingMessageWriterExplodeTests.swift, then review fixture conversationId generation in ExpiredConversationsWorkerTests.swift.

^{Macroscope summarized 3428af8.}

[yewreeka]

• Fix test interference from shared notification IDs #524 👈 (View in Graphite)
• dev

---

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

Claude finished @yewreeka's task —— View job

---

Code Review

Summary

This PR effectively fixes test interference from shared notification IDs. The changes are minimal, targeted, and correctly address the root cause of parallel test failures.

---

✅ Code Quality & Correctness

Strong Points:

• Root cause properly addressed: Using unique UUIDs ("expired-worker-test-\(UUID().uuidString)" and "explode-test-\(UUID().uuidString)") eliminates collision risk when tests run in parallel
• Minimal, surgical change: Only 2 lines modified in fixture constructors
• Proper notification filtering: All assertions correctly filter by conversationId to match their specific test fixture

File: ExpiredConversationsWorkerTests.swift:173

let conversationId: String = "expired-worker-test-\(UUID().uuidString)"

This change ensures each test suite instance has a unique conversation ID.

File: IncomingMessageWriterExplodeTests.swift:269

let conversationId: String = "explode-test-\(UUID().uuidString)"

Same pattern applied consistently.

---

🔍 Analysis of Notification Filtering

The fix includes proper filtering in all tests that capture notifications. Key examples:

ExpiredConversationsWorkerTests.swift:233-236

let matching = capture.notifications(named: .conversationExpired).filter {
    $0.userInfo?["conversationId"] as? String == fixtures.conversationId
}
#expect(matching.count == 1)

This ensures the test only counts notifications matching its own conversation ID, preventing cross-contamination from other parallel suites.

---

🎯 Best Practices

Follows Project Conventions:

• ✅ Uses descriptive prefixes (expired-worker-test-, explode-test-) for debugging clarity
• ✅ Consistent pattern applied across both test suites
• ✅ Maintains existing test structure - only fixtures changed
• ✅ Tests remain serialized within each suite (@Suite(..., .serialized))

---

🧪 Test Coverage

Excellent coverage of the interference scenario:

• The fix addresses the specific race condition described in the PR body
• All 12 tests in IncomingMessageWriterExplodeTests now have isolated conversation IDs
• All 5 tests in ExpiredConversationsWorkerTests now have isolated conversation IDs
• No new tests needed - the fix is at the fixture level

---

⚡ Performance Considerations

Negligible impact:

• UUID generation is lightweight (nanoseconds)
• Happens once per test suite instantiation
• No runtime performance concerns

---

🔒 Security Concerns

None identified:

• Test-only code changes
• UUIDs are not security-sensitive in this context
• No exposure of sensitive data

---

💡 Potential Improvements

Minor suggestions (not blockers):

1. Consider caching UUID in fixtures - Currently the UUID is generated as part of the property initialization, which is fine. If multiple tests in the same suite ever needed to share the same ID across different fixture instances (unlikely given current patterns), you could make it a static property.

2. Documentation opportunity - Consider adding a comment explaining why UUIDs are used:

   // Use unique ID per suite to prevent notification interference when tests run in parallel
   let conversationId: String = "expired-worker-test-\(UUID().uuidString)"

   Though per CLAUDE.md guidelines, comments should be minimal, so this is optional.

---

✅ Final Verdict

LGTM - Approved ✅

This is a clean, well-reasoned fix that:

• Correctly diagnoses and resolves the parallel test interference issue
• Uses the appropriate Swift Testing patterns
• Maintains code quality and follows project conventions
• Has zero risk of introducing new bugs

The fix is production-ready and can be merged.

---