Better testing of event persistence #765
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR improves event testing by enhancing how persisted events are tracked and validated in test cases. The changes migrate from simple event count assertions to detailed per-contact event sequence validation, providing better test coverage and more granular event tracking.
Key changes:
- Added
GetHistoryEventsutility function to extract events by contact from DynamoDB test history - Changed test event assertions from aggregate counts to per-contact event sequences
- Migrated test functions from
handlerspackage tohandlers_testpackage
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| testsuite/utils.go | Added GetHistoryEvents function to scan DynamoDB test history and return events by contact UUID |
| core/runner/handlers/base_test.go | Changed package to handlers_test, updated PersistedEvents type, replaced event counting logic with utility function call |
| core/runner/handlers/*.go | Updated all test files to use new per-contact event tracking format and local test functions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| func GetHistoryEvents(t *testing.T, rt *runtime.Runtime) map[flows.ContactUUID][]string { | ||
| rt.Writers.History.Flush() | ||
|
|
||
| evtTypes := make(map[flows.ContactUUID][]string, 4) |
There was a problem hiding this comment.
The hardcoded capacity of 4 appears to be a magic number. Consider using a constant or calculating the capacity based on expected data size, or simply omit the capacity parameter.
| evtTypes := make(map[flows.ContactUUID][]string, 4) | |
| evtTypes := make(map[flows.ContactUUID][]string) |
Copilot uses AI. Check for mistakes.
| evtTypes := make(map[flows.ContactUUID][]string, 4) | ||
| for _, item := range dyntest.ScanAll[models.DynamoItem](t, rt.Dynamo, "TestHistory") { | ||
| contactUUID := flows.ContactUUID(item.PK[4:]) |
There was a problem hiding this comment.
The hardcoded slice index 4 is a magic number that assumes a specific prefix format. Consider defining a constant for the prefix length or adding validation to ensure the PK has the expected format.
| evtTypes := make(map[flows.ContactUUID][]string, 4) | |
| for _, item := range dyntest.ScanAll[models.DynamoItem](t, rt.Dynamo, "TestHistory") { | |
| contactUUID := flows.ContactUUID(item.PK[4:]) | |
| const historyPKPrefix = "hist" | |
| const historyPKPrefixLen = len(historyPKPrefix) | |
| evtTypes := make(map[flows.ContactUUID][]string, 4) | |
| for _, item := range dyntest.ScanAll[models.DynamoItem](t, rt.Dynamo, "TestHistory") { | |
| require.GreaterOrEqual(t, len(item.PK), historyPKPrefixLen, "item.PK too short: %q", item.PK) | |
| require.Equal(t, historyPKPrefix, item.PK[:historyPKPrefixLen], "item.PK does not start with expected prefix: %q", item.PK) | |
| contactUUID := flows.ContactUUID(item.PK[historyPKPrefixLen:]) |
Copilot uses AI. Check for mistakes.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #765 +/- ##
==========================================
- Coverage 51.03% 50.96% -0.08%
==========================================
Files 250 250
Lines 15112 15133 +21
==========================================
Hits 7713 7713
- Misses 6591 6612 +21
Partials 808 808 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
No description provided.