refactor: remove reflection on unit tests #263
Conversation
polbins
changed the title
… setting dispatchers
polbins
force-pushed
the
AMP-128374/remove-reflection-on-unit-tests
branch
from
9b8d04a to
2e492d9
Compare
| @@ -41,7 +41,7 @@ import java.util.concurrent.Executors | |||
| * This is the SDK instance class that contains all of the SDK functionality.<br><br> | |||
| * Many of the SDK functions return the SDK instance back, allowing you to chain multiple methods calls together. | |||
| */ | |||
| open class Amplitude internal constructor( | |||
| open class Amplitude( | |||
There was a problem hiding this comment.
I had to open this up for testability
| eventsList.forEach { | ||
| delay(BACK_OFF) | ||
| eventPipeline.put(it) | ||
| } |
There was a problem hiding this comment.
I don't like to add this, but the current response handler integration tests are made in such a way that requires adding delays to control the flow of the test. This requires a lot more effort to fix, so it might be good to revisit when we're dealing with InMemoryStorage tasks.
core/src/main/java/com/amplitude/core/utilities/InMemoryResponseHandler.kt
Show resolved
Hide resolved
| import kotlinx.coroutines.test.TestScope | ||
|
|
||
| @OptIn(ExperimentalCoroutinesApi::class) | ||
| class FakeAmplitude( |
There was a problem hiding this comment.
do we really need fake? Can't we use spy?
There was a problem hiding this comment.
We can use Spy! But Fakes are easier to use (no need for a library) and less boilerplate too, so usually fakes are preferred for tests
polbins
force-pushed
the
AMP-128374/remove-reflection-on-unit-tests
branch
from
b2e1897 to
8ddb1fe
Compare
|
🎉 This PR is included in version 1.20.7 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.20.7 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Summary
Enable setting of your own dispatchers for tests, added factory methods for creating fakes, and cleaned up a bunch of tests as well.
It's easier to review just the last two commits: https://github.com/amplitude/Amplitude-Kotlin/pull/263/files/cc5de2659b7c5d4abc26a72231e624a8dccb39c4..2e492d92336e6133532741da93744dcd24708cb3#diff-9b0abba662a8a3d0d290c1970d3ea84a56e092ad06e3e5445f391ec51ff83e9eR82-R85
Checklist