KStateMachine / kstatemachine

KStateMachine is a Kotlin DSL library for creating state machines and statecharts.
https://kstatemachine.github.io/kstatemachine/
Boost Software License 1.0
340 stars 19 forks source link

Cannot execute multiple test cases #34

Closed mashtonian closed 1 year ago

mashtonian commented 1 year ago

Hi,

Firstly I love this library - it's super useful and clean.

My problem is when I run an automated Junit test suite against a class that hosts a state machine, only the first test executed passes. All subsequent tests fail with the following error:

Multiple transitions match com.example.fsmtesting.Do$Something@21d03963, [(DefaultTransition, ru.nsk.kstatemachine.TargetState@1f760b47), (DefaultTransition, ru.nsk.kstatemachine.TargetState@18ece7f4)] in One
java.lang.IllegalStateException: Multiple transitions match com.example.fsmtesting.Do$Something@21d03963, [(DefaultTransition, ru.nsk.kstatemachine.TargetState@1f760b47), (DefaultTransition, ru.nsk.kstatemachine.TargetState@18ece7f4)] in One
    at ru.nsk.kstatemachine.InternalStateKt.findUniqueResolvedTransition(InternalState.kt:48)
    at ru.nsk.kstatemachine.BaseStateImpl

If I run each test on their own, they will pass, individually.

Can you help? It's driving me crazy!

Matt

mashtonian commented 1 year ago

By debugging through the code it seems that the number of transitions in the BaseStateImpl class has multiplied (I think been reproduced) by the time the second test runs.

In my setup it goes from 2 when the first test runs, to 4. (I have two transitions defined from my initial state)

I can provide you with a minimal set of code to reproduce the issue if you would like

nsk90 commented 1 year ago

Hi Matt, thank you!

Yes, please provide minimal sample code. I will check what I can do. Currently I dont know the source of this behaviour.

By default JUnit creates new Test class instance for each test case, maybe this is related to the problem.

Seems that your States are populated with transitions for each test case. Do you use object keyword for them?

mashtonian commented 1 year ago

Here is the domain code:

package com.example.fsmtesting
import ru.nsk.kstatemachine.*

sealed class In {
    object One: DefaultState()
    object Two: DefaultState()
}

sealed class Do {
    object Something: Event
    object Nothing: Event
}

interface Thing {
}

enum class Things(): Thing {
    SOMETHING, NOTHING
}

class FSMHost {

    private var stateMachine: StateMachine = createStateMachine {
        addInitialState(In.One) {
            transition<Do.Something> {
                targetState = In.Two
            }

            transition<Do.Nothing> {
                targetState = In.One
            }
        }

        addState(In.Two) {
            transition<Do.Something> {
                targetState = In.One
            }

            transition<Do.Nothing> {
                targetState = In.Two
            }
        }
    }

    fun process(thing: Thing) {
        when (thing) {
            Things.SOMETHING -> stateMachine.processEvent(Do.Something)
            Things.NOTHING -> stateMachine.processEvent(Do.Nothing)
        }
    }

}

And here is the test code:

package com.example.fsmtesting

import org.junit. Test

class FSMHostTests {

    @Test
    fun one() {
        val host1 = FSMHost()
        host1.doThings(arrayOf (
            Things.NOTHING,
            Things.SOMETHING,
            Things.SOMETHING
        ))
    }

    @Test
    fun two() {
        val host2 = FSMHost()
        host2.doThings(arrayOf (
            Things.SOMETHING,
            Things.NOTHING,
            Things.SOMETHING,
            Things.NOTHING,
            Things.NOTHING,
            Things.SOMETHING
        ))
    }

    private fun FSMHost.doThings(things: Array<Thing>) {
        for (thing in things) this.process(thing)
    }

}

And for completeness, my build.gradle:

plugins {
    id 'com.android.application'
    id 'org.jetbrains.kotlin.android'
}

android {
    compileSdk 32

    defaultConfig {
        applicationId "com.example.fsmtesting"
        minSdk 21
        targetSdk 32
        versionCode 1
        versionName "1.0"

        testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner"
    }

    buildTypes {
        release {
            minifyEnabled false
            proguardFiles getDefaultProguardFile('proguard-android-optimize.txt'), 'proguard-rules.pro'
        }
    }
    compileOptions {
        sourceCompatibility JavaVersion.VERSION_1_8
        targetCompatibility JavaVersion.VERSION_1_8
    }
    kotlinOptions {
        jvmTarget = '1.8'
    }
}

dependencies {

    implementation 'androidx.core:core-ktx:1.7.0'
    implementation 'androidx.appcompat:appcompat:1.4.2'
    implementation 'com.google.android.material:material:1.6.1'
    testImplementation 'junit:junit:4.13.2'
    androidTestImplementation 'androidx.test.ext:junit:1.1.3'
    androidTestImplementation 'androidx.test.espresso:espresso-core:3.4.0'

    implementation 'io.github.nsk90:kstatemachine:0.9.5'
}
mashtonian commented 1 year ago

The slightly odd use of the Things enum class that inherits the Thing interface is there to mimic my real application as closely as possible, in terms of type structure. I don't think it has any bearing on this issue...

nsk90 commented 1 year ago

The problem is in object keyword for your states. See "do-not" section https://github.com/nsk90/kstatemachine/wiki#do-not

Quick fix: define your States without object keyword, or make sure that you create only one instance of statemachine that uses them.

Looks that it becomes popular missunderstanding and I have to do something with it.

Please see the previous issue https://github.com/nsk90/kstatemachine/issues/32

mashtonian commented 1 year ago

I see (sort of)

I used the object keyword by reference to the sealed class example in the wiki.

If I simply remove it, the code won't compile.

If I understand what you are saying, I will have to construct a completely new instance of statemachine in each test, with unique state classes? This seems to be very unwieldy and unmaintainable.

I must be missing something?

mashtonian commented 1 year ago

Ah, got it:


 private val StateOne = In.One()
    private val StateTwo = In.Two()

    private var stateMachine: StateMachine = createStateMachine {
        addInitialState(StateOne) {
            transition<Do.Something> {
                targetState = StateTwo
            }

            transition<Do.Nothing> {
                targetState = StateOne
            }
        }

        addState(StateTwo) {
            transition<Do.Something> {
                targetState = StateOne
            }

            transition<Do.Nothing> {
                targetState = StateTwo
            }
        }
    }

Thank you!

nsk90 commented 1 year ago

Yes, you just need to create new State instances for each new stateMachine instance.

My samples use object keyword for states widely as there is always only one machine instance, sorry)

I will think what I can do with singleton states.

nsk90 commented 1 year ago

Please see 0.10.0 version https://github.com/nsk90/kstatemachine/releases/tag/v0.10.0. It allows using object states in multiple machine instances.