Hey guys, me again ¯\_(ツ)_/¯ I’ve been adopting new 4.0.0 test model and found an issue: the `exce...

# orbit-mvi

[Oleksii Malovanyi]

Hey guys, me again ¯\_(ツ)_/¯

I’ve been adopting new 4.0.0 test model and found an issue: the exceptionHandler is never used so any exception would go to the TestCoroutineExceptionHandler which would fail the test afterwards (runBlockingTest is used)

[Oleksii Malovanyi]

I’ve made an investigation and I think it happens because of the InterceptingContainerDecorator and SuspendingTestContainerHost#suspendingIntent -> the latest receives the intent in the context of the test which, of course, ignores the Settings’s exceptionHandler

[Oleksii Malovanyi]

as a solution, what do you think about adding the withContext wrap in case there is settings.exceptionHandler exists – smth similar to the what RealContainer does

[Oleksii Malovanyi]

I mean this function

@Suppress("EXPERIMENTAL_API_USAGE")
    private suspend fun <STATE : Any, SIDE_EFFECT : Any, T : ContainerHost<STATE, SIDE_EFFECT>> T.suspendingIntent(
        shouldIsolateFlow: Boolean,
        block: T.() -> Unit
    ) {
        val testContainer = container.findTestContainer()

        this.block() // Invoke the Intent

        if (testContainer.savedIntents.isEmpty) {
            throw IllegalArgumentException("testIntent block must invoke an orbit intent!")
        }

        var firstIntentExecuted = false
        while (!testContainer.savedIntents.isEmpty) {
            val intent = testContainer.savedIntents.receive()
            if (!shouldIsolateFlow || !firstIntentExecuted) {
                firstIntentExecuted = true

// WRAP withContext() here
                intent()
            }
        }
    }

[Oleksii Malovanyi]

I’ve created a PR to demonstrate the problem and the possible fix: https://github.com/orbit-mvi/orbit-mvi/pull/50

Please note, this is a draft and not a final solution, despite it works (at least with the ExceptionHandler tests)

[Mikolaj Leszczynski]

yep that sounds about right, sorry about breaking this!

[Mikolaj Leszczynski]

@Oleksii Malovanyi remind me why we installed the excpetion handler at the orbit function level, and not the container scope level?

[Mikolaj Leszczynski]

is it because otherwise the orbit loop breaks after getting an exception?

[Mikolaj Leszczynski]

yeah, I just answered my own question…

[Oleksii Malovanyi]

yeap, yeap, yeap 😅

[Mikolaj Leszczynski]

I’ve commented on your PR

[Mikolaj Leszczynski]

and sorry - I used Github’s new batch suggestion feature and didn’t realise it would actually commit those suggestions!

[Oleksii Malovanyi]

thanks I guess, why now? 🙂

[Mikolaj Leszczynski]

at least now the tests are green 😛

[Mikolaj Leszczynski]

so I guess my suggestion worked

[Oleksii Malovanyi]

@Mikolaj Leszczynski I’ve added small explanation to the tests why scope would be active, and that’s it from my side, all checks are green. Do you think this PR is ready for a review?

[Mikolaj Leszczynski]

ready for merge even

[Mikolaj Leszczynski]

Thanks @Oleksii Malovanyi !

[Mikolaj Leszczynski]

started the release process

[Oleksii Malovanyi]

metal metal metal

[Mikolaj Leszczynski]

I think our treatment of scopes etc. could use an improvement though

[Mikolaj Leszczynski]

one step at a time I guess 🙂

[Oleksii Malovanyi]

that’s tricky… I feel like I need to get back to documentation about the scopes to keep track on how do they work together

[Mikolaj Leszczynski]

yes, it’s not an easy thing

[Mikolaj Leszczynski]

I don’t have a clear idea of how to simplify and improve what we have

[Mikolaj Leszczynski]

thanks a lot for your recent contributions, you’re pushing us to improve the library 🙂

[Oleksii Malovanyi]

thank you guys for a blazing fast response – that’s really valuable

[miqbaldc]

❤️