test: add unit tests for bento plugin modules, steps, and triggers#10
test: add unit tests for bento plugin modules, steps, and triggers#10
Conversation
There was a problem hiding this comment.
Pull request overview
Adds comprehensive unit test coverage across the Bento plugin’s modules (stream/input/output/broker), processor step, and trigger, plus bridge helper conversions, aiming to validate constructors, config parsing, and error paths without running external Bento streams.
Changes:
- Added new unit tests for module instances, trigger parsing, processor step execution, and plugin provider factory/schema behavior.
- Added bridge helper tests for YAML conversion and message/map conversions (including a basic round-trip).
- Minor lint-related tweaks (errcheck-related change in
bridge.go, andnolintannotation inbroker_module.go).
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/trigger_test.go | Tests trigger construction and subscription parsing/error cases. |
| internal/stream_module_test.go | Tests stream module constructor/init/stop behaviors. |
| internal/processor_step_test.go | Tests processor step config parsing and execution (including Bloblang via pure components). |
| internal/plugin_test.go | Tests plugin provider manifest, factories, and schema presence. |
| internal/output_module_test.go | Tests output module init validation and message wiring setters. |
| internal/input_module_test.go | Tests input module init validation and provides mocks used by other tests. |
| internal/broker_module_test.go | Tests broker defaults/config parsing and message wiring setters. |
| internal/broker_module.go | Adds nolint for an intentionally-unused helper. |
| internal/bridge_test.go | Tests config/map/message conversion helpers. |
| internal/bridge.go | Adjusts handling of returned error from MetaWalkMut. |
| go.mod / go.sum | Updates dependencies pulled in by new tests/components. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _ = msg.MetaWalkMut(func(key string, value any) error { | ||
| meta[key] = value | ||
| return nil | ||
| }) |
There was a problem hiding this comment.
messageToMap is explicitly discarding the error returned by msg.MetaWalkMut. If MetaWalkMut can return an error (e.g., from internal iteration), we should handle it and propagate/annotate it so metadata-copy failures don’t get silently ignored.
| _ = msg.MetaWalkMut(func(key string, value any) error { | |
| meta[key] = value | |
| return nil | |
| }) | |
| if err := msg.MetaWalkMut(func(key string, value any) error { | |
| meta[key] = value | |
| return nil | |
| }); err != nil { | |
| return nil, fmt.Errorf("copy message metadata: %w", err) | |
| } |
| reconstructed := mapToMessage(m) | ||
| if reconstructed == nil { | ||
| t.Fatal("mapToMessage() returned nil") | ||
| } | ||
| } |
There was a problem hiding this comment.
This round-trip test only checks that mapToMessage returns non-nil, but it doesn’t assert that the reconstructed message preserves the original body and metadata. Add assertions (e.g., compare AsBytes() and MetaGet values) so the test will fail if the conversion logic regresses.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
- Add plugin_test.go: covers Manifest(), ModuleTypes(), StepTypes(), TriggerTypes(), CreateModule/Step/Trigger for all valid and unknown types, ModuleSchemas(), and interface satisfaction at runtime - Add stream_module_test.go: constructor, Init() validation (empty config), Name field, Stop without Start, config storage, done channel - Add input_module_test.go: constructor, Init() validation (missing/empty target_topic, missing input config), topic/broker field parsing, SetMessagePublisher/Subscriber, MessageAwareModule interface - Add output_module_test.go: constructor, Init() validation (missing/empty source_topic, missing output config), topic/broker field parsing, SetMessageSubscriber/Publisher, MessageAwareModule interface - Add broker_module_test.go: constructor, Init() default transport "memory", explicit transport, transport_config storage, publisher/subscriber injection, streams map initialization, Start no-op, MessageAwareModule interface - Add processor_step_test.go: constructor with string/map/nil processors, invalid type error, Name field, Execute pass-through, current-overrides-trigger merge, empty inputs, bloblang pass-through, context cancel, StopPipeline=false - Add bridge_test.go: configToYAML (simple, nested, empty), messageToMap (JSON body, plain-text body, metadata, empty), mapToMessage (string body, JSON body, no body, metadata, empty), round-trip - Import pure Bento components in processor tests to enable bloblang processor - Fix pre-existing lint issues: errcheck in bridge.go, nolint for scaffolded ensureStream in broker_module.go Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
intel352
force-pushed
the
feat/issue-2-unit-tests
branch
from
af6eab3 to
45c0625
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Closes #2
Test plan
-raceflaggolangci-lint runpasses with 0 issuesgo build ./...)Notes
purecomponents side-effect importTestStreamModule_Stop_WithoutStarttest correctly handles the blockingdonechannel by relying on context cancellation