-
Notifications
You must be signed in to change notification settings - Fork 8
When subscribing to already subscribed channel or group should not resubscribe #384
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
When subscribing to already subscribed channel or group should not resubscribe #384
Conversation
| } | ||
|
|
||
| @Test | ||
| fun whenSubscribingToAlreadySubscribedChannelShouldNotResubscribeButShouldEmitSubscriptionChangedStatus() { |
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.
It's a pretty long name. What about shouldEmitSubscriptionChangedWhenAlreadySubscribed or something similar?
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.
shouldEmitSubscriptionChangedWhenAlreadySubscribed misses important part that we want to make sure that there is no resubscribe.
Name is pretty long but I think it serve the purpose of properly describing what the test does.
The test is complex and IMHO it is good that name describe its behaviour.
What do you think?
...tlin-impl/src/integrationTest/kotlin/com/pubnub/api/integration/SubscribeIntegrationTests.kt
Outdated
Show resolved
Hide resolved
...tlin-impl/src/integrationTest/kotlin/com/pubnub/api/integration/SubscribeIntegrationTests.kt
Outdated
Show resolved
Hide resolved
...tlin-impl/src/integrationTest/kotlin/com/pubnub/api/integration/SubscribeIntegrationTests.kt
Outdated
Show resolved
Hide resolved
...tlin-impl/src/integrationTest/kotlin/com/pubnub/api/integration/SubscribeIntegrationTests.kt
Outdated
Show resolved
Hide resolved
...tlin-impl/src/integrationTest/kotlin/com/pubnub/api/integration/SubscribeIntegrationTests.kt
Outdated
Show resolved
Hide resolved
| @Test | ||
| fun can_transit_from_RECEIVING_to_RECEIVING_when_there_is_SUBSCRIPTION_CHANGED_event() { | ||
| fun can_transit_from_RECEIVING_to_RECEIVING_when_there_is_SUBSCRIPTION_CHANGED_event_with_different_channels_and_groups() { | ||
| // given |
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.
I’d avoid adding comments like // given, // when, or // then. Instead, add comments only when they clarify something that is not obvious
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.
// given, // when, // then is convention that exist in many test in this SDK.
I would leave them for now.
Personally I find // given, // when, // then useful when reading tests.
…subscribe just emit SubscriptionChanged status.
68be032 to
d377ea8
Compare
just emit SubscriptionChanged status.