Fix Cypress crossfade/gapless failures: keep playback_session in the earliest event batch - #699
Merged
Merged
Conversation
The event sender dispatches at most 10 events per request. The crossfade and gapless e2e specs asserted on the first intercepted batch only, and the second track's playback_session event sat exactly at the batch boundary. The event timing changes in #697 shifted the enqueue order so it fell into the second batch, failing the specs on every PR. - Enqueue playback_session before the session's own playback_statistics and streaming_session_end events so the consumption-relevant event is never bumped to a later batch by its own session's metrics events. - Make the crossfade/gapless specs aggregate events across all intercepted batches (retrying until the second batch arrives) and include the seen event names in the assertion message. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses Cypress flakiness/regressions in the crossfade and gapless play-log specs by ensuring playback_session events are dispatched early enough to land in the earliest event batch, and by updating the specs to assert across all intercepted event batches (not just the first request).
Changes:
- Reordered end-of-session event sending so
playback_sessionis committed before the session’s streaming-metrics end events. - Updated crossfade/gapless Cypress specs to aggregate events across all intercepted event requests and include better assertion diagnostics.
- Documented the behavioral change in the player changelog.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/player/src/player/basePlayer.ts | Ensures playback_session is committed before auxiliary streaming-metrics events on session end to stabilize earliest-batch ordering. |
| packages/player/cypress/e2e/play-log/case-gapless.cy.ts | Aggregates events across all intercepted requests and retries until both playback_session events are observed. |
| packages/player/cypress/e2e/play-log/case-crossfade.cy.ts | Same aggregation/retry strategy as gapless to avoid asserting on only the first batch. |
| packages/player/CHANGELOG.md | Notes the updated enqueue/dispatch ordering for playback_session vs. streaming-metrics end events. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
After #697 merged, the Cypress
case-crossfade.cy.tsandcase-gapless.cy.tsspecs started failing on every open PR (expected 2 playback_session events, got 1). The specs never ran on #697 itself because it was stacked on the #696 branch and the Cypress workflow only triggers for PRs targetingmain.Root cause: the event sender dispatches at most 10 events per request (SQS batch limit), and the two specs asserted on the first intercepted batch only. A crossfade/gapless run produces enough events that the second track's
playback_sessionsat exactly at the 10-event batch boundary. The event enqueue-timing changes in #697 (serialized reducers, actual-start reporting) shifted the order so it fell into the second batch — which the specs never looked at.What
BasePlayer.eventTrackingStreamingEndednow enqueuesplayback_sessionbefore the session's ownplayback_statistics/streaming_session_endevents, so the consumption-relevant play_log event is never bumped to a later batch by its own session's metrics events.Verified
pnpm --dir packages/player typecheckandlint:cipass locally.Made with Cursor