Skip to content
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

Adding a new DevBackend for testing. #63

Merged
merged 2 commits into from
Apr 20, 2023
Merged

Adding a new DevBackend for testing. #63

merged 2 commits into from
Apr 20, 2023

Conversation

jwoertink
Copy link
Collaborator

Fixes #61

This PR adds in a new Cable::DevBackend class that can be used for testing to ensure that your channels are called when you expect.

In order to do this, I had to rename the current Backend classes because they were both named Cable::Backend, and that wouldn't let you figure out if you were using redis or just this in-memory dev backend. We will need the separate naming anyway in order to do #49 eventually.

I ended up moving some code from the connection_spec.cr in to helpers because as I was writing my spec, I thought I needed them, but turns out I didn't. I figure they belong in the spec/support anyway.

Lastly, in order to test, this requires setting the @@server in Cable to nil because we were calling Cable.restart before every spec. When that's called, Cable.server is defaulted to the Redis backend, and I need the DevBackend for my spec.

This also means that potentially other devs will need to do the same... I'm not a huge fan of that, but I also don't know another way to handle that.

This is a small breaking change for those that are using the legacy redis backend. If you are, you'll have to set settings.backend_class = Cable::RedisLegacyBackend in your Cable config for it to still work. This is officially deprecated now.

@jwoertink jwoertink added the BREAKING CHANGE This includes a change that breaks backwards compatibility label Apr 9, 2023
@jwoertink jwoertink requested a review from fernandes April 9, 2023 00:54
Comment on lines +34 to -45
setting backend_class : Cable::BackendCore.class = Cable::RedisBackend, example: "Cable::RedisBackend"
setting redis_ping_interval : Time::Span = 15.seconds
setting restart_error_allowance : Int32 = 20
setting on_error : Proc(Exception, String, Nil) = ->(exception : Exception, message : String) do
Cable::Logger.error(exception: exception) { message }
end

# DEPRECATED
# only use if you are using stefanwille/crystal-redis
# AND you want to use the connection pool
setting pool_redis_publish : Bool = false
setting redis_pool_size : Int32 = 5
setting redis_pool_timeout : Float64 = 5.0
setting redis_ping_interval : Time::Span = 15.seconds
setting restart_error_allowance : Int32 = 20
setting on_error : Proc(Exception, String, Nil) = ->(exception : Exception, message : String) do
Cable::Logger.error(exception: exception) { message }
end
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just moved these up so we can group which settings are deprecated and which are still in use.

Copy link
Collaborator

@fernandes fernandes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is pretty cool, I won't merge waiting for if @mjeffrey18 wants to add something once worked on this backend, but looks 💯

Copy link
Contributor

@mjeffrey18 mjeffrey18 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@jwoertink jwoertink merged commit fb45456 into master Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAKING CHANGE This includes a change that breaks backwards compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How do we test this stuff??
3 participants