New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Action Cable] Add `WebSocket` and `logger` configuration options #25170

Merged
merged 3 commits into from May 31, 2016

Conversation

Projects
None yet
3 participants
@maclover7
Member

maclover7 commented May 27, 2016

[Javan Makhmali, Jon Moss]

r? @javan

@maclover7 maclover7 changed the title from Add configuration cable to Add WebSocket and logger configuration options May 27, 2016

@maclover7 maclover7 changed the title from Add WebSocket and logger configuration options to [Action Cable ] Add `WebSocket` and `logger` configuration options May 27, 2016

@maclover7 maclover7 changed the title from [Action Cable ] Add `WebSocket` and `logger` configuration options to [Action Cable] Add `WebSocket` and `logger` configuration options May 27, 2016

@maclover7 maclover7 force-pushed the maclover7:add-configuration-cable branch May 27, 2016

@javan

View changes

actioncable/test/javascript/src/unit/action_cable_test.coffee Outdated
test 'configurable', (assert) ->
ActionCable.logger = ''
assert.equal ActionCable.logger, ''

This comment has been minimized.

@javan

javan May 27, 2016

Member

How about grouping all of these new tests in one module "Adapters"? And, I know this is nit picky, but let's use double quotes everywhere to match the existing style.

@javan

View changes

actioncable/test/javascript/src/test_helpers/mock_websocket.coffee Outdated
@@ -18,4 +16,3 @@ QUnit.testDone ->
if server?
server.clients().forEach (client) -> client.close()
server.close()
window.WebSocket = NativeWebSocket

This comment has been minimized.

@javan

javan May 27, 2016

Member

I think we should drop this file entirely swap out WebSocket as needed.

module "<Any group of tests that use WebSocket>", (hooks) ->
  originalWebSocket = ActionCable.WebSocket

  hooks.beforeEach ->
    ActionCable.WebSocket = MockWebSocket

  hooks.afterEach ->
    ActionCable.WebSocket = originalWebSocket

This comment has been minimized.

@javan

javan May 27, 2016

Member

^ Not a complete replacement there since it doesn't incorporate MockServer.

This comment has been minimized.

@maclover7

maclover7 May 27, 2016

Member

👍 how does eeed98c look?

This comment has been minimized.

@javan

javan May 28, 2016

Member

Wrapped that up even more maclover7#3

@maclover7 maclover7 force-pushed the maclover7:add-configuration-cable branch 2 times, most recently May 27, 2016

@maclover7 maclover7 force-pushed the maclover7:add-configuration-cable branch to 410a32f May 31, 2016

@jeremy jeremy merged commit 410a32f into rails:master May 31, 2016

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

jeremy added a commit that referenced this pull request May 31, 2016

Merge pull request #25170 from maclover7/add-configuration-cable
[Action Cable] Add `WebSocket` and `logger` configuration options

@maclover7 maclover7 deleted the maclover7:add-configuration-cable branch May 31, 2016

@maclover7

This comment has been minimized.

Member

maclover7 commented May 31, 2016

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment