Add StreamGroup.onIdle and StreamGroup.isIdle #164
Conversation
These match the corresponding APIs on FutureGroup.
nex3
force-pushed
the
stream-group-on-idle
branch
from
89ad01d to
419b7d9
Compare
test/stream_group_test.dart
Outdated
|
|
||
| test('emits an event when the group closes', () async { | ||
| // It's important that the order of events here stays consistent over | ||
| // time, since code may rely on it in subtle ways. |
There was a problem hiding this comment.
I don't think we should have any enforced ordering between the stream done and the onIdle done.
cc @lrhn for thoughts on testing behavior that we don't want to make guarantees about.
There was a problem hiding this comment.
I think there's a difference between "publicly making a guarantee that the behavior will remain consistent", and "internally trying to ensure that the behavior will remain consistent". I agree that you shouldn't advertise this behavior, or consider it part of the public API; however, I think it's worth doing what you can to avoid breaking it accidentally and thereby risking a bunch of subtle bugs without any benefit.
There was a problem hiding this comment.
Honest answer: I'd rather randomize things that people shouldn't depend on, so they can learn to not depend on them. It's a pain to change anything timing related because people have written bad code depending on the implementation, not the specification.
In practice, I'm fine testing that the implementation does what we expect the implementation to do, as long as it's clearly documented that a change of behavior is fine as long as you also change the test. It's often easier to test the actual behavior than test "any of a number of correct behaviors", you'll likelyend up missing some technically correct behaviors anyway.
There was a problem hiding this comment.
Added more explicit documentation.
test/stream_group_test.dart
Outdated
|
|
||
| test('emits an event when the group closes', () async { | ||
| // It's important that the order of events here stays consistent over | ||
| // time, since code may rely on it in subtle ways. |
There was a problem hiding this comment.
Honest answer: I'd rather randomize things that people shouldn't depend on, so they can learn to not depend on them. It's a pain to change anything timing related because people have written bad code depending on the implementation, not the specification.
In practice, I'm fine testing that the implementation does what we expect the implementation to do, as long as it's clearly documented that a change of behavior is fine as long as you also change the test. It's often easier to test the actual behavior than test "any of a number of correct behaviors", you'll likelyend up missing some technically correct behaviors anyway.
lib/src/stream_group.dart
Outdated
| /// single-subscription group). | ||
| /// | ||
| /// Note that events won't be emitted on this stream until [stream] has been | ||
| /// listened to. |
There was a problem hiding this comment.
Is it worth mentioning that the events are delivered asynchronously and the group can have become be non-idle when the event is delivered.
These match the corresponding APIs on FutureGroup.
These match the corresponding APIs on FutureGroup.