resolve comments

Signed-off-by: Said Altury <Said.Altury@ibm.com>

Author: SaidAltury-ibm

platform/fabricx/core/finality/provider.go

@@ -41,7 +41,6 @@ func NewListenerManagerProvider(fnsp *fabric.NetworkServiceProvider, configProvi
 		fnsp:                   fnsp,
 		configProvider:         configProvider,
 		managers:               make(map[string]ListenerManager),
-		initOnce:               sync.Once{},
 		newNotificationManager: newNotifi,
 		// Note: baseCtx and initOnce will be initialized in the Initialize method.
 	}

platform/fabricx/core/finality/provider_test.go

@@ -11,7 +11,6 @@ import (
 	"errors"
 	"reflect"
 	"sync"
-	"sync/atomic"
 	"testing"
 	"time"
 
@@ -70,23 +69,22 @@ func TestProvider_Initialize(t *testing.T) {
 		ctx := t.Context()
 		provider.Initialize(ctx)
 
-		require.NotNil(t, provider.baseCtx, "baseCtx should be set after Initialize")
-		assert.Same(t, ctx, provider.baseCtx, "baseCtx should match the provided context")
+		require.Same(t, ctx, provider.baseCtx, "baseCtx should match the provided context")
 	})
 
 	t.Run("Initialize_Only_Once_Ignores_Subsequent_Calls", func(t *testing.T) {
 		t.Parallel()
 		provider := NewListenerManagerProvider(nil, nil)
 
-		ctx1 := t.Context()                                       //context.WithValue(context.Background(), "key", "value1")
-		ctx2, cancel2 := context.WithCancel(context.Background()) //context.WithValue(context.Background(), "key", "value2")
+		ctx1 := t.Context()                                       
+		ctx2, cancel2 := context.WithCancel(context.Background()) 
 		defer cancel2()
 
 		provider.Initialize(ctx1)
 		provider.Initialize(ctx2) // Should be ignored due to sync.Once
 
-		assert.Same(t, ctx1, provider.baseCtx, "baseCtx should remain as first initialized value")
-		assert.NotSame(t, ctx2, provider.baseCtx, "Second Initialize call should be ignored")
+		require.Same(t, ctx1, provider.baseCtx, "baseCtx should remain as first initialized value")
+		require.NotSame(t, ctx2, provider.baseCtx, "Second Initialize call should be ignored")
 	})
 
 	t.Run("Thread_Safe_Concurrent_Initialize", func(t *testing.T) {
@@ -115,7 +113,7 @@ func TestProvider_NewManager(t *testing.T) {
 		provider := NewListenerManagerProvider(nil, nil)
 
 		// Don't call Initialize
-		assert.Panics(t, func() {
+		require.Panics(t, func() {
 			_, _ = provider.NewManager("network1", "channel1")
 		}, "Should panic when Provider is not initialized")
 	})
@@ -158,18 +156,15 @@ func TestProvider_NewManager(t *testing.T) {
 		storedManager := provider.managers["network1:channel1"]
 		provider.managersMu.Unlock()
 
-		assert.Equal(t, manager, storedManager, "Manager should be stored in map with correct key")
+		require.Same(t, manager, storedManager, "Manager should be stored in map with correct key")
 	})
 
 	t.Run("Returns_Existing_Manager_Singleton_Pattern", func(t *testing.T) {
 		t.Parallel()
 		provider := NewListenerManagerProvider(nil, nil)
 
-		var createCount atomic.Int32
-
 		// Mock with stream that blocks
 		provider.newNotificationManager = func(network string, fnsp *fabric.NetworkServiceProvider, cp config.Provider) (*notificationListenerManager, error) {
-			createCount.Add(1)
 			return setupMockNotificationManager(t, func(ctx context.Context, stream *mock2.FakeNotifier_OpenNotificationStreamClient) {
 				stream.RecvStub = func() (*protonotify.NotificationResponse, error) {
 					<-ctx.Done()
@@ -192,18 +187,15 @@ func TestProvider_NewManager(t *testing.T) {
 		require.NoError(t, err2)
 
 		// Should be the exact same instance
-		assert.Equal(t, manager1, manager2, "Should return same manager instance for same network:channel")
-		assert.Equal(t, int32(1), createCount.Load(), "Should only create manager once")
+		require.Same(t, manager1, manager2, "Should return same manager instance for same network:channel")
+		require.Equal(t, 1, len(provider.managers), "Should only create manager once")
 	})
 
 	t.Run("Different_Network_Channel_Get_Different_Managers", func(t *testing.T) {
 		t.Parallel()
 		provider := NewListenerManagerProvider(nil, nil)
 
-		var createCount atomic.Int32
-
 		provider.newNotificationManager = func(network string, fnsp *fabric.NetworkServiceProvider, cp config.Provider) (*notificationListenerManager, error) {
-			createCount.Add(1)
 			return setupMockNotificationManager(t, func(ctx context.Context, stream *mock2.FakeNotifier_OpenNotificationStreamClient) {
 				stream.RecvStub = func() (*protonotify.NotificationResponse, error) {
 					<-ctx.Done()
@@ -225,11 +217,11 @@ func TestProvider_NewManager(t *testing.T) {
 		require.NoError(t, err3)
 
 		// All should be different instances
-		assert.NotSame(t, manager1, manager2, "Different channels should have different managers")
-		assert.NotSame(t, manager1, manager3, "Different networks should have different managers")
-		assert.NotSame(t, manager2, manager3, "Different combinations should have different managers")
-
-		assert.Equal(t, int32(3), createCount.Load(), "Should create 3 different managers")
+		require.NotSame(t, manager1, manager2, "Different channels should have different managers")
+		require.NotSame(t, manager1, manager3, "Different networks should have different managers")
+		require.NotSame(t, manager2, manager3, "Different combinations should have different managers")
+		
+		require.Equal(t, 3, len(provider.managers), "Should create 3 different managers")
 	})
 
 	t.Run("Manager_Removed_When_Listen_Exits_With_Error", func(t *testing.T) {
@@ -258,11 +250,11 @@ func TestProvider_NewManager(t *testing.T) {
 		<-listenStarted
 
 		// Wait for cleanup - the REAL listen() will exit and trigger cleanup
-		require.Eventually(t, func() bool {
+		require.EventuallyWithT(t, func(collect *assert.CollectT) {
 			provider.managersMu.Lock()
 			defer provider.managersMu.Unlock()
 			_, exists := provider.managers["network1:channel1"]
-			return !exists
+			require.False(collect, exists, "Manager should be removed from map")
 		}, managerTimeout, managerTick, "Manager should be removed after listen() exits with error")
 	})
 
@@ -296,11 +288,11 @@ func TestProvider_NewManager(t *testing.T) {
 		cancel()
 
 		// The REAL listen() will handle context cancellation and exit gracefully
-		require.Eventually(t, func() bool {
+		require.EventuallyWithT(t, func(collect *assert.CollectT) {
 			provider.managersMu.Lock()
 			defer provider.managersMu.Unlock()
 			_, exists := provider.managers["network1:channel1"]
-			return !exists
+			require.False(collect, exists, "Manager should be removed from map")
 		}, managerTimeout, managerTick, "Manager should be removed after context cancellation")
 	})
 
@@ -321,25 +313,22 @@ func TestProvider_NewManager(t *testing.T) {
 		manager, err := provider.NewManager("network1", "channel1")
 
 		require.Error(t, err, "Should return error from newNotificationManager")
-		assert.Nil(t, manager, "Manager should be nil on error")
-		assert.Equal(t, expectedErr, err, "Should return the exact error")
+		require.Nil(t, manager, "Manager should be nil on error")
+		require.Equal(t, expectedErr, err, "Should return the exact error")
 
 		// Manager should not be stored
 		provider.managersMu.Lock()
 		_, exists := provider.managers["network1:channel1"]
 		provider.managersMu.Unlock()
 
-		assert.False(t, exists, "Failed manager should not be stored")
+		require.False(t, exists, "Failed manager should not be stored")
 	})
 
 	t.Run("Concurrent_NewManager_Calls_Same_Key", func(t *testing.T) {
 		t.Parallel()
 		provider := NewListenerManagerProvider(nil, nil)
 
-		var createCount atomic.Int32
-
 		provider.newNotificationManager = func(network string, fnsp *fabric.NetworkServiceProvider, cp config.Provider) (*notificationListenerManager, error) {
-			createCount.Add(1)
 			return setupMockNotificationManager(t, func(ctx context.Context, stream *mock2.FakeNotifier_OpenNotificationStreamClient) {
 				stream.RecvStub = func() (*protonotify.NotificationResponse, error) {
 					<-ctx.Done()
@@ -369,12 +358,11 @@ func TestProvider_NewManager(t *testing.T) {
 
 		// All should be the same instance
 		for i := 1; i < numGoroutines; i++ {
-			assert.Equal(t, managers[0], managers[i], "All concurrent calls should get same manager")
+			require.Equal(t, managers[0], managers[i], "All concurrent calls should get same manager")
 		}
 
 		// Should only create manager once
-		count := createCount.Load()
-		assert.Equal(t, count, int32(1), "Should create manager at most twice even with concurrent access")
+		require.Equal(t, len(provider.managers), 1, "Should create manager only once even with concurrent access")
 	})
 }
 
@@ -407,9 +395,9 @@ func TestGetListenerManager(t *testing.T) {
 		require.NotNil(t, manager, "Manager should not be nil")
 
 		// Verify GetService was called with correct type
-		assert.Equal(t, 1, mockSP.GetServiceCallCount())
+		require.Equal(t, 1, mockSP.GetServiceCallCount())
 		serviceType := mockSP.GetServiceArgsForCall(0)
-		assert.Equal(t, reflect.TypeOf((*ListenerManagerProvider)(nil)), serviceType)
+		require.Equal(t, reflect.TypeOf((*ListenerManagerProvider)(nil)), serviceType)
 	})
 
 	t.Run("Returns_Error_When_Service_Not_Found", func(t *testing.T) {
@@ -425,8 +413,8 @@ func TestGetListenerManager(t *testing.T) {
 		manager, err := GetListenerManager(mockSP, "network1", "channel1")
 
 		require.Error(t, err, "Should return error when service not found")
-		assert.Nil(t, manager, "Manager should be nil on error")
-		assert.Contains(t, err.Error(), "could not find provider")
+		require.Nil(t, manager, "Manager should be nil on error")
+		require.Contains(t, err.Error(), "could not find provider")
 	})
 
 	t.Run("Returns_Error_When_NewManager_Fails", func(t *testing.T) {
@@ -451,7 +439,7 @@ func TestGetListenerManager(t *testing.T) {
 		manager, err := GetListenerManager(mockSP, "network1", "channel1")
 
 		require.Error(t, err, "Should return error from NewManager")
-		assert.Nil(t, manager)
-		assert.Equal(t, expectedErr, err)
+		require.Nil(t, manager)
+		require.Equal(t, expectedErr, err)
 	})
 }