Skip to content

Commit

Permalink
Fix a termination race-condition within formula test. (#375)
Browse files Browse the repository at this point in the history
* Fix a termination race-condition within formula test.

* Run formula-test module tests.
  • Loading branch information
Laimiux authored Aug 15, 2024
1 parent bccd4ba commit 58d5431
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 12 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/build-workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ jobs:
run: ./gradlew :formula-android:testRelease
- name: Run Formula Android Instrumentation Tests
run: ./gradlew :formula-android-tests:testRelease
- name: Run Formula Test Module tests
run: ./gradlew :formula-test:test
- name: Run Formula Lint Tests
run: ./gradlew :formula-lint:build
- name: Generate Jacoco Report
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import com.instacart.formula.Action
import com.instacart.formula.Evaluation
import com.instacart.formula.Formula
import com.instacart.formula.Snapshot
import java.lang.IllegalStateException
import java.util.concurrent.atomic.AtomicLong

/**
* Test formula is used to provide a fake formula implementation. It allows you to [send][output]
Expand All @@ -30,15 +30,19 @@ abstract class TestFormula<Input, Output> :
}

data class State<Output>(
val uniqueIdentifier: Long,
val key: Any?,
val output: Output
)

data class Value<Input, Output>(
val key: Any?,
val input: Input,
val onNewOutput: (Output) -> Unit
)

private val identifierGenerator = AtomicLong(0)

/**
* Uses initial input as key (to be decided if its robust enough)
*/
Expand All @@ -47,11 +51,16 @@ abstract class TestFormula<Input, Output> :
abstract fun initialOutput(): Output

override fun initialState(input: Input): State<Output> {
return State(key = key(input), output = initialOutput())
return State(
uniqueIdentifier = identifierGenerator.getAndIncrement(),
key = key(input),
output = initialOutput(),
)
}

override fun Snapshot<Input, State<Output>>.evaluate(): Evaluation<Output> {
stateMap[state.key] = Value(
stateMap[state.uniqueIdentifier] = Value(
key = state.key,
input = input,
onNewOutput = context.onEvent {
transition(state.copy(output = it))
Expand All @@ -62,9 +71,8 @@ abstract class TestFormula<Input, Output> :
output = state.output,
actions = context.actions {
Action.onTerminate().onEvent {
transition {
stateMap.remove(state.key)
}
stateMap.remove(state.uniqueIdentifier)
none()
}
}
)
Expand All @@ -81,9 +89,7 @@ abstract class TestFormula<Input, Output> :
}

fun output(key: Any?, output: Output) {
val instance = requireNotNull(stateMap[key]) {
"Formula is not running"
}
val instance = getByKey(key)
instance.onNewOutput(output)
}

Expand All @@ -101,9 +107,7 @@ abstract class TestFormula<Input, Output> :
* Performs an interaction on the current [Input] passed by the parent.
*/
fun input(key: Any?, interact: Input.() -> Unit) {
val instance = requireNotNull(stateMap[key]) {
"Formula for $key is not running, there are ${stateMap.keys} running"
}
val instance = getByKey(key)
instance.input.interact()
}

Expand All @@ -113,4 +117,11 @@ abstract class TestFormula<Input, Output> :
throw AssertionError("Expected $expected running formulas, but there were $count instead")
}
}

private fun getByKey(key: Any?): Value<Input, Output> {
return requireNotNull(stateMap.entries.firstOrNull { it.key == key }?.value) {
val existingKeys = stateMap.entries.map { it.key }
"Formula for $key is not running, there are $existingKeys running"
}
}
}

0 comments on commit 58d5431

Please sign in to comment.