Coroutines and necessary Ktlint configuration #39
Conversation
| import org.junit.runner.Description | ||
| import org.junit.runners.model.Statement | ||
|
|
||
| class GravatarSdkTest : TestRule { |
There was a problem hiding this comment.
This TestRule sets the basic container to be able to test the GravatarApi.
| ktlint_function_signature_body_expression_wrapping = default | ||
| ktlint_function_signature_rule_force_multiline_when_parameter_count_greater_or_equal_than = 6 |
There was a problem hiding this comment.
I think it looks better in the same line when possible, but let me know what you think.
| ktlint_standard_multiline-expression-wrapping = disabled | ||
| ktlint_standard_string-template-indent = disabled |
There was a problem hiding this comment.
Allow to keep the expression in the same line when it fits. Example:
@Test
fun `given a file, email and accessToken when uploading avatar then Gravatar service is invoked`() = runTest {
Without disabling those rules:
@Test
fun `given a file, email and accessToken when uploading avatar then Gravatar service is invoked`() =
runTest {
| @@ -27,6 +26,8 @@ class GravatarApi(private val okHttpClient: OkHttpClient? = null) { | |||
| UNKNOWN, | |||
| } | |||
|
|
|||
| val coroutineScope = CoroutineScope(GravatarSdkContainer.instance.dispatcherDefault) | |||
There was a problem hiding this comment.
Injecting the dispatchers is a good practice to be able to replace them when unit testing.
hamorillo
changed the title
| if (response.isSuccessful) { | ||
| gravatarUploadListener.onSuccess() | ||
| } else { | ||
| // Log the response body for debugging purposes if the response is not successful | ||
| Log.w( | ||
| GravatarSdkContainer.instance.logger.w( |
There was a problem hiding this comment.
Is there a way to alias GravatarSdkContainer.instance.logger to something shorter?
There was a problem hiding this comment.
I've refactored this part. Rethinking about this, we probably don't need to inject the logger; the main reason to have a wrapper around it would be to have better control over our library logs and send some error analytics when/if needed. WDYT?
There was a problem hiding this comment.
we probably don't need to inject the logger; the main reason to have a wrapper around it would be to have better control over our library logs and send some error analytics when/if needed
It sounds like this falls into the premature optimization category. Let's keep it simple for now, we can inject a logger abstraction later if we need to.
| ktlint_function_signature_body_expression_wrapping = default | ||
| ktlint_function_signature_rule_force_multiline_when_parameter_count_greater_or_equal_than = 6 |
#30
Using Kotlin Coroutines will allow us to improve our testing possibilities. This PR adds several tests to the current GravatarApi.
I've created a wrapper for the Android logger and injected it in the GravatarApi using the container. That way, we don't need an Android class in the unit test, and why not? We can verify that the Logger is invoked (if needed).
This PR also modified our Ktlint rules to remove some unnecessary "new line" requirements.