flowmade-one/aether
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[Issue #18] Add integration tests for NATSEventBus #50

Merged
HugoNijhuis merged 1 commits from issue-18-nats-eventbus-integration-tests into main 2026-01-10 18:27:09 +00:00
Conversation 1 Commits 1 Files Changed 1 +1205
HugoNijhuis commented 2026-01-10 18:24:27 +00:00
Owner
Copy Link

Summary

  • Add comprehensive integration tests for NATSEventBus that verify cross-node event delivery and namespace isolation with a real NATS server
  • Tests cover high-throughput scenarios, event ordering, reconnection behavior, and large payloads
  • All tests are tagged with +build integration and require a running NATS server

Changes

  • Add nats_eventbus_integration_test.go with 16 integration tests and 2 benchmarks
  • Test cross-node event delivery with multiple NATSEventBus instances
  • Test namespace isolation with both single and multiple NATS connections
  • Test high-throughput scenarios (1000 events)
  • Test event ordering within namespace
  • Verify no cross-namespace leakage
  • Test concurrent publish/subscribe operations
  • Test multiple subscribers to same namespace
  • Test event metadata preservation across NATS serialization
  • Test large event payload handling (100KB)
  • Test subscribe/unsubscribe lifecycle
  • Test reconnection behavior and graceful degradation

Test Plan

  • Run unit tests: go test ./...
  • Run integration tests (requires NATS): go test -tags=integration -v ./...
  • Verify tests skip gracefully when NATS is not available

Closes #18

## Summary - Add comprehensive integration tests for NATSEventBus that verify cross-node event delivery and namespace isolation with a real NATS server - Tests cover high-throughput scenarios, event ordering, reconnection behavior, and large payloads - All tests are tagged with `+build integration` and require a running NATS server ## Changes - Add `nats_eventbus_integration_test.go` with 16 integration tests and 2 benchmarks - Test cross-node event delivery with multiple NATSEventBus instances - Test namespace isolation with both single and multiple NATS connections - Test high-throughput scenarios (1000 events) - Test event ordering within namespace - Verify no cross-namespace leakage - Test concurrent publish/subscribe operations - Test multiple subscribers to same namespace - Test event metadata preservation across NATS serialization - Test large event payload handling (100KB) - Test subscribe/unsubscribe lifecycle - Test reconnection behavior and graceful degradation ## Test Plan - [x] Run unit tests: `go test ./...` - [ ] Run integration tests (requires NATS): `go test -tags=integration -v ./...` - [ ] Verify tests skip gracefully when NATS is not available Closes #18
HugoNijhuis added 1 commit 2026-01-10 18:24:28 +00:00
Add integration tests for NATSEventBus
All checks were successful
CI / build (pull_request) Successful in 16s
Details
7a29e08ef5 
Add comprehensive integration tests that verify NATSEventBus behavior
with a real NATS server. Tests cover:

- Cross-node event delivery (multiple NATSEventBus instances)
- Namespace isolation with single and multiple NATS connections
- High-throughput scenarios (1000 events)
- Event ordering within namespace
- No cross-namespace leakage verification
- Concurrent publish/subscribe operations
- Multiple subscribers to same namespace
- Event metadata preservation across NATS
- Large event payload handling (100KB)
- Subscribe/unsubscribe lifecycle
- Reconnection behavior
- Graceful degradation under load
- Benchmarks for publish and publish-receive

Tests require a running NATS server and are tagged with +build integration.
Run with: go test -tags=integration -v ./...

Closes #18

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
HugoNijhuis commented 2026-01-10 18:27:04 +00:00
Author
Owner
Copy Link

AI Code Review

This is an automated review generated by the code-reviewer agent.

Summary

Comprehensive integration test suite for NATSEventBus with excellent coverage of cross-node delivery, namespace isolation, high-throughput scenarios, and edge cases. Tests are well-structured, use appropriate synchronization, and include benchmarks.

Findings

Code Quality

  • Excellent test organization: 16 tests cover all critical scenarios (cross-node delivery, namespace isolation, high throughput, metadata preservation, reconnection)
  • Good use of unique namespaces: Each test generates unique namespace IDs to prevent test interference
  • Clear naming and documentation: Test names clearly indicate what they test, inline comments explain the purpose
  • Proper resource cleanup: Consistent use of defer for closing connections and stopping buses
  • Minor: Deprecated build tag syntax: Line 1 uses old-style build tag syntax. Go 1.17+ prefers //go:build integration format (though // +build integration still works)

Potential Bugs

  • TestNATSEventBus_EventOrdering assumes NATS ordering: Line 338-355 expects strict event ordering, but NATS core (non-JetStream) does not guarantee message order. This test may be flaky under high load or network delays. Consider documenting this assumption or using JetStream streams for ordering guarantees.
  • Race condition in concurrent test: TestNATSEventBus_ConcurrentPublishSubscribe (line 546) publishes from 10 goroutines without waiting for subscription setup to fully propagate. While the 100ms sleep helps, under load this could miss early events. Consider using a sync barrier or waiting for first event.
  • Hardcoded timeouts: Multiple tests use time.Sleep(100 * time.Millisecond) for subscription setup. This is fragile in CI environments or under load. Consider polling with retries or using NATS subscription flush.

Security Concerns

  • No security concerns identified

Style Notes

  • Consistent with codebase: Test structure matches existing tests in event_test.go
  • Good use of table-driven approach: Tests like TestNATSEventBus_MultipleNodesMultipleNamespaces demonstrate complex scenarios effectively
  • Benchmark tests included: BenchmarkNATSEventBus_Publish and BenchmarkNATSEventBus_PublishReceive provide performance baseline

Test Coverage

  • Comprehensive coverage: Cross-node delivery, namespace isolation (single/multiple connections), high throughput (1000 events), event ordering, metadata preservation, large payloads, subscription lifecycle, reconnection, graceful degradation
  • Edge cases well-tested: Subscribe/unsubscribe, multiple subscribers, concurrent operations, stop cleanup
  • Missing: Error injection tests: No tests for NATS connection failures during operation, publish errors, or subscription failures
  • Missing: Test for duplicate event handling: What happens if same event is received twice (network retransmission)?
  • Integration test tag works: Tests properly skip when NATS unavailable

Recommendations

  1. Update build tag to modern syntax: //go:build integration (with blank line before package)
  2. Document ordering assumptions or use JetStream for guaranteed ordering
  3. Replace sleep-based synchronization with NATS Flush() or polling
  4. Add error injection tests for connection failures
  5. Consider adding test for idempotent event handling

Verdict

LGTM

Excellent test suite that thoroughly validates NATSEventBus functionality. The minor issues identified are not blocking - the tests provide strong confidence in cross-node event delivery and namespace isolation. The ordering test assumption should be documented but doesn't prevent merge.

## AI Code Review > This is an automated review generated by the code-reviewer agent. ### Summary Comprehensive integration test suite for NATSEventBus with excellent coverage of cross-node delivery, namespace isolation, high-throughput scenarios, and edge cases. Tests are well-structured, use appropriate synchronization, and include benchmarks. ### Findings #### Code Quality - **Excellent test organization**: 16 tests cover all critical scenarios (cross-node delivery, namespace isolation, high throughput, metadata preservation, reconnection) - **Good use of unique namespaces**: Each test generates unique namespace IDs to prevent test interference - **Clear naming and documentation**: Test names clearly indicate what they test, inline comments explain the purpose - **Proper resource cleanup**: Consistent use of defer for closing connections and stopping buses - **Minor: Deprecated build tag syntax**: Line 1 uses old-style build tag syntax. Go 1.17+ prefers `//go:build integration` format (though `// +build integration` still works) #### Potential Bugs - **TestNATSEventBus_EventOrdering assumes NATS ordering**: Line 338-355 expects strict event ordering, but NATS core (non-JetStream) does not guarantee message order. This test may be flaky under high load or network delays. Consider documenting this assumption or using JetStream streams for ordering guarantees. - **Race condition in concurrent test**: TestNATSEventBus_ConcurrentPublishSubscribe (line 546) publishes from 10 goroutines without waiting for subscription setup to fully propagate. While the 100ms sleep helps, under load this could miss early events. Consider using a sync barrier or waiting for first event. - **Hardcoded timeouts**: Multiple tests use time.Sleep(100 * time.Millisecond) for subscription setup. This is fragile in CI environments or under load. Consider polling with retries or using NATS subscription flush. #### Security Concerns - No security concerns identified #### Style Notes - **Consistent with codebase**: Test structure matches existing tests in event_test.go - **Good use of table-driven approach**: Tests like TestNATSEventBus_MultipleNodesMultipleNamespaces demonstrate complex scenarios effectively - **Benchmark tests included**: BenchmarkNATSEventBus_Publish and BenchmarkNATSEventBus_PublishReceive provide performance baseline #### Test Coverage - **Comprehensive coverage**: Cross-node delivery, namespace isolation (single/multiple connections), high throughput (1000 events), event ordering, metadata preservation, large payloads, subscription lifecycle, reconnection, graceful degradation - **Edge cases well-tested**: Subscribe/unsubscribe, multiple subscribers, concurrent operations, stop cleanup - **Missing: Error injection tests**: No tests for NATS connection failures during operation, publish errors, or subscription failures - **Missing: Test for duplicate event handling**: What happens if same event is received twice (network retransmission)? - **Integration test tag works**: Tests properly skip when NATS unavailable ### Recommendations 1. Update build tag to modern syntax: `//go:build integration` (with blank line before package) 2. Document ordering assumptions or use JetStream for guaranteed ordering 3. Replace sleep-based synchronization with NATS Flush() or polling 4. Add error injection tests for connection failures 5. Consider adding test for idempotent event handling ### Verdict **LGTM** Excellent test suite that thoroughly validates NATSEventBus functionality. The minor issues identified are not blocking - the tests provide strong confidence in cross-node event delivery and namespace isolation. The ordering test assumption should be documented but doesn't prevent merge.
HugoNijhuis merged commit 9e238c5e70 into main 2026-01-10 18:27:09 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
risk/high
risk/low
risk/medium
value/critical
value/high
value/low
value/medium
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
HugoNijhuis fluxcd-bot renovate-bot
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: flowmade-one/aether#50
Delete Branch "issue-18-nats-eventbus-integration-tests"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?