Getting the test suite in shape to match the phoenix js one #4
Conversation
This file contains 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
Phoenix calls it connect and disconnect and their events are open and close (not opened and closed)
30 tasks
The test at https://github.com/phoenixframework/phoenix/blob/2e67c0c4b52566410c536a94b0fdb26f9455591c/assets/test/socket_test.js#L465 demonstrates this. One can double check the number at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER and learn an explanation about why it exists
… the big todo list
…t instead of soon
This would leave items added before joining in pending until some other push happened
I feel this is now very very close to the javascript version
This doesn’t clean up as much as I had hoped. I basically already did the work to delay connecting because I wanted to be able to setup a subscription before connecting. The reason for wanting that is that the connection might happen so fast that I miss the first message. Either way, this is the way they like to do it and we do get a nice autoconnect() method for free, so why not. /cc @atdrendel
* Always do throws * Use autoconnect where it makes sense * Added tests to prove that cancelling the autoconnected subscriber calls disconnect indirectly /cc @atdrendel
…m socket’s list of channels
|
@myobie This is ready to go in. The tests are pretty reliable on my local machine. They aren't perfect on CI, but I'm confident in the overall behavior of the socket. |
The tests being there is more important than them passing IMO. 👍🏻🙏 |
atdrendel
added a commit
that referenced
this pull request
Jul 1, 2020
…ol (#2) * Swift package and v2 upgrade first draft * Remove unecessary double Phoenix namespace and what was called Phoenix is now Socket * Socket state machine is closer to the js one * Constants (enums) are now like the js version * Comment out tests for now * Better tracking of joined channels * Throw away stale messages * Comment out more tests for now * Fixup mistakes around joinRefs * Better re-joining after a re-connect * Add comment * The WebSocket will be a producer, the Channel will be a Subscriber, and the Socket will be the middle-class * Test that WebSocket works * Boot the mix server in setUp/tearDown * I am so confused * Every file * Update all tests to use the new test helper * Use Synchronized and Forever packages * Fix up tests * Update dependencies * More data modeling * Basic tests pass * Unify test structure * Created SimplePublisher and Channel is now a publisher * Add some extra APIs for testing channel behavior * Moar kinds of push() on Channel * Make sure a reply is event type reply * Add some amazing tests * Add a way to tell the server we want to disconnect soon * Socket builds the URL Made it easier to append query items to a URL * Failing test for receiving close message from Socket * Will need to know if we should try to reconnect * Got test URLs messed up * Fix a bug where _task might disappear * Socket is a Publisher and publishes .closed message * Trying to be fancy with sync isn't a good idea * Only try to start the phoenix app if we are macOS * Big refactor * Pipeline is now linear: WebSocket → Socket → Channel * The anticipated public API is still very much "use Channel and push / sink / forever there" but now the Socket also becomes a place one can sink / forever * Socket publishes all incoming messages, open, and close events * Pusher * Channel pushes into a Pusher class * Socket polls the Pusher for the next batch of messages to write * The receiveBatch callback is similar to the WebSocket URL Task one in that you get one batch at a time and must re-call it after processing the batch * This is beneficial because the Pusher can use `sync` everywhere and that keeps everyone in line for appending/receiving (it's the join point) * Latest synchronized doesn't raise exception with wrap-around anymore * Use the latest version of Synchronized * Don't try to boot the example web app * Update Forever to latest * Only use the XCTestException and not the manual wait * Publish out of Socket for all possible types instead of silently printing and failing * Use the correct expectation factory so failure propigate threads * Things are messed up * Simplify sync logic * Install and use SimplePublisher * Start an Action * Make a simple start script for CI * chmod +x start * Try to boot the server as a service * Maybe the options is a string * Try to just install and run on mac * Ah, forgot to force install hex * Install rebar as well * Try to pre-compile the app * Forgot the and * Mix isn't in test env * Remove SimplePublisher * Shore up the error stuff so tests pass * Let the expectation manage the counting * Rewrite the repeat test to be more sensible * OK, this test is now way more accurate This has been wrong the whole time... * Showing that SimplePublisher is a drop in from what we had * Fixup tests to share a single helper and therefore a ref generator Also deal with the fact that apple will tell me the web socket is closed twice * Better expectation message * Rejoins a channel after reconnecting to the socket * Stay focused and only test what you are testing * I mean for real * Wait a little bit longer * Print the event * Make WebSocket internal * Add a failing test (skip for now) for not rejoining a left channel * Removing the pusher entirely, but not done * Compiles once again * Use a weak var for the internal subscriber so we don't have so many things hanging onto the Socket * DelegatingSubscriber * Now Channel doesn't rejoin but the leave events are broken * Getting the test suite in shape to match the phoenix js one (#4) * Skip the failing join/leave event test for now * Don’t ever send a completion through the Socket subject * Implement init and connection tests for Socket * Rename connection related functions and messages Phoenix calls it connect and disconnect and their events are open and close (not opened and closed) * Rename test to indicate scope * Socket doesn’t connect on init * Socket state now conforms to the same names and principles as the js * Socket can produce channels * Can join a channel even if disconnected and can push directly onto an open socket * Socket has a pending buffer * Restart refs after the maximum safe integer for Javascript The test at https://github.com/phoenixframework/phoenix/blob/2e67c0c4b52566410c536a94b0fdb26f9455591c/assets/test/socket_test.js#L465 demonstrates this. One can double check the number at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER and learn an explanation about why it exists * Wait longer for the server to start in CI * Fixup the ref and ref tests to link to the docs for why * Heartbeat tests * Flushes pending in socket on open * Build a quick javascript socket page for testing * Channels now error when the socket closes from the server-side * Skip a channel test for now until I make it to the Channel section of the big todo list * Test to not error a channel that explicitly left if the remote closes * Update dependencies for the test phoenix app * Found a way to have a socket level message and using it for disconnect instead of soon * Remove all uses of the soon query param from the Socket tests * Explain the custom socket command for disconnecting * Fix a bug where a channel doesn’t flush right after joining This would leave items added before joining in pending until some other push happened * Final tests for Socket I feel this is now very very close to the javascript version * Make Socket a ConnectablePublisher This doesn’t clean up as much as I had hoped. I basically already did the work to delay connecting because I wanted to be able to setup a subscription before connecting. The reason for wanting that is that the connection might happen so fast that I miss the first message. Either way, this is the way they like to do it and we do get a nice autoconnect() method for free, so why not. /cc @atdrendel * Prettify SocketTests * Always do throws * Use autoconnect where it makes sense * Added tests to prove that cancelling the autoconnected subscriber calls disconnect indirectly /cc @atdrendel * Unskip test * Completely seperate subscriber and publisher typealiases And use Never for the failures of Socket and Channel * Remove Result from Channel and add error to Channel.Event * Cleanup channel test a bit * Beginning the ChannelTests comformation with the js * Channel is joining after push * No longer need the “disconnect soon” connection params in UserSocket example phoenix app * Joining twice is a no-op * Can confirm join params make it to the server Refactored where the channel asks the socket to subscribe itself in init Also added an assigns for the join_params so I can then ask for it later with a handle_in. So far, having these message passing ways is better than anything else for testing state like this. * Can have a custom join timeout for a channel And more overloads for push to complete the set * Skip old tests Also just always call disconnect on tearDown * Begining of timeout tests * Extract all enums, structs, and classes each to their own file Also sort imports * Fix unused variable in example elixir app * Call completion handler on send if could not serialize outgoing message * Go ahead and mark as closing so the next send fails immediately Instead of waiting for the WebSocket to async notify the Socket * Forgot the Error case * joinPayload doesn’t need to be public * Need to set the state to closing for failures in both send methods And added a comment that this behavior needs to be tested * Add tolerance to the heartbeat timer for niceness * Call completion handlers after state is updated * Remove unused function * Uh, this flush stuff is overly complicated for no good reason * Reformat this code to match whats below * Re-arrange order of events for heartbeat timeout * Socket init doesn’t need to throw, Channel now subscribes to all Socket events The channel should be notified when the socket state changes, so mapping socket messages into channel interesting messages Cleaned up all instances where try Socket() Channel Pushes have a timeout timer Channe joinPush is written when socket open is received * Only accept a join reply if it’s an ok status 😞 * Handle socket open and close for all the channel states * All tests pass now except for the channel join timeout one, which is what I intended * Organize channel methods a bit before adding new timeout code * Timers suck, I’ll remove them soon * OK, let’s try to make my own expectation framework Now I need to download XCode 11.4 because it should work better with the throw-y-ness of what I made /cc @atdrendel * OK, my own wait and Expectation works well enough to move forward * Move this var up * It doesn’t work * Remove custom test framework * Use the new Timer which uses DispatchSourceTimer * Channel join timeout and backoff works * Uncomment old tests as they now work again 🙌🏻 * Rename example to server * Add a helper script to run the server and add information about running tests to README.md * Update GitHub Action * Add link to available macOS environments * Update path to Phoenix server * Rename Example to Server * Replace Channel rejoin timeout with a function * Whitespace * Fix spelling mistake * Modify timeouts * Fix warning * Reduce running time of ChannelTests * Extract asserting on open in SocketTests * Follow example set by Starscream daltoniam/Starscream@990a4c8#diff-015c4d5ea3574374329ed92ced271cedR24-R28 * Add RawCaseConvertible * Add more test helpers to SocketTests * Add a couple tests to illustrate differences of the autoconnect subscription * Simplify SocketTests * Update comment in ChannelTests * Set .connecting state before sending .connecting message * Disconnect when Socket deinits * Set .connecting state when connecting WebSocket * Fix two tests to be less flaky * Make sure the channels in a test are retained for the entire test * Extend timeout for CI for testJoinRetriesWithBackoffIfTimeout * Add SocketTests.testSocketIsClosed() * Add link to JavaScript test * Start adding socket test coverage * Update server dependencies * Fix Phoenix.PubSub warning * Improve readability of testSocketAutoconnectSubscriberCancelDisconnects() * Fix whitespace * Fix WebSocket leak WebSocket never deallocated itself because: 1. The session never invalidated itself. 2. The WebSocket tasks held a strong reference to WebSocket. * Make WebSocket.delegateQueue serial * Remove SimplePublisher * Add some additional socket tests * Make WebSocket delegate queue serial * Simplify public interface of Socket * Continue working on SocketTests * Upgrade broken plug_cowboy * Continue adding and improving SocketTests * Remove ineffective “boom” command from user_socket * Re-add “boom” handling in user_socket.ex * Implement socket reconnection logic and tests * Make Channel’s background queue synchronous * Remove global Ref.Generator and make Channel user Socket’s generator * Add hacky XCTestCase.expectationWithTest() * Make Socket.subject let instead of var * Send all messages to Socket.subject on notifySubjectQueue * Improve reliability of testSocketIsConnecting and testSocketIsClosing * Move test helpers to XCTestCase+Phoenix * Rename ChannelJoinLeaveTimer to ChannelJoinTimer * Remove Comparable conformance from PushedMessage and replace with Sequence.sortedByTimeoutDate() * Simplify Channel.joinTimer closes #11 * Improve reliability of testJoinRetriesWithBackoffIfTimeout() * Speed up SocketTests.testSocketIsConnectedEvenAfterSubscriptionIsCancelled * Move test helpers to XCTestCase+Phoenix * Add Topic typealias * Upgrade to synchronized v2.0.0 * Clean up comments and some Socket tests * Try to improve reliability of testJoinRetriesWithBackoffIfTimeout() * Clean up tests * Clean up and add channel tests * Add socket_test.js links to socket-test-coverage.md * Remove unused Atomic * Add room:error Phoenix channel * Add more channel tests * Remove joinedOnce * Fix unreliable tests * Add ChannelTests.testDoesNotSendAnyBufferedMessagesAfterJoinError() * Fix expectError expectation name * Add onError channel tests * Replace Forever in tests * Remove Forever from phoenix-apple * Properly notify subscribers of Channel closing and remove channel from socket’s list of channels * Extract duplicated code into sendLeaveAndCompletionToSubjectAsync() * Add start-phoenix-js command to test `phoenix-js` * Add tests to channel-test-coverage.md * Add more channel tests * Add more channel tests * Finish implementing all required tests for the channel * Finish all leaving tests * Clean up tests * Try to fix ChannelTests.testJoinRetriesWithBackoffIfTimeout() * Try to fix ChannelTests.testJoinRetriesWithBackoffIfTimeout() * Remove unnecessary print() statements * Remove noisy print() statement Co-authored-by: Anthony Drendel <[email protected]> * Try to fix ChannelTests.testJoinRetriesWithBackoffIfTimeout() * Ensure the proper amount of backoff has occured * Remove test too vulnerable to race conditions * Increase timeout for ChannelTests.testJoinRetriesWithBackoffIfTimeout() * Increase timeout to ensure leave() is called * Reduce unnecessary wait timeout * Re-increase timeout for ChannelTests.testJoinRetriesWithBackoffIfTimeout() * Last try at making ChannelTests.testJoinRetriesWithBackoffIfTimeout() reliably pass Co-authored-by: Anthony Drendel <[email protected]>
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.
Javascript source for phoenix: https://github.com/phoenixframework/phoenix/blob/v1.5/assets/js/phoenix.js
Unit tests
Socket
Defaults
URLwith an extension to make it easier to append query components so I believe this case is covered by thisConnecting and disconnecting
connectmethod exists and worksdisconnectmethod exists and worksdisconnectmethod supports a completion handler callback – since one can subscribe for the close event, I don't think we need to provide a direct callbackdisconnectpublishes acloseeventdisconnectwhen not connected without exceptionConnection state
Channel (from the perspective of a Socket)
Push
ref(joinRefin this case isnil)Ref
refmakeRefHeartbeat
Send buffer
These two will be tested indirectly and would require me to un-private some methods that I don't want to un-private.
On open
On close
Connection error
Messages
Channel
init
joinPush
join
sets joinedOnceWe are going to diverge here – keeping track of our joined state is already managed in thestate. Also, joining twice should no-op for us just as the Socket.connect no-ops (will also mention below). We regenerate the joinRef and it is stored in the state enum, so we know that we won't receive messages intended for an older or newer instance of Channel.does not allow to join multiple timesWe are diverging and going with double join is a no-op.timeout
error
error / backoff / rejoin
close
phx_closePublishes messages
canPushcanPushboolean which is true when joined and false in other statesPub/sub
Push
leave
Also take a look at https://github.com/phoenixframework/phoenix/blob/master/test/phoenix/integration/websocket_channels_test.exs and https://github.com/phoenixframework/phoenix/blob/master/test/phoenix/integration/websocket_socket_test.exs