Merge pull request #12797 from wojtek-t/refactor_registry_test

Prepare for refactoring registry etcd tests

pkg/api/rest/resttest/resttest.go

@@ -91,39 +91,99 @@ func copyOrDie(obj runtime.Object) runtime.Object {
 	return out
 }
 
+// Test creating an object.
 func (t *Tester) TestCreate(valid runtime.Object, invalid ...runtime.Object) {
-	t.TestCreateHasMetadata(copyOrDie(valid))
+	t.testCreateHasMetadata(copyOrDie(valid))
-	t.TestCreateGeneratesName(copyOrDie(valid))
+	t.testCreateGeneratesName(copyOrDie(valid))
-	t.TestCreateGeneratesNameReturnsServerTimeout(copyOrDie(valid))
+	t.testCreateGeneratesNameReturnsServerTimeout(copyOrDie(valid))
 	if t.clusterScope {
-		t.TestCreateDiscardsObjectNamespace(copyOrDie(valid))
+		t.testCreateDiscardsObjectNamespace(copyOrDie(valid))
-		t.TestCreateIgnoresContextNamespace(copyOrDie(valid))
+		t.testCreateIgnoresContextNamespace(copyOrDie(valid))
-		t.TestCreateIgnoresMismatchedNamespace(copyOrDie(valid))
+		t.testCreateIgnoresMismatchedNamespace(copyOrDie(valid))
 	} else {
-		t.TestCreateRejectsMismatchedNamespace(copyOrDie(valid))
+		t.testCreateRejectsMismatchedNamespace(copyOrDie(valid))
 	}
-	t.TestCreateInvokesValidation(invalid...)
+	t.testCreateInvokesValidation(invalid...)
 }
 
-func (t *Tester) TestCreateResetsUserData(valid runtime.Object) {
+// Test updating an object.
-	objectMeta := t.getObjectMetaOrFail(valid)
+func (t *Tester) TestUpdate(valid runtime.Object, existing, older runtime.Object) {
-	now := util.Now()
+	t.testUpdateFailsOnNotFound(copyOrDie(valid))
-	objectMeta.UID = "bad-uid"
+	t.testUpdateFailsOnVersion(copyOrDie(older))
-	objectMeta.CreationTimestamp = now
+}
 
-	obj, err := t.storage.(rest.Creater).Create(t.TestContext(), valid)
+// Test deleting an object.
+func (t *Tester) TestDelete(createFn func() runtime.Object, wasGracefulFn func() bool, invalid ...runtime.Object) {
+	t.testDeleteNonExist(createFn)
+	t.testDeleteNoGraceful(createFn, wasGracefulFn)
+	t.testDeleteInvokesValidation(invalid...)
+	// TODO: Test delete namespace mismatch rejection
+	// once #5684 is fixed.
+}
+
+// Test graceful deletion.
+func (t *Tester) TestDeleteGraceful(createFn func() runtime.Object, expectedGrace int64, wasGracefulFn func() bool) {
+	t.testDeleteGracefulHasDefault(createFn(), expectedGrace, wasGracefulFn)
+	t.testDeleteGracefulUsesZeroOnNil(createFn(), 0)
+}
+
+// Test getting object.
+func (t *Tester) TestGet(obj runtime.Object) {
+	t.testGetFound(obj)
+	t.testGetNotFound(obj)
+	t.testGetMimatchedNamespace(obj)
+	if !t.clusterScope {
+		t.testGetDifferentNamespace(obj)
+	}
+}
+
+// =============================================================================
+// Creation tests.
+
+func (t *Tester) testCreateDiscardsObjectNamespace(valid runtime.Object) {
+	objectMeta := t.getObjectMetaOrFail(valid)
+
+	// Ignore non-empty namespace in object meta
+	objectMeta.Namespace = "not-default"
+
+	// Ideally, we'd get an error back here, but at least verify the namespace wasn't persisted
+	created, err := t.storage.(rest.Creater).Create(t.TestContext(), copyOrDie(valid))
 	if err != nil {
 		t.Fatalf("Unexpected error: %v", err)
 	}
-	if obj == nil {
+	createdObjectMeta := t.getObjectMetaOrFail(created)
-		t.Fatalf("Unexpected object from result: %#v", obj)
+	if createdObjectMeta.Namespace != api.NamespaceNone {
-	}
+		t.Errorf("Expected empty namespace on created object, got '%v'", createdObjectMeta.Namespace)
-	if objectMeta.UID == "bad-uid" || objectMeta.CreationTimestamp == now {
-		t.Errorf("ObjectMeta did not reset basic fields: %#v", objectMeta)
 	}
 }
 
-func (t *Tester) TestCreateHasMetadata(valid runtime.Object) {
+func (t *Tester) testCreateGeneratesName(valid runtime.Object) {
+	objectMeta := t.getObjectMetaOrFail(valid)
+	objectMeta.Name = ""
+	objectMeta.GenerateName = "test-"
+
+	_, err := t.storage.(rest.Creater).Create(t.TestContext(), valid)
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+	if objectMeta.Name == "test-" || !strings.HasPrefix(objectMeta.Name, "test-") {
+		t.Errorf("unexpected name: %#v", valid)
+	}
+}
+
+func (t *Tester) testCreateGeneratesNameReturnsServerTimeout(valid runtime.Object) {
+	objectMeta := t.getObjectMetaOrFail(valid)
+	objectMeta.Name = ""
+	objectMeta.GenerateName = "test-"
+	t.withStorageError(errors.NewAlreadyExists("kind", "thing"), func() {
+		_, err := t.storage.(rest.Creater).Create(t.TestContext(), valid)
+		if err == nil || !errors.IsServerTimeout(err) {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+	})
+}
+
+func (t *Tester) testCreateHasMetadata(valid runtime.Object) {
 	objectMeta := t.getObjectMetaOrFail(valid)
 	objectMeta.Name = ""
 	objectMeta.GenerateName = "test-"
@@ -141,72 +201,7 @@ func (t *Tester) TestCreateHasMetadata(valid runtime.Object) {
 	}
 }
 
-func (t *Tester) TestCreateGeneratesName(valid runtime.Object) {
+func (t *Tester) testCreateIgnoresContextNamespace(valid runtime.Object) {
-	objectMeta := t.getObjectMetaOrFail(valid)
-	objectMeta.Name = ""
-	objectMeta.GenerateName = "test-"
-
-	_, err := t.storage.(rest.Creater).Create(t.TestContext(), valid)
-	if err != nil {
-		t.Fatalf("Unexpected error: %v", err)
-	}
-	if objectMeta.Name == "test-" || !strings.HasPrefix(objectMeta.Name, "test-") {
-		t.Errorf("unexpected name: %#v", valid)
-	}
-}
-
-func (t *Tester) TestCreateGeneratesNameReturnsServerTimeout(valid runtime.Object) {
-	objectMeta := t.getObjectMetaOrFail(valid)
-	objectMeta.Name = ""
-	objectMeta.GenerateName = "test-"
-	t.withStorageError(errors.NewAlreadyExists("kind", "thing"), func() {
-		_, err := t.storage.(rest.Creater).Create(t.TestContext(), valid)
-		if err == nil || !errors.IsServerTimeout(err) {
-			t.Fatalf("Unexpected error: %v", err)
-		}
-	})
-}
-
-func (t *Tester) TestCreateInvokesValidation(invalid ...runtime.Object) {
-	for i, obj := range invalid {
-		ctx := t.TestContext()
-		_, err := t.storage.(rest.Creater).Create(ctx, obj)
-		if !errors.IsInvalid(err) {
-			t.Errorf("%d: Expected to get an invalid resource error, got %v", i, err)
-		}
-	}
-}
-
-func (t *Tester) TestCreateRejectsMismatchedNamespace(valid runtime.Object) {
-	objectMeta := t.getObjectMetaOrFail(valid)
-	objectMeta.Namespace = "not-default"
-
-	_, err := t.storage.(rest.Creater).Create(t.TestContext(), valid)
-	if err == nil {
-		t.Errorf("Expected an error, but we didn't get one")
-	} else if !strings.Contains(err.Error(), "does not match the namespace sent on the request") {
-		t.Errorf("Expected 'does not match the namespace sent on the request' error, got '%v'", err.Error())
-	}
-}
-
-func (t *Tester) TestCreateDiscardsObjectNamespace(valid runtime.Object) {
-	objectMeta := t.getObjectMetaOrFail(valid)
-
-	// Ignore non-empty namespace in object meta
-	objectMeta.Namespace = "not-default"
-
-	// Ideally, we'd get an error back here, but at least verify the namespace wasn't persisted
-	created, err := t.storage.(rest.Creater).Create(t.TestContext(), copyOrDie(valid))
-	if err != nil {
-		t.Fatalf("Unexpected error: %v", err)
-	}
-	createdObjectMeta := t.getObjectMetaOrFail(created)
-	if createdObjectMeta.Namespace != api.NamespaceNone {
-		t.Errorf("Expected empty namespace on created object, got '%v'", createdObjectMeta.Namespace)
-	}
-}
-
-func (t *Tester) TestCreateIgnoresContextNamespace(valid runtime.Object) {
 	// Ignore non-empty namespace in context
 	ctx := api.WithNamespace(api.NewContext(), "not-default2")
 
@@ -221,7 +216,7 @@ func (t *Tester) TestCreateIgnoresContextNamespace(valid runtime.Object) {
 	}
 }
 
-func (t *Tester) TestCreateIgnoresMismatchedNamespace(valid runtime.Object) {
+func (t *Tester) testCreateIgnoresMismatchedNamespace(valid runtime.Object) {
 	objectMeta := t.getObjectMetaOrFail(valid)
 
 	// Ignore non-empty namespace in object meta
@@ -239,12 +234,50 @@ func (t *Tester) TestCreateIgnoresMismatchedNamespace(valid runtime.Object) {
 	}
 }
 
-func (t *Tester) TestUpdate(valid runtime.Object, existing, older runtime.Object) {
+func (t *Tester) testCreateInvokesValidation(invalid ...runtime.Object) {
-	t.TestUpdateFailsOnNotFound(copyOrDie(valid))
+	for i, obj := range invalid {
-	t.TestUpdateFailsOnVersion(copyOrDie(older))
+		ctx := t.TestContext()
+		_, err := t.storage.(rest.Creater).Create(ctx, obj)
+		if !errors.IsInvalid(err) {
+			t.Errorf("%d: Expected to get an invalid resource error, got %v", i, err)
+		}
+	}
 }
 
-func (t *Tester) TestUpdateFailsOnNotFound(valid runtime.Object) {
+func (t *Tester) testCreateRejectsMismatchedNamespace(valid runtime.Object) {
+	objectMeta := t.getObjectMetaOrFail(valid)
+	objectMeta.Namespace = "not-default"
+
+	_, err := t.storage.(rest.Creater).Create(t.TestContext(), valid)
+	if err == nil {
+		t.Errorf("Expected an error, but we didn't get one")
+	} else if !strings.Contains(err.Error(), "does not match the namespace sent on the request") {
+		t.Errorf("Expected 'does not match the namespace sent on the request' error, got '%v'", err.Error())
+	}
+}
+
+func (t *Tester) testCreateResetsUserData(valid runtime.Object) {
+	objectMeta := t.getObjectMetaOrFail(valid)
+	now := util.Now()
+	objectMeta.UID = "bad-uid"
+	objectMeta.CreationTimestamp = now
+
+	obj, err := t.storage.(rest.Creater).Create(t.TestContext(), valid)
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+	if obj == nil {
+		t.Fatalf("Unexpected object from result: %#v", obj)
+	}
+	if objectMeta.UID == "bad-uid" || objectMeta.CreationTimestamp == now {
+		t.Errorf("ObjectMeta did not reset basic fields: %#v", objectMeta)
+	}
+}
+
+// =============================================================================
+// Update tests.
+
+func (t *Tester) testUpdateFailsOnNotFound(valid runtime.Object) {
 	_, _, err := t.storage.(rest.Updater).Update(t.TestContext(), valid)
 	if err == nil {
 		t.Errorf("Expected an error, but we didn't get one")
@@ -253,7 +286,7 @@ func (t *Tester) TestUpdateFailsOnNotFound(valid runtime.Object) {
 	}
 }
 
-func (t *Tester) TestUpdateFailsOnVersion(older runtime.Object) {
+func (t *Tester) testUpdateFailsOnVersion(older runtime.Object) {
 	_, _, err := t.storage.(rest.Updater).Update(t.TestContext(), older)
 	if err == nil {
 		t.Errorf("Expected an error, but we didn't get one")
@@ -262,7 +295,10 @@ func (t *Tester) TestUpdateFailsOnVersion(older runtime.Object) {
 	}
 }
 
-func (t *Tester) TestDeleteInvokesValidation(invalid ...runtime.Object) {
+// =============================================================================
+// Deletion tests.
+
+func (t *Tester) testDeleteInvokesValidation(invalid ...runtime.Object) {
 	for i, obj := range invalid {
 		objectMeta := t.getObjectMetaOrFail(obj)
 		ctx := t.TestContext()
@@ -273,15 +309,7 @@ func (t *Tester) TestDeleteInvokesValidation(invalid ...runtime.Object) {
 	}
 }
 
-func (t *Tester) TestDelete(createFn func() runtime.Object, wasGracefulFn func() bool, invalid ...runtime.Object) {
+func (t *Tester) testDeleteNonExist(createFn func() runtime.Object) {
-	t.TestDeleteNonExist(createFn)
-	t.TestDeleteNoGraceful(createFn, wasGracefulFn)
-	t.TestDeleteInvokesValidation(invalid...)
-	// TODO: Test delete namespace mismatch rejection
-	// once #5684 is fixed.
-}
-
-func (t *Tester) TestDeleteNonExist(createFn func() runtime.Object) {
 	existing := createFn()
 	objectMeta := t.getObjectMetaOrFail(existing)
 	context := t.TestContext()
@@ -294,12 +322,7 @@ func (t *Tester) TestDeleteNonExist(createFn func() runtime.Object) {
 	})
 }
 
-func (t *Tester) TestDeleteGraceful(createFn func() runtime.Object, expectedGrace int64, wasGracefulFn func() bool) {
+func (t *Tester) testDeleteNoGraceful(createFn func() runtime.Object, wasGracefulFn func() bool) {
-	t.TestDeleteGracefulHasDefault(createFn(), expectedGrace, wasGracefulFn)
-	t.TestDeleteGracefulUsesZeroOnNil(createFn(), 0)
-}
-
-func (t *Tester) TestDeleteNoGraceful(createFn func() runtime.Object, wasGracefulFn func() bool) {
 	existing := createFn()
 	objectMeta := t.getObjectMetaOrFail(existing)
 	ctx := api.WithNamespace(t.TestContext(), objectMeta.Namespace)
@@ -315,7 +338,10 @@ func (t *Tester) TestDeleteNoGraceful(createFn func() runtime.Object, wasGracefu
 	}
 }
 
-func (t *Tester) TestDeleteGracefulHasDefault(existing runtime.Object, expectedGrace int64, wasGracefulFn func() bool) {
+// =============================================================================
+// Graceful Deletion tests.
+
+func (t *Tester) testDeleteGracefulHasDefault(existing runtime.Object, expectedGrace int64, wasGracefulFn func() bool) {
 	objectMeta := t.getObjectMetaOrFail(existing)
 	ctx := api.WithNamespace(t.TestContext(), objectMeta.Namespace)
 	_, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, &api.DeleteOptions{})
@@ -330,7 +356,7 @@ func (t *Tester) TestDeleteGracefulHasDefault(existing runtime.Object, expectedG
 	}
 }
 
-func (t *Tester) TestDeleteGracefulUsesZeroOnNil(existing runtime.Object, expectedGrace int64) {
+func (t *Tester) testDeleteGracefulUsesZeroOnNil(existing runtime.Object, expectedGrace int64) {
 	objectMeta := t.getObjectMetaOrFail(existing)
 	ctx := api.WithNamespace(t.TestContext(), objectMeta.Namespace)
 	_, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, nil)
@@ -342,68 +368,11 @@ func (t *Tester) TestDeleteGracefulUsesZeroOnNil(existing runtime.Object, expect
 	}
 }
 
-func (t *Tester) TestGetFound(obj runtime.Object) {
+// =============================================================================
-	ctx := t.TestContext()
+// Get tests.
-	objMeta := t.getObjectMetaOrFail(obj)
-	objMeta.Name = "foo1"
-	objMeta.Namespace = api.NamespaceValue(ctx)
 
-	existing, err := t.storage.(rest.Creater).Create(ctx, obj)
+// testGetDifferentNamespace ensures same-name objects in different namespaces do not clash
-	if err != nil {
+func (t *Tester) testGetDifferentNamespace(obj runtime.Object) {
-		t.Errorf("unexpected error: %v", err)
-	}
-	existingMeta := t.getObjectMetaOrFail(existing)
-
-	got, err := t.storage.(rest.Getter).Get(ctx, "foo1")
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-	gotMeta := t.getObjectMetaOrFail(got)
-	gotMeta.ResourceVersion = existingMeta.ResourceVersion
-	if e, a := existing, got; !api.Semantic.DeepEqual(e, a) {
-		t.Errorf("unexpected obj: %#v, expected %#v", e, a)
-	}
-}
-
-func (t *Tester) TestGetNotFound(obj runtime.Object) {
-	ctx := t.TestContext()
-	objMeta := t.getObjectMetaOrFail(obj)
-	objMeta.Name = "foo2"
-	objMeta.Namespace = api.NamespaceValue(ctx)
-	_, err := t.storage.(rest.Creater).Create(ctx, obj)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-	_, err = t.storage.(rest.Getter).Get(ctx, "foo3")
-	if !errors.IsNotFound(err) {
-		t.Errorf("unexpected error returned: %#v", err)
-	}
-}
-
-func (t *Tester) TestGetMimatchedNamespace(obj runtime.Object) {
-	ctx1 := api.WithNamespace(api.NewContext(), "bar1")
-	ctx2 := api.WithNamespace(api.NewContext(), "bar2")
-	objMeta := t.getObjectMetaOrFail(obj)
-	objMeta.Name = "foo4"
-	objMeta.Namespace = api.NamespaceValue(ctx1)
-	_, err := t.storage.(rest.Creater).Create(ctx1, obj)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-	_, err = t.storage.(rest.Getter).Get(ctx2, "foo4")
-	if t.clusterScope {
-		if err != nil {
-			t.Errorf("unexpected error: %v", err)
-		}
-	} else {
-		if !errors.IsNotFound(err) {
-			t.Errorf("unexpected error returned: %#v", err)
-		}
-	}
-}
-
-// TestGetDifferentNamespace ensures same-name objects in different namespaces do not clash
-func (t *Tester) TestGetDifferentNamespace(obj runtime.Object) {
 	if t.clusterScope {
 		t.Fatalf("the test does not work in in cluster-scope")
 	}
@@ -450,11 +419,62 @@ func (t *Tester) TestGetDifferentNamespace(obj runtime.Object) {
 	}
 }
 
-func (t *Tester) TestGet(obj runtime.Object) {
+func (t *Tester) testGetFound(obj runtime.Object) {
-	t.TestGetFound(obj)
+	ctx := t.TestContext()
-	t.TestGetNotFound(obj)
+	objMeta := t.getObjectMetaOrFail(obj)
-	t.TestGetMimatchedNamespace(obj)
+	objMeta.Name = "foo1"
-	if !t.clusterScope {
+	objMeta.Namespace = api.NamespaceValue(ctx)
-		t.TestGetDifferentNamespace(obj)
+
+	existing, err := t.storage.(rest.Creater).Create(ctx, obj)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	existingMeta := t.getObjectMetaOrFail(existing)
+
+	got, err := t.storage.(rest.Getter).Get(ctx, "foo1")
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	gotMeta := t.getObjectMetaOrFail(got)
+	gotMeta.ResourceVersion = existingMeta.ResourceVersion
+	if e, a := existing, got; !api.Semantic.DeepEqual(e, a) {
+		t.Errorf("unexpected obj: %#v, expected %#v", e, a)
+	}
+}
+
+func (t *Tester) testGetMimatchedNamespace(obj runtime.Object) {
+	ctx1 := api.WithNamespace(api.NewContext(), "bar1")
+	ctx2 := api.WithNamespace(api.NewContext(), "bar2")
+	objMeta := t.getObjectMetaOrFail(obj)
+	objMeta.Name = "foo4"
+	objMeta.Namespace = api.NamespaceValue(ctx1)
+	_, err := t.storage.(rest.Creater).Create(ctx1, obj)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	_, err = t.storage.(rest.Getter).Get(ctx2, "foo4")
+	if t.clusterScope {
+		if err != nil {
+			t.Errorf("unexpected error: %v", err)
+		}
+	} else {
+		if !errors.IsNotFound(err) {
+			t.Errorf("unexpected error returned: %#v", err)
+		}
+	}
+}
+
+func (t *Tester) testGetNotFound(obj runtime.Object) {
+	ctx := t.TestContext()
+	objMeta := t.getObjectMetaOrFail(obj)
+	objMeta.Name = "foo2"
+	objMeta.Namespace = api.NamespaceValue(ctx)
+	_, err := t.storage.(rest.Creater).Create(ctx, obj)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	_, err = t.storage.(rest.Getter).Get(ctx, "foo3")
+	if !errors.IsNotFound(err) {
+		t.Errorf("unexpected error returned: %#v", err)
 	}
 }

pkg/registry/endpoint/etcd/etcd_test.go

@@ -110,7 +110,7 @@ func TestDelete(t *testing.T) {
 		}
 		return fakeEtcdClient.Data[key].R.Node.TTL == 30
 	}
-	test.TestDeleteNoGraceful(createFn, gracefulSetFn)
+	test.TestDelete(createFn, gracefulSetFn)
 }
 
 func TestEtcdListEndpoints(t *testing.T) {

pkg/registry/horizontalpodautoscaler/etcd/etcd_test.go

@@ -133,7 +133,7 @@ func TestDelete(t *testing.T) {
 		}
 		return fakeEtcdClient.Data[key].R.Node.TTL == 30
 	}
-	test.TestDeleteNoGraceful(createFn, gracefulSetFn)
+	test.TestDelete(createFn, gracefulSetFn)
 }
 
 func TestGet(t *testing.T) {

pkg/registry/minion/etcd/etcd_test.go

@@ -127,7 +127,7 @@ func TestDelete(t *testing.T) {
 		}
 		return fakeEtcdClient.Data[key].R.Node.TTL == 30
 	}
-	test.TestDeleteNoGraceful(createFn, gracefulSetFn)
+	test.TestDelete(createFn, gracefulSetFn)
 }
 
 func TestEtcdListNodes(t *testing.T) {

pkg/registry/persistentvolume/etcd/etcd_test.go

@@ -113,7 +113,7 @@ func TestDelete(t *testing.T) {
 		}
 		return fakeEtcdClient.Data[key].R.Node.TTL == 30
 	}
-	test.TestDeleteNoGraceful(createFn, gracefulSetFn)
+	test.TestDelete(createFn, gracefulSetFn)
 }
 
 func TestEtcdListPersistentVolumes(t *testing.T) {

pkg/registry/persistentvolumeclaim/etcd/etcd_test.go

@@ -111,7 +111,7 @@ func TestDelete(t *testing.T) {
 		}
 		return fakeEtcdClient.Data[key].R.Node.TTL == 30
 	}
-	test.TestDeleteNoGraceful(createFn, gracefulSetFn)
+	test.TestDelete(createFn, gracefulSetFn)
 }
 
 func TestEtcdListPersistentVolumeClaims(t *testing.T) {