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 "includeAndroidResources" flag makes koin starts twice in roboelectric tests #254

Closed
esdudnik opened this issue Oct 1, 2018 · 8 comments
Labels
android status:checking currently in analysis - discussion or need more detailed specs test

Comments

@esdudnik
Copy link

esdudnik commented Oct 1, 2018

Description
Hello. It seems that using "includeAndroidResources" flag makes koin starts twice in tests, even if it's not used there. Here is the link to repo with test behavior
https://github.com/esdudnik/mapwall
branch "koin-error".
I have 2 unit tests that worked perfectly. One is extended from AutoCloseKoinTest (MapCacheStrategyTest), another one has no parent(SharedPreferencesStorageTest).
After adding
testOptions {
unitTests {
includeAndroidResources = true
}
}
(recommendation from Robolectric), MapCacheStrategyTest run crashed with error "Singleton instance already exists"(seems that App.kt starts twice) and SharedPreferencesStorageTest crashed with "BeanOverrideException - Try to override definition with Single".
I asked a question in slack channel, and show example of tests and project, and guys said that it seems that koin starts twice in robolectic tests, and recommend to create a repo with that reproduces such problem.

To Reproduce
Run gradlew build or separate tests to see error.

Expected behavior
Actually after adding adding "includeAndroidResources" I expect everything to work as before.

Koin project used and used version (please complete the following information):
'koin-android-viewmodel version 1.0.0'

@arnaudgiuliani arnaudgiuliani added test android status:checking currently in analysis - discussion or need more detailed specs type:issue labels Oct 11, 2018
@s0nerik
Copy link

s0nerik commented Nov 4, 2018

I had a very similar problem (BeanOverrideException in Robolectric test) but found out that it wasn't a Koin's fault. The problem was in the way I registered the modules to be included in startKoin() method. I had all Koin module definitions in a static list variable. Here's an example of what it looked like when I had a problem:

@RunWith(AndroidJUnit4::class)
class SomeTest : AutoCloseKoinTest() {
    @Before
    fun before() {
        startKoin(appModules("..."))
    }
}

Here's how I solved a problem:

val appModules by lazy {
    appModules("...")
}

@RunWith(AndroidJUnit4::class)
class SomeTest : AutoCloseKoinTest() {
    @Before
    fun before() {
        startKoin(appModules)
    }
}

The classes/functions used in the example:

private fun appModules(vararg whitelistPackages: String): List<Module> {
    val classGraph = ClassGraph().apply {
        enableAllInfo()
        whitelistPackages(*whitelistPackages)
    }
    val scanResult = classGraph.scan()

    val appModules = scanResult.getSubclasses(AppModule::class.qualifiedName)

    appModules.forEach {
        val kClass = Class.forName(it.name).kotlin
        val instance = kClass.constructors.first().call()
        (instance as AppModule).onCreate()
    }

    return AppModule.registeredModules
}

abstract class AppModule(
        private val definition: ModuleDefinition.() -> Unit
) : ContentProvider() {
    companion object {
        private val modules = mutableListOf<Module>()

        val registeredModules: List<Module>
            get() = modules
    }

    var module: Module
        get
        private set

    init {
        module = module(Path.ROOT, false, false, definition)
    }

    override fun onCreate(): Boolean {
        modules += module
        return true
    }
}

Hope this will help someone.
BTW, I also use "includeAndroidResources = true" with Robolectric 4 and it doesn't seem to change anything regarding the issue.

@arnaudgiuliani arnaudgiuliani added question Usage question and removed status:checking currently in analysis - discussion or need more detailed specs type:issue labels Nov 9, 2018
@roccadev
Copy link

I also have this problem but the solution above is not feasible, since I encounter the exception when creating the Application object:

@Before
fun initContext() {
    context = ApplicationProvider.getApplicationContext()
}

In my Application class, I start Koin in the onCreate() as usual.
I think that we only have a workaround and not a solution to the problem. So far I can only revert Roboelectric tests to on-device instrumented tests to have it working. Please consider fixing it.

@esdudnik
Copy link
Author

esdudnik commented Dec 19, 2018

Hello guys.
Sorry for long response, had a lot of work. Thank you all for your responses, let me describe what I found now.
First of all I remove all unnecessary code and branches. Now it only one "master" branch, in what only one test class "KoinErrorTestClass" with 2 simple tests, that doesn't use anything from modules, classes and so on. So they are as independent as it can be.
@s0nerik thanks for response, I tried your solution with "lazy", but it doesn't help, I still receive the same error.
@arnaudgiuliani I tried to extend my KoinErrorTestClass from AutoCloseKoinTest and without, in both cases I receive error. With extended parent class I receive "Singleton already exist", without I receive "Try to override definition with Single" with name of the first single class in my module list.
Also what I notice, that without "@RunWith(RobolectricTestRunner::class" error disappear and everything works fine.
So I am agree with @roccadev that problem is still exists here.
Thanks.

To Reproduce
Run "gradlew test" or separate tests to see error.

Expected behavior
Actually after adding adding "includeAndroidResources" I expect everything to work as before.

Koin project used and used version (please complete the following information):
'koin-android-viewmodel version 1.0.0'

@arnaudgiuliani arnaudgiuliani added status:checking currently in analysis - discussion or need more detailed specs type:issue and removed question Usage question labels Dec 20, 2018
@curioustechizen
Copy link

With Koin 2.0.0-alpha-6 this issue seems to be fixed. I have successfully used this version of Koin with Robolectric 4.1, with the includeAndroidResources="true" option. I can verify that in my tests that the actual string resource from strings.xml is being picked up by Robolectric.

I did have a moment where I could not override some dependencies in my test but it turns out I was using it wrong.

For sake of completion, this is how I used it

@RunWith(RobolectricTestRunner::class)
class MainActivityTest: AutoCloseKoinTest() {

    lateinit var activity : MainActivity
    @Before
    fun setUp() {
        loadKoinModules(testModule)
        activity = Robolectric.setupActivity(MainActivity::class.java)
    }

    @Test
    fun textView_containsCorrectText() {
        val text = activity.findViewById<TextView>(R.id.hello_world).text
        assertEquals("Hello World", text)
    }
}

Notice the use of loadKoinModules in the setup() function. If you try startKoin here you'll hit an error because when you start the Activity with Robolectric, it already startKoin that has been implemented in my MainApplication class. So startKoin ends up being called twice.

@kenyee
Copy link

kenyee commented Feb 8, 2019

Having issues w/ Koin 2.0.0-beta-1 trying to swap out Kodein as well...complains Koin was started twice. Why can't it just ignore the second startKoin() request and make it a warning instead?

@kenyee
Copy link

kenyee commented Feb 8, 2019

I think it's because RoboElectric tests run in parallel...fixed it by making my app's Koin init check whether it's been initialized like so:

        // this check is for RoboElectric tests that run in parallel so Koin gets set up multiple times
        if (GlobalContext.getOrNull() == null) {
            startKoin {
                logger()
                modules(appModule)
            }
        }

Everything passes with this change :-)

@arnaudgiuliani
Copy link
Member

linked to startKoin or loadModules with a verification to see if it's already started

@arnaudgiuliani
Copy link
Member

to follow on next 2.1.x. reopen if needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android status:checking currently in analysis - discussion or need more detailed specs test
Projects
None yet
Development

No branches or pull requests

6 participants