[GitHub] [incubator-yunikorn-k8shim] HuangTing-Yao commented on a change in pull request #248: [YUNIKORN-600] PlaceholderManager structure init

[GitBox <gi...@apache.org>]

HuangTing-Yao commented on a change in pull request #248:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/248#discussion_r601101370



##########
File path: pkg/cache/placeholder_manager_test.go
##########
@@ -220,27 +220,44 @@ func TestCleanOrphanPlaceholders(t *testing.T) {
 			UID:  "UID-01",
 		},
 	}
-	placeholderMgr.orphanPod["task01"] = pod1
-	assert.Equal(t, len(placeholderMgr.orphanPod), 1)
+	placeholderMgr.orphanPods["task01"] = pod1
+	assert.Equal(t, len(placeholderMgr.orphanPods), 1)
 	placeholderMgr.cleanOrphanPlaceholders()
-	assert.Equal(t, len(placeholderMgr.orphanPod), 0)
+	assert.Equal(t, len(placeholderMgr.orphanPods), 0)
 }
 
 func TestPlaceholderManagerStartStop(t *testing.T) {
 	mockedAPIProvider := client.NewMockedAPIProvider()
-	placeholderMgr := &PlaceholderManager{
-		clients:   mockedAPIProvider.GetAPIs(),
-		orphanPod: make(map[string]*v1.Pod),
-		running:   atomic.Value{},
-		RWMutex:   sync.RWMutex{},
-	}
-	placeholderMgr.setRunning(false)
+	mgr := NewPlaceholderManager(mockedAPIProvider.GetAPIs())
+	assert.Equal(t, mgr.running.Load(), false, "new manager should not run")
 	// start clean up goroutine
-	placeholderMgr.Start()
-	assert.Equal(t, placeholderMgr.isRunning(), true)
+	mgr.Start()
+	assert.Equal(t, mgr.isRunning(), true, "manager should be running after start")
+
+	// starting a second time should do nothing
+	mgr.Start()
+	assert.Equal(t, mgr.isRunning(), true, "sending start 2nd time should not do anything")
+
+	// this is a blocking call
+	mgr.Stop()
+	// allow time for processing as the call can return faster than the flag is set
+	time.Sleep(5 * time.Millisecond)

Review comment:
       In `Statt()` it might sleep `100 * time.Millisecond` then check the stop chan, but here only sleep `5 * time.Millisecond`.
   I am not sure it is enough for it.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org