search

StackExchange / StackExchange.Redis

General purpose redis client
https://stackexchange.github.io/StackExchange.Redis/
Other
5.91k stars 1.51k forks source link

ISubscriber no longer mockable #969

Open BrennanConroy opened 6 years ago

BrennanConroy commented 6 years ago

With the introduction of ChannelMessageQueue Subscribe(...) and the async version, ISubscriber is no longer mockable (because of sealed class ChannelMessageQueue) so it's difficult/impossible to unit test code that uses Redis.

mgravell commented 6 years ago

Fair point. I'll think about how we can re-expose this.

On Mon, 8 Oct 2018, 05:39 BrennanConroy, notifications@github.com wrote:

With the introduction of ChannelMessageQueue Subscribe(...) and the async version, ISubscriber is no longer mockable (because of sealed class ChannelMessageQueue) so it's difficult/impossible to unit test code that uses Redis.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/StackExchange/StackExchange.Redis/issues/969, or mute the thread https://github.com/notifications/unsubscribe-auth/AABDsDcyog3lJwh10qMwSmeJ-K0Mtrspks5uitcagaJpZM4XMPlZ .

analogrelay commented 6 years ago

A really awesome way to expose this for testing would be if we could wrap a ChannelReader<RedisMessage> up into a ChannelMessageQueue

analogrelay commented 6 years ago

Any update here? We'd like to be able to restore our testing here :). We're happy to help contribute something if it makes sense. My initial design thought is to make it possible by having ChannelMessageQueue be constructable from a ChannelReader<ChannelMessage>

NickCraver commented 6 years ago

@anurse I'm not 100% sure I follow there - if you were able to PR what you mean, could be an instant win (as long as it's not breaking of course) - have time to by chance?

analogrelay commented 6 years ago

Opened PR #1000 (lucky me! do I get a prize?) with a sketch of my idea.

I realized it was a little more severe than just a "fix for testability" once I realized that ISubscriber is not implementable anymore because ChannelMessageQueue can only be constructed via an internal constructor. Since ISubscriber is public, this seems pretty bad.

hansarnevartdal commented 5 years ago

Would at least be beneficial to enable unit testing on the channel message handler called by the channel message queue, but this uses the ChannelMessage struct with internal ctor, so not really doable as is.

viktors-telle commented 2 years ago

Are there any plans to resolve this issue?

kmcclellan commented 12 months ago

Agreed. Is the problem with extracting an IChannelMessageQueue simply because it is breaking? I'd love to see that in v3.

kmcclellan commented 12 months ago

FWIW this isn't just about testing. I had wanted to implement an ISubscriber that wraps an inner subscriber using Polly retries, and was stymied by this limitation.