Refactor TestFormula and key logic. #379
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Laimiux
commented
Aug 23, 2024
| import com.instacart.formula.Snapshot | ||
| import java.util.concurrent.atomic.AtomicLong | ||
|
|
||
| /** | ||
| * Test formula is used to provide a fake formula implementation. It allows you to [send][output] | ||
| * output updates and [inspect/interact][input] with input. | ||
| */ | ||
| abstract class TestFormula<Input, Output> : |
There was a problem hiding this comment.
Having both inheritance and factory approaches creates inconsistencies in the API. I've decided to make this a final class and keep the following API as the primary approach
class FakeUserFormula : UserFormula {
override val implementation = testFormula(
initialOutput = User(
name = "Fake user name",
)
)
}
JaCoCo Code Coverage 88.01% ✅
Generated by 🚫 Danger |
Laimiux
commented
Aug 23, 2024
| * override fun key(input: ItemInput) = input.itemId | ||
| * ``` | ||
| */ | ||
| override fun key(input: Input): Any? = null |
There was a problem hiding this comment.
Not needed since the default implementation uses implementation.key() which is null.
Laimiux
force-pushed
the
laimonas/refactor-key-logic
branch
6 times, most recently
from
b400b66 to
c2bd013
Compare
Laimiux
force-pushed
the
laimonas/refactor-key-logic
branch
from
c2bd013 to
c826fc2
Compare
Laimiux
commented
Aug 23, 2024
| private val stateMap = mutableMapOf<Any?, Value<Input, Output>>() | ||
|
|
||
| abstract fun initialOutput(): Output | ||
| private val runningInstanceManager = RunningInstanceManager<Input, Output>( |
There was a problem hiding this comment.
Moving the logic to keep track of individual formulas state to this class
Laimiux
commented
Aug 23, 2024
| return requireNotNull(stateMap.values.lastOrNull()) { | ||
| "Formula is not running" | ||
| } | ||
| fun mostRecentInputs(): List<Input> { |
There was a problem hiding this comment.
Adding the ability to get a list of inputs instead of the most recent one.
jasonostrander
approved these changes
Aug 23, 2024
| * output updates and [inspect/interact][input] with input. | ||
| * Test formula is used to replace a real formula with a fake implementation. This allows you to: | ||
| * - Verify that parent passes correct inputs. Take a look at [input]. | ||
| * - Verify that parent deals with output changes correctly. Take a look at [output] |
There was a problem hiding this comment.
Could we also call out that we support the running count? This is a handy feature that I think a lot of devs don't know about.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are a couple of changes around this PR
TestFormulainto a final class constructed viatestFormulaextension.TestFormulakey resolutionTestFormulausage.RunningInstanceManager