Skip to content

Commit 8d3702a

Browse files
committed
fix(test-utils): Fix buffer pruning to use strict < instead of <=
The buffer pruning condition (<=) was inconsistent with the live event sending condition (>=). This meant buffered events at exactly the same millisecond as listener registration would be incorrectly pruned, even though live events at the same timestamp would be sent. With millisecond timestamps, same-timestamp collisions are more likely, which could cause intermittent test failures. Changed to strict < to match the >= behavior for live events.
1 parent 6465550 commit 8d3702a

File tree

2 files changed

+159
-2
lines changed

2 files changed

+159
-2
lines changed
Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
---
2+
name: Migrate E2E to Spotlight
3+
overview: Replace the custom event proxy server with Spotlight for E2E test event capture. This eliminates race conditions by using Spotlight's reliable event streaming instead of the fragile timestamp-based filtering in the current proxy.
4+
todos:
5+
- id: add-spotlight-dep
6+
content: Add @spotlightjs/spotlight as devDependency to test-utils
7+
status: completed
8+
- id: create-spotlight-helpers
9+
content: Create spotlight.ts with startSpotlight() and event waiting helpers
10+
status: completed
11+
- id: update-playwright-config
12+
content: Add Spotlight support to getPlaywrightConfig()
13+
status: completed
14+
- id: migrate-first-app
15+
content: Migrate react-router-7-lazy-routes as proof of concept
16+
status: in_progress
17+
- id: migrate-remaining
18+
content: Migrate remaining ~106 test apps incrementally
19+
status: pending
20+
- id: cleanup-old-proxy
21+
content: Remove deprecated event proxy code after full migration
22+
status: pending
23+
---
24+
25+
# Migrate E2E Tests from Event Proxy to Spotlight
26+
27+
## Current Architecture
28+
29+
```mermaid
30+
flowchart LR
31+
subgraph current [Current Setup]
32+
App[Test App] -->|"tunnel: localhost:3031"| Proxy[Event Proxy Server]
33+
Proxy -->|Buffer + Timestamp Filter| Buffer[(Event Buffer)]
34+
Test[Playwright Test] -->|waitForTransaction| Buffer
35+
end
36+
```
37+
38+
**Problems with current approach:**
39+
40+
- Timestamp filtering has race conditions (stale events leak through)
41+
- Complex buffer management with timing edge cases
42+
- 108 test apps, ~162 files use `tunnel: localhost:3031`
43+
44+
## Proposed Architecture
45+
46+
```mermaid
47+
flowchart LR
48+
subgraph proposed [Spotlight Setup]
49+
App2[Test App] -->|"DSN: spotlight@localhost:PORT/0"| Spotlight[Spotlight Sidecar]
50+
Spotlight -->|JSON Stream| Stream[(stdout/stderr)]
51+
Test2[Playwright Test] -->|waitForTransaction| Stream
52+
end
53+
```
54+
55+
**Benefits:**
56+
57+
- No timestamp filtering needed - Spotlight streams events in order
58+
- Production-tested event handling
59+
- Cleaner architecture with less custom code
60+
61+
## Implementation Plan
62+
63+
### Phase 1: Add Spotlight Infrastructure
64+
65+
1. **Add `@spotlightjs/spotlight` dependency** to [`dev-packages/test-utils/package.json`](dev-packages/test-utils/package.json)
66+
67+
2. **Create new Spotlight-based helpers** in [`dev-packages/test-utils/src/spotlight.ts`](dev-packages/test-utils/src/spotlight.ts):
68+
69+
- `startSpotlight(options)` - Spawns `spotlight run` with dynamic port, parses port from stderr
70+
- `waitForSpotlightEvent(filter)` - Reads from Spotlight's JSON output stream
71+
- `waitForTransaction()`, `waitForError()`, etc. - High-level wrappers matching current API
72+
73+
3. **Update [`dev-packages/test-utils/src/playwright-config.ts`](dev-packages/test-utils/src/playwright-config.ts)**:
74+
75+
- Add option to use Spotlight instead of event proxy
76+
- Configure webServer to run Spotlight with dynamic port
77+
- Pass port via environment variable to test app
78+
79+
### Phase 2: Create Migration Path
80+
81+
4. **Design backward-compatible API**:
82+
83+
- Keep existing `waitForTransaction()` signature
84+
- New implementation reads from Spotlight instead of HTTP streaming
85+
- Port is passed via `SPOTLIGHT_PORT` env var
86+
87+
5. **Update DSN handling in test apps**:
88+
89+
- Replace `tunnel: 'http://localhost:3031'` with DSN workaround
90+
- DSN format: `http://spotlight@localhost:${SPOTLIGHT_PORT}/0`
91+
- This makes SDK operate in "normal" mode (no tunnel)
92+
93+
### Phase 3: Migrate Test Apps (Incremental)
94+
95+
6. **Migration per test app involves**:
96+
97+
- Remove `start-event-proxy.mjs`
98+
- Update `playwright.config.mjs` to use new Spotlight config
99+
- Update Sentry init to use DSN workaround instead of tunnel
100+
- No changes needed to test files (API stays the same)
101+
102+
7. **Start with problematic apps**:
103+
104+
- `react-router-7-lazy-routes` (the failing one)
105+
- `react-router-7-framework`
106+
- Then expand to remaining ~106 apps
107+
108+
### Phase 4: Cleanup
109+
110+
8. **After all apps migrated**:
111+
112+
- Remove old event proxy code from `event-proxy-server.ts`
113+
- Update documentation
114+
- Remove unused dependencies
115+
116+
## Key Technical Details
117+
118+
### Port Extraction from Spotlight
119+
120+
Spotlight outputs startup info to stderr. We parse it to get the dynamic port:
121+
122+
```
123+
Spotlight listening on http://localhost:8969
124+
```
125+
126+
### DSN Workaround Format
127+
128+
Per [Spotlight docs](https://spotlightjs.com/docs/cli/run/#unsupported-sdks-dsn-workaround):
129+
130+
```javascript
131+
Sentry.init({
132+
dsn: `http://spotlight@localhost:${process.env.SPOTLIGHT_PORT}/0`
133+
});
134+
```
135+
136+
### Playwright Config Changes
137+
138+
```javascript
139+
// Before
140+
webServer: [
141+
{ command: 'node start-event-proxy.mjs', port: 3031 },
142+
{ command: 'yarn start', port: 3030 }
143+
]
144+
145+
// After - spotlight run auto-detects and runs the app
146+
webServer: [
147+
{ command: 'yarn spotlight run -p 0 -f json', port: 3030 }
148+
]
149+
```
150+
151+
Note: `spotlight run` automatically:
152+
153+
- Detects the start script from package.json (`dev`, `develop`, `serve`, `start`)
154+
- Starts the Spotlight sidecar on the specified port (or dynamic with `-p 0`)
155+
- Streams events to stdout in JSON format (with `-f json`)
156+
- Sets `SENTRY_SPOTLIGHT` env var for the child process

dev-packages/test-utils/src/event-proxy-server.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,9 +150,10 @@ export async function startProxyServer(
150150

151151
eventCallbackListeners.add(callbackListener);
152152

153-
// Prune old events from the buffer that are definitely stale (older than this listener).
153+
// Prune old events from the buffer that are definitely stale (strictly older than this listener).
154+
// Use strict < to match the live event behavior (>= at line 103) - same-ms events are kept.
154155
// This prevents memory buildup and ensures old events can never leak to future listeners.
155-
while (eventBuffer.length > 0 && eventBuffer[0].timestamp <= listenerTimestamp) {
156+
while (eventBuffer.length > 0 && eventBuffer[0].timestamp < listenerTimestamp) {
156157
eventBuffer.shift();
157158
}
158159

0 commit comments

Comments
 (0)