gemini-code-assist[bot] commented on code in PR #38979:
URL: https://github.com/apache/beam/pull/38979#discussion_r3418629142
##########
sdks/go/pkg/beam/core/runtime/harness/statemgr_test.go:
##########
@@ -508,3 +510,99 @@ func contains(got, want error) bool {
}
return strings.Contains(got.Error(), want.Error())
}
+
+func TestNewScopedStateReader(t *testing.T) {
+ mgr := &StateChannelManager{}
+ s := NewScopedStateReader(mgr, "inst1")
+ if s == nil {
+ t.Fatal("NewScopedStateReader returned nil")
+ }
+ if s.mgr != mgr {
+ t.Error("mgr field not set")
+ }
+ if s.instID != "inst1" {
+ t.Errorf("instID = %v, want inst1", s.instID)
+ }
+ if s.cache != nil {
+ t.Error("cache should be nil for NewScopedStateReader")
+ }
+}
+
+func TestNewScopedStateReaderWithCache(t *testing.T) {
+ mgr := &StateChannelManager{}
+ cache := &statecache.SideInputCache{}
+ cache.Init(1)
+ s := NewScopedStateReaderWithCache(mgr, "inst2", cache)
+ if s == nil {
+ t.Fatal("NewScopedStateReaderWithCache returned nil")
+ }
+ if s.mgr != mgr {
+ t.Error("mgr field not set")
+ }
+ if s.instID != "inst2" {
+ t.Errorf("instID = %v, want inst2", s.instID)
+ }
+ if s.cache != cache {
+ t.Error("cache field not set correctly")
+ }
+}
+
+func TestScopedStateReader_GetSideInputCache(t *testing.T) {
+ cache := &statecache.SideInputCache{}
+ cache.Init(1)
+ s := NewScopedStateReaderWithCache(&StateChannelManager{}, "inst",
cache)
+ got := s.GetSideInputCache()
+ if got != cache {
+ t.Error("GetSideInputCache returned wrong cache")
+ }
+}
+
+func TestScopedStateReader_Close(t *testing.T) {
+ mgr := &StateChannelManager{}
+ s := NewScopedStateReader(mgr, "inst")
+ err := s.Close()
+ if err != nil {
+ t.Errorf("Close returned error: %v", err)
+ }
+ if !s.closed {
+ t.Error("Close did not set closed flag")
+ }
+ // Second close should not error but should no-op.
+ err = s.Close()
+ if err != nil {
+ t.Errorf("Second Close returned error: %v", err)
+ }
+}
+
+func TestStateChannelManager_PortsInit(t *testing.T) {
+ mgr := &StateChannelManager{}
+ if mgr.ports != nil {
+ t.Error("ports should be nil before first Open call")
+ }
+ // Force-initialize ports by attempting Open; even if it fails, the map
should be created.
+ // Restore: just set it directly since we don't want to dial.
+ mgr.mu.Lock()
+ mgr.ports = make(map[string]*StateChannel)
+ mgr.mu.Unlock()
+ if mgr.ports == nil {
+ t.Error("ports map should exist after initialization")
+ }
Review Comment:

This test does not actually verify any production code behavior. It manually
initializes `mgr.ports` inside the test itself (lines 584-586) and then asserts
that `mgr.ports` is not nil. This only tests that Go's `make` function works,
rather than testing that `StateChannelManager` initializes its ports map during
actual usage (e.g., when `Open` is called). Consider triggering the actual
initialization logic (e.g., by calling `Open` with an invalid port and ignoring
the dial error) to ensure the production initialization path is covered.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]