You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Findings from a read of Marten/Events/TestSupport/ (796 lines across 7 files) while assessing the cross-store lift. None of these are correctness bugs — the one bug found is marten#5126, filed separately so it can be fixed without waiting on any of this.
Grouped by how much they cost a user.
Visible to users
1. Append / StartStream return a StreamAction that never executes. Each method queues a closure and separately constructs a throwaway result:
publicStreamActionAppend(Guidstream,paramsobject[]events){varstep=action(e =>e.Append(stream,events));// this is what runs
...
return StreamAction.Append(_store.Events,stream,events);// this is not}
The returned value looks like the operation's result and is not connected to it. Either return void/the ScenarioStep, or document loudly.
2. Assertions throw bare Exception.
thrownewException($"Document {typeof(T).FullNameInCode()} with id '{id}' does not exist");
Nothing can distinguish an assertion failure from an infrastructure failure. A typed ProjectionAssertionException gets that without pulling an assertion library into Marten core.
3. DoNotDeleteExistingData is a double negative on an API that wipes the database by default.EventProjectionScenario deletes all event data plus every projection's storage types unless you set this. The XML doc says "Only use this in testing," which is right, but the opt-out reads backwards at the call site.
4. No documentation.git grep EventProjectionScenario docs/ returns nothing. The feature is only discoverable from the AdvancedOperations XML comment.
5. Public constructor, internal Execute.ProjectionScenario(DocumentStore) is public but Execute is internal, so a directly-constructed scenario can be configured and never run. Either make the ctor internal or make Execute public.
Internal quality
6. ~120 lines of copy-paste in the assertions.DocumentShouldExist<T> and DocumentShouldNotExist<T> are each written four times — Guid/int/long/string — with byte-identical bodies apart from the id parameter type. One private helper over object id collapses both sets; this is exactly the shape LoadDocumentAsync(session, object id, ct) already uses in JasperFx.Events.ComplianceTests.
7. Double enumeration of IEnumerable<object> overloads.
varstep=action(e =>e.Append(stream,events));if(events.Count()>3)// walk #1, just for the description
...
$"...{events.Select(x =>x.ToString()).Join(", ")}" // walk #2
plus a third walk when the closure runs. A lazy or single-pass sequence is enumerated repeatedly, and building the description eagerly costs even when the scenario passes.
8. WaitForNonStaleData() hard-codes 30 seconds and ignores the scenario's CancellationToken. Not configurable per scenario or per step.
9. Nullability.public string TenantId { get; set; } is non-nullable but defaults to null; internal IProjectionDaemon Daemon and internal IDocumentSession Session are non-nullable but assigned during Execute. (Polecat's port already declares string? TenantId.)
10. A scenario cannot be re-run. Steps live in a Queue<ScenarioStep> drained by Execute, so a second Execute silently does nothing.
11. Failure handling is all-or-nothing. Every step runs even after one fails, and all exceptions are aggregated into ProjectionScenarioException. Good for assertions; questionable for actions, where a failed append means the remaining assertions run against a state nobody intended and produce cascading noise. Consider fail-fast on actions, accumulate on assertions.
Not a finding
CompactStreamAsync<T> — both overloads are IEventOperations stubs that throw new NotSupportedException(). Worth deciding whether they should exist at all, but they are not a gap.
Relationship to other issues
marten#5126 — the trailing-append bug. Independent, fix first.
The cross-store lift into JasperFx.Events is a separate design conversation; items 1, 2, 6 and 11 are the ones that would change what a shared surface should look like.
Findings from a read of
Marten/Events/TestSupport/(796 lines across 7 files) while assessing the cross-store lift. None of these are correctness bugs — the one bug found is marten#5126, filed separately so it can be fixed without waiting on any of this.Grouped by how much they cost a user.
Visible to users
1.
Append/StartStreamreturn aStreamActionthat never executes. Each method queues a closure and separately constructs a throwaway result:The returned value looks like the operation's result and is not connected to it. Either return
void/theScenarioStep, or document loudly.2. Assertions throw bare
Exception.Nothing can distinguish an assertion failure from an infrastructure failure. A typed
ProjectionAssertionExceptiongets that without pulling an assertion library into Marten core.3.
DoNotDeleteExistingDatais a double negative on an API that wipes the database by default.EventProjectionScenariodeletes all event data plus every projection's storage types unless you set this. The XML doc says "Only use this in testing," which is right, but the opt-out reads backwards at the call site.4. No documentation.
git grep EventProjectionScenario docs/returns nothing. The feature is only discoverable from theAdvancedOperationsXML comment.5. Public constructor, internal
Execute.ProjectionScenario(DocumentStore)is public butExecuteisinternal, so a directly-constructed scenario can be configured and never run. Either make the ctor internal or makeExecutepublic.Internal quality
6. ~120 lines of copy-paste in the assertions.
DocumentShouldExist<T>andDocumentShouldNotExist<T>are each written four times — Guid/int/long/string — with byte-identical bodies apart from the id parameter type. One private helper overobject idcollapses both sets; this is exactly the shapeLoadDocumentAsync(session, object id, ct)already uses inJasperFx.Events.ComplianceTests.7. Double enumeration of
IEnumerable<object>overloads.plus a third walk when the closure runs. A lazy or single-pass sequence is enumerated repeatedly, and building the description eagerly costs even when the scenario passes.
8.
WaitForNonStaleData()hard-codes 30 seconds and ignores the scenario'sCancellationToken. Not configurable per scenario or per step.9. Nullability.
public string TenantId { get; set; }is non-nullable but defaults to null;internal IProjectionDaemon Daemonandinternal IDocumentSession Sessionare non-nullable but assigned duringExecute. (Polecat's port already declaresstring? TenantId.)10. A scenario cannot be re-run. Steps live in a
Queue<ScenarioStep>drained byExecute, so a secondExecutesilently does nothing.11. Failure handling is all-or-nothing. Every step runs even after one fails, and all exceptions are aggregated into
ProjectionScenarioException. Good for assertions; questionable for actions, where a failed append means the remaining assertions run against a state nobody intended and produce cascading noise. Consider fail-fast on actions, accumulate on assertions.Not a finding
CompactStreamAsync<T>— both overloads areIEventOperationsstubs thatthrow new NotSupportedException(). Worth deciding whether they should exist at all, but they are not a gap.Relationship to other issues
JasperFx.Eventsis a separate design conversation; items 1, 2, 6 and 11 are the ones that would change what a shared surface should look like.🤖 Generated with Claude Code