-
Notifications
You must be signed in to change notification settings - Fork 263
Include create, reconnect, and update events (with associated tests) #273
Include create, reconnect, and update events (with associated tests) #273
Conversation
please feel free to provide your feedback and suggestions, especially with the tests; they're a top weakness of mine |
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 good, thanks for this.
Can I ask you change the events to StreamPreConnect and StreamPreDisconnect - sorry that wasn't captured in the issue originally.
no worries—pushed! |
pixelStreaming.disconnect(); | ||
pixelStreaming.connect(); | ||
|
||
expect(streamReconnectSpy).toHaveBeenCalled(); |
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.
This appears to be failing tests. I believe because reconnect isn't being called since connect is manually immediately called.
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.
ah, my mistake, thank you for fix!
…d-CRD-events Include create, reconnect, and update events (with associated tests)
…d-CRD-events Include create, reconnect, and update events (with associated tests)
Relevant components:
Problem statement:
#270
Solution
uses contextual structure of methods and events to add the new events
Documentation
WIP
Test Plan and Compatibility
event emitters should not affect functionality; implementation of these functions are complemented by the new tests