-
Notifications
You must be signed in to change notification settings - Fork 24
Conversation
Closes #54
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there are some lint issues but otherwise LGTM.
Grr...one failing test! |
…same connection is sufficient for the intent of the test
|
||
// Ensure there's still a connection and it's marked as in the keep-alive | ||
// state. | ||
expect(connection.isInKeepAlivePeriod, isTrue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@grouma - Do you think that the round tripping of the value 'bar'
through the same connection
object as was used before and already closed should be sufficient for this test?
I could add a stream of reconnect events, but I don't think it's necessary to demonstrate correctness here.
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's probably fine since closeSink
is synchronous and should guarantee that the connection is severed before we send bar
.
Closes dart-lang/sse#54 Fix formatting in one file. Remove test expectations around keep alive periods. Testing communication through the same connection that had been closed is sufficient for the intent of the test.
Closes dart-lang/tools#1648