Skip to content

Conversation

@mattt
Copy link
Owner

@mattt mattt commented Oct 31, 2025

This PR attempts to fix an apparent race condition indicated by a flakey integration test by making the onMessage callback async and awaiting that before proceeding with reconnection or completion.

It also adds a new maximumFinalizationEventCount property (default = 100) to prevent unbounded memory growth when handling events sent by the server during finalization.

@mattt mattt requested a review from Copilot October 31, 2025 09:22
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the test suite and updates the EventSource API to make callbacks async. The main changes include:

  • Test function naming: Removes "test" prefix from test function names across all test files, aligning with Swift Testing conventions
  • Async callbacks: Changes EventSource callbacks (onOpen, onMessage, onError) from synchronous to async functions
  • Event delivery: Adds explicit event delivery loop after parser.finish() to ensure pending events are delivered with a safety limit of 100 events
  • Test organization: Reorganizes IntegrationTests.swift into clearer nested suites: EventSourceAPITests and URLSessionBytesAPITests

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
Tests/EventSourceTests/ParserTests.swift Renamed all test functions to remove "test" prefix for consistency with Swift Testing conventions
Tests/EventSourceTests/IntegrationTests.swift Renamed tests, reorganized into nested suites, updated to use async callbacks, removed unused ErrorTracker actor
Tests/EventSourceTests/AsyncEventsSequenceTests.swift Renamed all test functions to remove "test" prefix
Sources/EventSource/EventSource.swift Changed callbacks to async, added event delivery loop after parser.finish(), removed some explanatory comments

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@mattt mattt merged commit 6167d0f into main Oct 31, 2025
3 checks passed
@mattt mattt deleted the mattt/async branch October 31, 2025 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants