diff options
| author | DanConwayDev <DanConwayDev@protonmail.com> | 2025-11-18 16:50:02 +0000 |
|---|---|---|
| committer | DanConwayDev <DanConwayDev@protonmail.com> | 2025-11-18 16:50:02 +0000 |
| commit | 98c6fa4bfa897ff0b8f9c95ea698d4d065b5e9f3 (patch) | |
| tree | 14e94bbbfa1658229dbcd6ce9a03b3eb5cce4aef /docs/archive/2025-11-06-testcontext-migration-guide.md | |
| parent | 1fe9c179d5dd73d443ab4792d4c2fbd19690afcb (diff) | |
docs: switch focus onto grasp implementation
Diffstat (limited to 'docs/archive/2025-11-06-testcontext-migration-guide.md')
| -rw-r--r-- | docs/archive/2025-11-06-testcontext-migration-guide.md | 279 |
1 files changed, 279 insertions, 0 deletions
diff --git a/docs/archive/2025-11-06-testcontext-migration-guide.md b/docs/archive/2025-11-06-testcontext-migration-guide.md new file mode 100644 index 0000000..c7d29c8 --- /dev/null +++ b/docs/archive/2025-11-06-testcontext-migration-guide.md | |||
| @@ -0,0 +1,279 @@ | |||
| 1 | # TestContext Pattern Migration Guide | ||
| 2 | |||
| 3 | ## Overview | ||
| 4 | |||
| 5 | The `TestContext` pattern solves the isolation vs. rate-limiting problem for grasp-audit tests by supporting dual-mode operation: | ||
| 6 | |||
| 7 | - **CI Mode (Isolated)**: Creates fresh events for each test - full isolation | ||
| 8 | - **Production Mode (Shared)**: Caches and reuses fixtures - 60-90% fewer events | ||
| 9 | |||
| 10 | ## Architecture | ||
| 11 | |||
| 12 | ### Core Components | ||
| 13 | |||
| 14 | 1. **`FixtureKind`** - Enum defining available fixture types | ||
| 15 | 2. **`ContextMode`** - Enum controlling behavior (Isolated vs Shared) | ||
| 16 | 3. **`TestContext<'a>`** - Mode-aware fixture manager with caching | ||
| 17 | |||
| 18 | ### Files Modified | ||
| 19 | |||
| 20 | - [`grasp-audit/src/fixtures.rs`](../grasp-audit/src/fixtures.rs) - New file with TestContext implementation | ||
| 21 | - [`grasp-audit/src/lib.rs`](../grasp-audit/src/lib.rs) - Exports new types | ||
| 22 | - [`grasp-audit/src/specs/grasp01/event_acceptance_policy.rs`](../grasp-audit/src/specs/grasp01/event_acceptance_policy.rs) - Example migrations | ||
| 23 | |||
| 24 | ## Migration Strategy | ||
| 25 | |||
| 26 | ### Step 1: Identify Prerequisite Events | ||
| 27 | |||
| 28 | Look for tests that create prerequisite events (repos, issues, etc.) before testing the actual functionality. | ||
| 29 | |||
| 30 | **Before:** | ||
| 31 | |||
| 32 | ```rust | ||
| 33 | async fn test_accept_issue_via_a_tag(client: &AuditClient) -> TestResult { | ||
| 34 | // 1. Create and send repo announcement | ||
| 35 | let repo = Self::create_test_repo(client, "test-repo-1").await?; | ||
| 36 | Self::send_and_verify_accepted(client, repo.clone(), "repository announcement").await?; | ||
| 37 | |||
| 38 | // 2. Create issue that references the repo | ||
| 39 | let issue = Self::create_issue_for_repo(client, &repo, "Test Issue 1")?; | ||
| 40 | |||
| 41 | // 3. Test actual functionality | ||
| 42 | Self::send_and_verify_accepted(client, issue, "issue via 'a' tag").await?; | ||
| 43 | Ok(()) | ||
| 44 | } | ||
| 45 | ``` | ||
| 46 | |||
| 47 | ### Step 2: Replace with TestContext | ||
| 48 | |||
| 49 | **After:** | ||
| 50 | |||
| 51 | ```rust | ||
| 52 | async fn test_accept_issue_via_a_tag(client: &AuditClient) -> TestResult { | ||
| 53 | // 1. Create TestContext | ||
| 54 | let ctx = TestContext::new(client); | ||
| 55 | |||
| 56 | // 2. Get repository fixture (mode-aware) | ||
| 57 | let repo = ctx.get_fixture(FixtureKind::ValidRepo).await?; | ||
| 58 | |||
| 59 | // 3. Create issue and test actual functionality | ||
| 60 | let issue = Self::create_issue_for_repo(client, &repo, "Test Issue 1")?; | ||
| 61 | Self::send_and_verify_accepted(client, issue, "issue via 'a' tag").await?; | ||
| 62 | Ok(()) | ||
| 63 | } | ||
| 64 | ``` | ||
| 65 | |||
| 66 | ### Step 3: Add Imports | ||
| 67 | |||
| 68 | At the top of your test file: | ||
| 69 | |||
| 70 | ```rust | ||
| 71 | use crate::{TestContext, FixtureKind}; | ||
| 72 | ``` | ||
| 73 | |||
| 74 | ## Available Fixtures | ||
| 75 | |||
| 76 | ### Current Fixture Types | ||
| 77 | |||
| 78 | 1. **`FixtureKind::ValidRepo`** - Basic repository announcement (kind 30617) | ||
| 79 | 2. **`FixtureKind::RepoWithIssue`** - Repository with one issue (kind 1621) | ||
| 80 | 3. **`FixtureKind::RepoWithComment`** - Repository with issue and comment (kind 1111) | ||
| 81 | 4. **`FixtureKind::RepoState`** - Repository state announcement (kind 30618) | ||
| 82 | |||
| 83 | ### Adding New Fixtures | ||
| 84 | |||
| 85 | To add a new fixture type: | ||
| 86 | |||
| 87 | 1. Add variant to `FixtureKind` enum: | ||
| 88 | |||
| 89 | ```rust | ||
| 90 | pub enum FixtureKind { | ||
| 91 | // ... existing variants | ||
| 92 | NewFixtureType, | ||
| 93 | } | ||
| 94 | ``` | ||
| 95 | |||
| 96 | 2. Add case to `build_fixture` method: | ||
| 97 | |||
| 98 | ```rust | ||
| 99 | async fn build_fixture(&self, kind: FixtureKind) -> Result<Event> { | ||
| 100 | match kind { | ||
| 101 | // ... existing cases | ||
| 102 | FixtureKind::NewFixtureType => { | ||
| 103 | // Create and return event | ||
| 104 | } | ||
| 105 | } | ||
| 106 | } | ||
| 107 | ``` | ||
| 108 | |||
| 109 | ## Event Count Comparison | ||
| 110 | |||
| 111 | ### Before Migration (All Tests) | ||
| 112 | |||
| 113 | All modes send the same number of events: | ||
| 114 | |||
| 115 | - 15 tests × ~3 events each = **~45 events total** | ||
| 116 | |||
| 117 | ### After Migration | ||
| 118 | |||
| 119 | **CI Mode (Isolated):** | ||
| 120 | |||
| 121 | - Still ~45 events (maintains full isolation) | ||
| 122 | |||
| 123 | **Production Mode (Shared):** | ||
| 124 | |||
| 125 | - Initial setup: ~5 events (one per fixture type) | ||
| 126 | - Subsequent tests: Reuse cached fixtures | ||
| 127 | - Total: **~5-35 events (60-90% reduction)** | ||
| 128 | |||
| 129 | ## Mode-Specific Behavior | ||
| 130 | |||
| 131 | ### CI Mode (Default for Tests) | ||
| 132 | |||
| 133 | ```rust | ||
| 134 | let config = AuditConfig::ci(); | ||
| 135 | let client = AuditClient::new("ws://localhost:7000", config).await?; | ||
| 136 | let ctx = TestContext::new(&client); | ||
| 137 | |||
| 138 | // Always creates fresh fixture | ||
| 139 | let repo1 = ctx.get_fixture(FixtureKind::ValidRepo).await?; | ||
| 140 | let repo2 = ctx.get_fixture(FixtureKind::ValidRepo).await?; | ||
| 141 | assert_ne!(repo1.id, repo2.id); // Different IDs - fresh events | ||
| 142 | ``` | ||
| 143 | |||
| 144 | ### Production Mode (CLI Default) | ||
| 145 | |||
| 146 | ```rust | ||
| 147 | let config = AuditConfig::production(); | ||
| 148 | let client = AuditClient::new("ws://localhost:7000", config).await?; | ||
| 149 | let ctx = TestContext::new(&client); | ||
| 150 | |||
| 151 | // Returns cached fixture on second call | ||
| 152 | let repo1 = ctx.get_fixture(FixtureKind::ValidRepo).await?; | ||
| 153 | let repo2 = ctx.get_fixture(FixtureKind::ValidRepo).await?; | ||
| 154 | assert_eq!(repo1.id, repo2.id); // Same ID - reused event | ||
| 155 | ``` | ||
| 156 | |||
| 157 | ## Testing the Migration | ||
| 158 | |||
| 159 | ### Run Refactored Tests | ||
| 160 | |||
| 161 | ```bash | ||
| 162 | # Using test-ngit-relay.sh (recommended) | ||
| 163 | cd grasp-audit && nix develop -c bash test-ngit-relay.sh --mode test | ||
| 164 | |||
| 165 | # Manual testing | ||
| 166 | RELAY_URL="ws://localhost:18081" nix develop -c cargo test --lib test_accept_issue_via_a_tag -- --ignored --nocapture | ||
| 167 | ``` | ||
| 168 | |||
| 169 | ### Verify Event Counts | ||
| 170 | |||
| 171 | Monitor event publication in relay logs: | ||
| 172 | |||
| 173 | ```bash | ||
| 174 | # Count events sent during test run | ||
| 175 | RELAY_URL="ws://localhost:18081" nix develop -c cargo test --lib -- --ignored --nocapture 2>&1 | grep -c "EVENT" | ||
| 176 | ``` | ||
| 177 | |||
| 178 | ## Best Practices | ||
| 179 | |||
| 180 | ### 1. Use TestContext for Prerequisites Only | ||
| 181 | |||
| 182 | ✅ **Good:** Use TestContext for setup events | ||
| 183 | |||
| 184 | ```rust | ||
| 185 | let ctx = TestContext::new(client); | ||
| 186 | let repo = ctx.get_fixture(FixtureKind::ValidRepo).await?; | ||
| 187 | let test_event = create_custom_event(&repo)?; // Test-specific event | ||
| 188 | ``` | ||
| 189 | |||
| 190 | ❌ **Bad:** Don't use for events you're actually testing | ||
| 191 | |||
| 192 | ```rust | ||
| 193 | // Wrong - you want to test THIS event, not reuse it | ||
| 194 | let issue = ctx.get_fixture(FixtureKind::RepoWithIssue).await?; | ||
| 195 | ``` | ||
| 196 | |||
| 197 | ### 2. Error Handling | ||
| 198 | |||
| 199 | Never use use `.map_err(|e| e.to_string())?` to convert anyhow errors accept for final display but instead use the error: | ||
| 200 | |||
| 201 | ❌ **Bad:** Don't use `.map_err(|e| e.to_string())?` to convert anyhow errors unless displaying. | ||
| 202 | |||
| 203 | ```rust | ||
| 204 | let repo = ctx.get_fixture(FixtureKind::ValidRepo).await | ||
| 205 | .map_err(|e| e.to_string())?; | ||
| 206 | ``` | ||
| 207 | |||
| 208 | ### 3. Clear Cache When Needed | ||
| 209 | |||
| 210 | For tests that modify fixtures: | ||
| 211 | |||
| 212 | ```rust | ||
| 213 | let ctx = TestContext::new(client); | ||
| 214 | // ... test that modifies state ... | ||
| 215 | ctx.clear_cache(); // Ensure fresh fixtures for next test | ||
| 216 | ``` | ||
| 217 | |||
| 218 | ### 4. Document Mode Behavior | ||
| 219 | |||
| 220 | Add comments explaining mode-specific behavior: | ||
| 221 | |||
| 222 | ```rust | ||
| 223 | // NEW: Request repository fixture - behavior depends on mode | ||
| 224 | // CI mode: Creates fresh repo for this test | ||
| 225 | // Production mode: Returns cached repo if available | ||
| 226 | let repo = ctx.get_fixture(FixtureKind::ValidRepo).await?; | ||
| 227 | ``` | ||
| 228 | |||
| 229 | ## Migration Checklist | ||
| 230 | |||
| 231 | For each test: | ||
| 232 | |||
| 233 | - [ ] Identify prerequisite events (repos, issues, etc.) | ||
| 234 | - [ ] Determine appropriate `FixtureKind` | ||
| 235 | - [ ] Add `TestContext` imports | ||
| 236 | - [ ] Replace manual event creation with `ctx.get_fixture()` | ||
| 237 | - [ ] Add `.map_err(|e| e.to_string())?` for error handling | ||
| 238 | - [ ] Add mode-behavior comments | ||
| 239 | - [ ] Verify test still passes in CI mode | ||
| 240 | - [ ] Test in production mode (optional verification) | ||
| 241 | |||
| 242 | ## Examples | ||
| 243 | |||
| 244 | ### Example 1: Simple Repository Prerequisite | ||
| 245 | |||
| 246 | See [`test_accept_issue_via_a_tag`](../grasp-audit/src/specs/grasp01/event_acceptance_policy.rs:513-530) for a complete example. | ||
| 247 | |||
| 248 | ### Example 2: Complex State Setup | ||
| 249 | |||
| 250 | See [`test_accept_valid_repo_state_announcement`](../grasp-audit/src/specs/grasp01/event_acceptance_policy.rs:354-397) for state announcement example. | ||
| 251 | |||
| 252 | ## Troubleshooting | ||
| 253 | |||
| 254 | ### Tests Failing in Production Mode | ||
| 255 | |||
| 256 | If tests fail when reusing fixtures, the test may be: | ||
| 257 | |||
| 258 | 1. Modifying shared state | ||
| 259 | 2. Depending on unique event IDs | ||
| 260 | 3. Testing fixture creation itself (should use CI mode) | ||
| 261 | |||
| 262 | **Solution:** Either fix the test or use `ContextMode::Isolated` explicitly: | ||
| 263 | |||
| 264 | ```rust | ||
| 265 | let ctx = TestContext::with_mode(client, ContextMode::Isolated); | ||
| 266 | ``` | ||
| 267 | |||
| 268 | ## Future Work | ||
| 269 | |||
| 270 | - [ ] Migrate remaining tests (gradual migration) | ||
| 271 | - [ ] Add more fixture types as needed | ||
| 272 | - [ ] Add fixture cleanup strategies | ||
| 273 | - [ ] Add metrics for event count reduction | ||
| 274 | |||
| 275 | ## References | ||
| 276 | |||
| 277 | - [`fixtures.rs`](../grasp-audit/src/fixtures.rs) - TestContext implementation | ||
| 278 | - [`event_acceptance_policy.rs`](../grasp-audit/src/specs/grasp01/event_acceptance_policy.rs) - Migration examples | ||
| 279 | - [Original proposal](./testcontext-pattern-proposal.md) - Design rationale | ||