From 05cf9043b57362ce93f818cbf45feac462413f20 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Sat, 11 Jul 2015 10:54:18 +0200 Subject: [PATCH 1/5] Update stretchr/testify godep Fix panic in tests using mock due to stretchr/testify#180 --- Godeps/Godeps.json | 6 ++--- .../stretchr/testify/assert/assertions.go | 7 +++++- .../testify/assert/assertions_test.go | 22 +++++++++++++++++++ 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index ce35855f176..6898d12e09b 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -496,15 +496,15 @@ }, { "ImportPath": "github.com/stretchr/testify/assert", - "Rev": "7e4a149930b09fe4c2b134c50ce637457ba6e966" + "Rev": "089c7181b8c728499929ff09b62d3fdd8df8adff" }, { "ImportPath": "github.com/stretchr/testify/mock", - "Rev": "7e4a149930b09fe4c2b134c50ce637457ba6e966" + "Rev": "089c7181b8c728499929ff09b62d3fdd8df8adff" }, { "ImportPath": "github.com/stretchr/testify/require", - "Rev": "7e4a149930b09fe4c2b134c50ce637457ba6e966" + "Rev": "089c7181b8c728499929ff09b62d3fdd8df8adff" }, { "ImportPath": "github.com/syndtr/gocapability/capability", diff --git a/Godeps/_workspace/src/github.com/stretchr/testify/assert/assertions.go b/Godeps/_workspace/src/github.com/stretchr/testify/assert/assertions.go index 7b5ce7257e0..fbf03f4d82f 100644 --- a/Godeps/_workspace/src/github.com/stretchr/testify/assert/assertions.go +++ b/Godeps/_workspace/src/github.com/stretchr/testify/assert/assertions.go @@ -84,6 +84,11 @@ func CallerInfo() []string { return nil } + // This is a huge edge case, but it will panic if this is the case, see #180 + if file == "" { + break + } + parts := strings.Split(file, "/") dir := parts[len(parts)-2] file = parts[len(parts)-1] @@ -296,7 +301,7 @@ func NotNil(t TestingT, object interface{}, msgAndArgs ...interface{}) bool { } if !success { - Fail(t, "Expected not to be nil.", msgAndArgs...) + Fail(t, "Expected value not to be nil.", msgAndArgs...) } return success diff --git a/Godeps/_workspace/src/github.com/stretchr/testify/assert/assertions_test.go b/Godeps/_workspace/src/github.com/stretchr/testify/assert/assertions_test.go index d859c77b912..36c671eefef 100644 --- a/Godeps/_workspace/src/github.com/stretchr/testify/assert/assertions_test.go +++ b/Godeps/_workspace/src/github.com/stretchr/testify/assert/assertions_test.go @@ -2,6 +2,7 @@ package assert import ( "errors" + "io" "math" "regexp" "testing" @@ -789,3 +790,24 @@ func TestRegexp(t *testing.T) { True(t, NotRegexp(mockT, regexp.MustCompile(tc.rx), tc.str)) } } + +func testAutogeneratedFunction() { + defer func() { + if err := recover(); err == nil { + panic("did not panic") + } + CallerInfo() + }() + t := struct { + io.Closer + }{} + var c io.Closer + c = t + c.Close() +} + +func TestCallerInfoWithAutogeneratedFunctions(t *testing.T) { + NotPanics(t, func() { + testAutogeneratedFunction() + }) +} From bf44f5df28d63891339761ac3d58d9aba23834eb Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Sat, 11 Jul 2015 11:07:14 +0200 Subject: [PATCH 2/5] Add DeclineOffer return value to mock driver in mesos scheduler test Depending on timing the mesos scheduler might call DeclineOffer: The default ttl of an offer in mesos scheduler is 5sec. If the tests run longer, the old, unused offers are declined, leading to an mock error. Probably fixes GoogleCloudPlatform/kubernetes#10795 --- contrib/mesos/pkg/scheduler/plugin_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contrib/mesos/pkg/scheduler/plugin_test.go b/contrib/mesos/pkg/scheduler/plugin_test.go index 19ccf22c8ff..d6dc121d14e 100644 --- a/contrib/mesos/pkg/scheduler/plugin_test.go +++ b/contrib/mesos/pkg/scheduler/plugin_test.go @@ -440,6 +440,8 @@ func TestPlugin_LifeCycle(t *testing.T) { } mockDriver.On("LaunchTasks", mAny("[]*mesosproto.OfferID"), mAny("[]*mesosproto.TaskInfo"), mAny("*mesosproto.Filters")). Return(mesos.Status_DRIVER_RUNNING, nil).Run(launchTasksCalledFunc) + mockDriver.On("DeclineOffer", mAny("*mesosproto.OfferID"), mAny("*mesosproto.Filters")). + Return(mesos.Status_DRIVER_RUNNING, nil) // elect master with mock driver driverFactory := ha.DriverFactory(func() (bindings.SchedulerDriver, error) { From dd7345b25fa117f24f421052059a36869e4bc25a Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Sat, 11 Jul 2015 12:14:55 +0200 Subject: [PATCH 3/5] Fix offer+pod races in mesos scheduler plugin test - Offers were reused and led to unexpected declining by the scheduler because the reused offer did not get a new expiration time. - Pod scheduling and offer creation was not synchronized. When scheduling happened after aging of offers, the first issue was trigger. Because the mesos driver DeclineOffer was not mocked this lead to a test error. --- contrib/mesos/pkg/scheduler/plugin_test.go | 24 ++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/contrib/mesos/pkg/scheduler/plugin_test.go b/contrib/mesos/pkg/scheduler/plugin_test.go index d6dc121d14e..870c1bb7441 100644 --- a/contrib/mesos/pkg/scheduler/plugin_test.go +++ b/contrib/mesos/pkg/scheduler/plugin_test.go @@ -499,14 +499,22 @@ func TestPlugin_LifeCycle(t *testing.T) { } // start another pod - podNum := 1 - startPod := func(offers []*mesos.Offer) (*api.Pod, *mesos.TaskInfo) { + podNum := 2 + startPod := func() (*api.Pod, *mesos.TaskInfo) { podNum = podNum + 1 - // create pod and matching offer + // create pod pod := NewTestPod(podNum) podListWatch.Add(pod, true) // notify watchers + + // wait for failedScheduling event because there is no offer + assert.EventWithReason(eventObserver, "failedScheduling", "failedScheduling event not received") + + // supply a matching offer + offers := []*mesos.Offer{NewTestOffer(podNum)} testScheduler.ResourceOffers(mockDriver, offers) + + // and wait to get scheduled assert.EventWithReason(eventObserver, "scheduled") // wait for driver.launchTasks call @@ -522,7 +530,7 @@ func TestPlugin_LifeCycle(t *testing.T) { } } - pod, launchedTask := startPod(offers1) + pod, launchedTask := startPod() // mock drvier.KillTask, should be invoked when a pod is deleted mockDriver.On("KillTask", mAny("*mesosproto.TaskID")).Return(mesos.Status_DRIVER_RUNNING, nil).Run(func(args mock.Arguments) { @@ -563,22 +571,22 @@ func TestPlugin_LifeCycle(t *testing.T) { } // 1. with pod deleted from the apiserver - pod, launchedTask = startPod(offers1) + pod, launchedTask = startPod() podListWatch.Delete(pod, false) // not notifying the watchers failPodFromExecutor(launchedTask) // 2. with pod still on the apiserver, not bound - pod, launchedTask = startPod(offers1) + pod, launchedTask = startPod() failPodFromExecutor(launchedTask) // 3. with pod still on the apiserver, bound i.e. host!="" - pod, launchedTask = startPod(offers1) + pod, launchedTask = startPod() pod.Spec.NodeName = *offers1[0].Hostname podListWatch.Modify(pod, false) // not notifying the watchers failPodFromExecutor(launchedTask) // 4. with pod still on the apiserver, bound i.e. host!="", notified via ListWatch - pod, launchedTask = startPod(offers1) + pod, launchedTask = startPod() pod.Spec.NodeName = *offers1[0].Hostname podListWatch.Modify(pod, true) // notifying the watchers time.Sleep(time.Second / 2) From 143cf4b08d57339904ea393cc6092712162b67d5 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Sat, 11 Jul 2015 21:45:47 +0200 Subject: [PATCH 4/5] Use correct offer's hostname of test pods in mesos scheduler plugin tests --- contrib/mesos/pkg/scheduler/plugin_test.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/contrib/mesos/pkg/scheduler/plugin_test.go b/contrib/mesos/pkg/scheduler/plugin_test.go index 870c1bb7441..b1f74e9f6b1 100644 --- a/contrib/mesos/pkg/scheduler/plugin_test.go +++ b/contrib/mesos/pkg/scheduler/plugin_test.go @@ -500,7 +500,7 @@ func TestPlugin_LifeCycle(t *testing.T) { // start another pod podNum := 2 - startPod := func() (*api.Pod, *mesos.TaskInfo) { + startPod := func() (*api.Pod, *mesos.TaskInfo, *mesos.Offer) { podNum = podNum + 1 // create pod @@ -522,15 +522,15 @@ func TestPlugin_LifeCycle(t *testing.T) { case launchedTask := <-launchedTasks: testScheduler.StatusUpdate(mockDriver, newTaskStatusForTask(launchedTask, mesos.TaskState_TASK_STAGING)) testScheduler.StatusUpdate(mockDriver, newTaskStatusForTask(launchedTask, mesos.TaskState_TASK_RUNNING)) - return pod, launchedTask + return pod, launchedTask, offers[0] case <-time.After(5 * time.Second): t.Fatal("timed out waiting for launchTasks") - return nil, nil + return nil, nil, nil } } - pod, launchedTask := startPod() + pod, launchedTask, _ := startPod() // mock drvier.KillTask, should be invoked when a pod is deleted mockDriver.On("KillTask", mAny("*mesosproto.TaskID")).Return(mesos.Status_DRIVER_RUNNING, nil).Run(func(args mock.Arguments) { @@ -571,23 +571,23 @@ func TestPlugin_LifeCycle(t *testing.T) { } // 1. with pod deleted from the apiserver - pod, launchedTask = startPod() + pod, launchedTask, _ = startPod() podListWatch.Delete(pod, false) // not notifying the watchers failPodFromExecutor(launchedTask) // 2. with pod still on the apiserver, not bound - pod, launchedTask = startPod() + pod, launchedTask, _ = startPod() failPodFromExecutor(launchedTask) // 3. with pod still on the apiserver, bound i.e. host!="" - pod, launchedTask = startPod() - pod.Spec.NodeName = *offers1[0].Hostname + pod, launchedTask, usedOffer := startPod() + pod.Spec.NodeName = *usedOffer.Hostname podListWatch.Modify(pod, false) // not notifying the watchers failPodFromExecutor(launchedTask) // 4. with pod still on the apiserver, bound i.e. host!="", notified via ListWatch - pod, launchedTask = startPod() - pod.Spec.NodeName = *offers1[0].Hostname + pod, launchedTask, usedOffer = startPod() + pod.Spec.NodeName = *usedOffer.Hostname podListWatch.Modify(pod, true) // notifying the watchers time.Sleep(time.Second / 2) failPodFromExecutor(launchedTask) From 95c7dc8cb3797cd820f9c22360ba4bbdde9478f6 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Mon, 13 Jul 2015 22:41:51 +0200 Subject: [PATCH 5/5] Re-enable mesos scheduler TestPlugin_LifeCycle test --- contrib/mesos/pkg/scheduler/plugin_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/contrib/mesos/pkg/scheduler/plugin_test.go b/contrib/mesos/pkg/scheduler/plugin_test.go index b1f74e9f6b1..4cf655b5fb2 100644 --- a/contrib/mesos/pkg/scheduler/plugin_test.go +++ b/contrib/mesos/pkg/scheduler/plugin_test.go @@ -370,7 +370,6 @@ func TestPlugin_New(t *testing.T) { // and play through the whole life cycle of the plugin while creating pods, deleting // and failing them. func TestPlugin_LifeCycle(t *testing.T) { - t.Skip("disabled due to flakiness; see #10795") assert := &EventAssertions{*assert.New(t)} // create a fake pod watch. We use that below to submit new pods to the scheduler