feat(testing): FakeBroadcastManager records listeners and can dispatch to them - #119
Merged
Merged
Conversation
…h to them
`_FakeBroadcastChannel.listen()` returned `this` and dropped both the event name
and the callback. So the one line every realtime feature depends on was the one
line no consumer could cover: delete `..listen('order.shipped', handler)` from an
application and its entire suite stays green, because `assertSubscribed` only
proves a channel was opened, not that anything is listening on it.
Found while a consuming app was hunting a vacuous test. Two independent reviews
landed on the same line, and neither could suggest a fix that did not start here.
The registry lives on the driver rather than the channel, and that is forced: the
channel factories mint a NEW channel object per call and keep no reference, so a
record held on the channel vanishes as soon as the caller lets go of it.
Adds:
- `assertListening(channel, event)` / `assertNotListening(channel, event)`, in the
existing idiom (AssertionError, message naming what was registered instead)
- `dispatch(channel, event, data)`, which runs the handler with a decoded payload
exactly as the driver would on a frame, and throws when nothing is listening
because dispatching into silence is the failure it exists to reveal
- `driver.listeners` for low-level inspection
- `stopListening()` now actually unregisters, and `reset()` clears the registry
A second `listen()` for one event REPLACES the first, matching
ReverbBroadcastDriver, which cancels the previous subscription before storing the
new one. A fake that appended would hide a double-registration bug rather than
reproduce it.
Red phase measured: restoring the old discard turns four of the eight new tests
red. 1290 tests pass, analyze clean.
There was a problem hiding this comment.
Pull request overview
This PR enhances the broadcast testing fake (FakeBroadcastManager / FakeBroadcastDriver) so listener registrations are actually recorded and testable, and so tests can dispatch events to registered handlers—closing a gap where listen() previously discarded its inputs and made “deleted listen line” regressions invisible to consumer test suites.
Changes:
- Add listener-focused testing helpers:
assertListening,assertNotListening, anddispatch. - Record event listeners in
FakeBroadcastDriver(with replace-on-duplicate semantics) and implementstopListening()+reset()behavior accordingly. - Add/extend unit tests validating listener registration, dispatch behavior, replacement semantics, stopListening, and reset; update contributor docs accordingly.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/testing/fake_broadcast_manager_test.dart | Adds coverage for listener registration, dispatch behavior, replacement semantics, stopListening, and reset. |
| lib/src/testing/fake_broadcast_manager.dart | Implements listener registry + assertions/dispatch, and wires channel instances to register/unregister handlers. |
| .claude/rules/broadcasting.md | Documents the new FakeBroadcastManager testing surface and clarifies the “subscribed vs listening” trap. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
anilcancakir
added a commit
to anilcancakir/uptizm
that referenced
this pull request
Aug 9, 2026
…suming it
The sixth vacuous assertion this feature produced, and the one on the single line
the whole realtime half depends on. `assertSubscribed` proves a channel was
OPENED and nothing more, so deleting `..listen('analyze.progress', …)` from
_reconcileSubscription left this entire suite green. Two independent reviews
landed on exactly that line and neither could suggest a fix that started here:
magic's fake channel discarded both the event name and the callback, so there was
nothing to assert against.
Fixed upstream first (fluttersdk/magic#119, merged), then used here. Two tests:
- all four events this service depends on are registered, not just the channel
- a real frame, dispatched the way the driver dispatches one, runs the handler and
arrives with its payload intact
The second matters more than it looks. Every other test in this feature calls
noteAnalyzeProgress directly, which is honest about its own subject and proves
nothing about the socket path; with the 2500ms poll masking a dead socket, no gate
at any layer covered it.
Red phase measured against magic master, not against the local branch: deleting
the listen line turns both new tests red where the suite was previously green.
bin/check green across all seven jobs.
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.
What
_FakeBroadcastChannel.listen()returnedthisand dropped both the event name and the callback. So the one line every realtime feature depends on was the one line no consumer could cover:Delete it from an application and its entire suite still passes.
assertSubscribeddoes not catch it: subscribing to a channel and listening for an event are separate steps, and it only proves the first. An app can hold a live channel and register nothing at all.Why it came up
A consuming app was hunting a vacuous test on exactly that line. Two independent reviews landed on it, and neither could propose a fix that started anywhere but here, since the fake channel is private and the package is a dependency.
Verified against that consumer before opening this, using the local path dependency: adding
assertListeningfor its four events plus onedispatchturned a deleted..listen('analyze.progress', …)line from green into two failing tests.The change
The registry lives on
FakeBroadcastDriver, not on the channel, and that is forced rather than stylistic:channel()/private()/join()mint a new channel object on every call and keep no reference, so a record held on the channel vanishes the moment the caller lets go of it.assertListening(channel, event)listen()line is deletedassertNotListening(channel, event)dispatch(channel, event, data)driver.listeners.subscribedChannelsstopListening()now actually unregisters (it was an empty body), andreset()clears the registry with everything else.Two behaviours that are decisions rather than accidents:
dispatch()throws when nothing is listening. Dispatching into silence is the exact failure this PR exists to reveal, so it must not pass quietly.listen()for one event REPLACES the first, matchingReverbBroadcastDriver, which cancels the previous subscription before storing the new one (reverb_broadcast_driver.dart:846-847). A fake that appended would hide a double-registration bug rather than reproduce it.Testing
flutter analyzeclean.flutter test1290 pass, 8 of them new.Red phase measured, because a test for a testing helper is precisely where a vacuous assertion hides: restoring the old discard turns 4 of the 8 red. The 4 that stay green are the negative assertions and the empty-registry cases, which is correct and is why they are not the whole coverage.
Docs
.claude/rules/broadcasting.md'sFakeBroadcastManagersection lists the new surface and states the trap plainly, since that file is what a contributor reads before reaching forassertSubscribedand believing it covers this.Compatibility
Purely additive to the public surface. The one behaviour change to an existing method is
stopListening(), which went from a no-op to doing what its name says; nothing could have depended on the no-op, because there was nothing registered for it to remove.