Conversation
awln-temporal
force-pushed
the
awln/chasm-test-engine-base
branch
from
19cf028 to
171cdf0
Compare
chasm/chasmtest/test_engine.go
Outdated
| ) | ||
|
|
||
| type ( | ||
| Option[T chasm.RootComponent] func(*Engine[T]) |
chasm/chasmtest/test_engine.go
Outdated
| type ( | ||
| Option[T chasm.RootComponent] func(*Engine[T]) | ||
|
|
||
| Engine[T chasm.RootComponent] struct { |
There was a problem hiding this comment.
Why does the engine need to be typed?
There was a problem hiding this comment.
doesn't, it was only added for the helper func to get the Root component, but definitely not needed, removing this along with other accessor methods as mentioned below
chasm/chasmtest/test_engine.go
Outdated
| return e | ||
| } | ||
|
|
||
| func WithRoot[T chasm.RootComponent]( |
There was a problem hiding this comment.
Not needed, StartExecution should be used here instead to create a root.
chasm/chasmtest/test_engine.go
Outdated
| } | ||
| } | ||
|
|
||
| func WithExecutionKey[T chasm.RootComponent](key chasm.ExecutionKey) Option[T] { |
There was a problem hiding this comment.
That will also be part of StartExecution.
chasm/chasmtest/test_engine.go
Outdated
| } | ||
| } | ||
|
|
||
| func (e *Engine[T]) Root() T { |
There was a problem hiding this comment.
We can use ReadComponent to extract whatever we need from the engine.
awln-temporal
changed the title
chasm/chasmtest/test_engine.go
Outdated
| registry *chasm.Registry | ||
| logger log.Logger | ||
| metrics metrics.Handler | ||
| executions map[executionLookupKey]*execution |
There was a problem hiding this comment.
We can start with this but you'll want to expand it to handle multiple runs and have a way to locate the current run for completeness.
There was a problem hiding this comment.
This limits what can be tested today and we'll have to rely on functional test coverage for ID policy enforcement for example.
There was a problem hiding this comment.
i'm wondering if this these cases should be in scope of functional tests. In this case, it's testing for the real behavior of handling conflicting IDs, while the TestEngine and real chasm_engine.go have technically different implementations.
chasm/chasmtest/test_engine.go
Outdated
| func (e *Engine) EngineContext() context.Context { | ||
| return chasm.NewEngineContext(context.Background(), e) | ||
| } |
There was a problem hiding this comment.
Don't add this, tests have their own context that they'll want to install the engine on.
| return chasm.StartExecutionResult{ | ||
| ExecutionKey: execution.key, | ||
| ExecutionRef: serializedRef, | ||
| Created: true, |
There was a problem hiding this comment.
This should be false when using UseExisting policy.
| return serviceerror.NewUnimplemented("chasmtest.Engine.DeleteExecution") | ||
| } | ||
|
|
||
| func (e *Engine) NotifyExecution(chasm.ExecutionKey) {} |
There was a problem hiding this comment.
Is this intentionally not implemented?
There was a problem hiding this comment.
no, I wanted to review the important methods first to make sure we're aligned on the contract, will add DeleteExecution impl.
| HandleNextTransitionCount: func() int64 { return 2 }, | ||
| HandleGetCurrentVersion: func() int64 { return 1 }, | ||
| HandleGetWorkflowKey: func() definition.WorkflowKey { | ||
| return definition.NewWorkflowKey(key.NamespaceID, key.BusinessID, key.RunID) | ||
| }, | ||
| HandleIsWorkflow: func() bool { return false }, | ||
| HandleCurrentVersionedTransition: func() *persistencespb.VersionedTransition { | ||
| return &persistencespb.VersionedTransition{ | ||
| NamespaceFailoverVersion: 1, | ||
| TransitionCount: 1, | ||
| } | ||
| }, |
There was a problem hiding this comment.
You're going to want to increment the current version on every transaction (UpdateComponent, StartExecution, UpdateWithStartExecution).
There was a problem hiding this comment.
hm ok, yeah I left this part out since the backend is not even accessible right now. Should we discuss what verifications would be useful to check on the MockNodeBackend?
There was a problem hiding this comment.
If we don't really care about verifications on transition counts and rather that be in scope of functional tests, should we just leave the MockNodeBackend impl as is?
chasm/chasmtest/test_engine.go
Outdated
| key.NamespaceID = "test-namespace-id" | ||
| } | ||
| if key.BusinessID == "" { | ||
| key.BusinessID = "test-workflow-id" |
There was a problem hiding this comment.
| key.BusinessID = "test-workflow-id" | |
| key.BusinessID = "test-business-id" |
chasm/chasmtest/test_engine.go
Outdated
| } | ||
| } | ||
|
|
||
| func normalizeExecutionKey(key chasm.ExecutionKey) chasm.ExecutionKey { |
There was a problem hiding this comment.
I would just ask test authors to create a valid execution key instead of implicitly adding defaults.
| return chasm.NewEngineContext(context.Background(), e) | ||
| } | ||
|
|
||
| func (e *Engine) Ref(component chasm.Component) chasm.ComponentRef { |
There was a problem hiding this comment.
Not needed IMHO. Test authors should already have refs to components they create using start execution.
There was a problem hiding this comment.
how would they get the ref to the component within StartExecution or after? Subcomponents only get set in valueToNode map after syncSubComponents when SetRootComponent is called, so we'd either need to return this map as a result of StartExecution or something similar.
chasm/lib/callback/tasks_test.go
Outdated
| func readCallbackFromEngine( | ||
| t *testing.T, | ||
| testEngine *chasmtest.Engine, | ||
| callback *Callback, | ||
| ) *Callback { |
There was a problem hiding this comment.
What is this supposed to do? If I already have a callback component, isn't this just returning the same callback? Seems confusing to me.
There was a problem hiding this comment.
yeah not sure what I was thinking, removed
chasm/lib/callback/tasks_test.go
Outdated
|
|
||
| // Verify the outcome and tasks | ||
| tc.assertOutcome(t, callback, err) | ||
| tc.assertOutcome(t, readCallbackFromEngine(t, testEngine, callback), err) |
There was a problem hiding this comment.
I would just wrap this call in chasm.ReadComponent
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
awln-temporal
force-pushed
the
awln/chasm-test-engine-base
branch
from
8a834bc to
f502701
Compare
2 participants
What changed?
Add chasm test engine for unit tests
Why?
Currently, CHASM library unit tests need to create mock chasm engine behavior, manually wire expected framework calls to the CHASM tree/node to CloseTransaction/generate any state needed for validation. Instead, unit tests should behave similar to how they are implemented, and delegate any chasm tree creation and reading component state to chasm engine methods.
This change introduces an implementation of the test CHASM engine that mocks the NodeBackend, but implements all methods that mutate or create Component state.
How did you test it?