Skip to content

Conversation

@xlc
Copy link
Member

@xlc xlc commented Aug 3, 2025

Summary of Changes:

  • Adds a new test suite, EventBusSubscribingTests.swift, to provide comprehensive test coverage for the EventBus component.
  • The new tests cover a wide range of scenarios for subscribing and unsubscribing, including:
    • Basic subscription and event reception.
    • Unsubscribing to stop receiving events.
    • Multiple subscribers for a single event.
    • Subscribing to different event types.
    • Unsubscribing one of multiple handlers.
  • Includes tests for both eventMiddleware and handlerMiddleware to ensure they are processed correctly.
  • Adds tests for the EventSubscriptions helper class, verifying automatic unsubscription on deinitialization and explicit unsubscription.

Highlights of Changed Code:

  • New File: Utils/Tests/UtilsTests/EventBus/EventBusSubscribingTests.swift
    • Implements testBasicSubscription to verify a handler is called.
    • Implements testUnsubscribe to ensure a handler is no longer called after unsubscribing.
    • Implements testMultipleSubscribers to confirm all handlers receive an event.
    • Implements testEventMiddleware to check if middleware can modify an event.
    • Implements testHandlerMiddleware to count handler invocations via middleware.
    • Implements testEventSubscriptionsDeinit to ensure automatic cleanup.

Adds a new test suite for the EventBus, covering:
- Basic subscription and unsubscription logic.
- Handling of multiple subscribers and different event types.
- Event and handler middleware functionality.
- Behavior of the `EventSubscriptions` helper, including deinit and explicit unsubscription.
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

The new tests rely on fixed-duration Task.sleep calls for synchronization, which can lead to flaky and slow tests. This anti-pattern should be replaced with a more robust mechanism, such as polling for a condition to be met, to ensure test reliability and improve execution speed.

await eventBus.publish(testEvent)

// Give some time for the event to be processed
try await Task.sleep(for: .milliseconds(100))
Copy link

Choose a reason for hiding this comment

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

Using Task.sleep with a fixed duration to wait for an asynchronous operation to complete makes the test both slow and unreliable. If the operation takes longer than the sleep duration, the test will fail incorrectly (flakiness). A better approach is to actively wait for the desired condition to become true.

This issue is present in all tests in this file. Please replace the Task.sleep calls with a more deterministic mechanism.

For example, you could introduce a polling helper function:

private func waitUntil(timeout: TimeInterval = 1.0, condition: @escaping () async -> Bool) async throws {
    let deadline = Date.now.addingTimeInterval(timeout)
    while await !condition() {
        if Date.now >= deadline {
            struct TimeoutError: Error {}
            throw TimeoutError()
        }
        try await Task.sleep(for: .milliseconds(10))
    }
}

And then use it in the test like this, removing the Task.sleep:

await eventBus.publish(testEvent)

try await waitUntil { await handlerChecker.callCount == 1 }

let callCount = await handlerChecker.callCount
#expect(callCount == 1)

@xlc xlc merged commit 405f8b3 into master Aug 4, 2025
3 checks passed
@xlc xlc deleted the eventbus-test branch August 4, 2025 03:19
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