diff --git a/apis/duck/podspec_test.go b/apis/duck/podspec_test.go index 1ad5b5cec..00e563c08 100644 --- a/apis/duck/podspec_test.go +++ b/apis/duck/podspec_test.go @@ -42,16 +42,6 @@ type WithPodSpec struct { Template PodSpecable `json:"template,omitempty"` } -// Check that our canonical type implements PodSpecable. -var _ = duck.VerifyType(&WithPod{}, &PodSpecable{}) - -// Check that several Kubernetes built-in type implement PodSpecable. -var _ = duck.VerifyType(&appsv1.ReplicaSet{}, &PodSpecable{}) -var _ = duck.VerifyType(&appsv1.Deployment{}, &PodSpecable{}) -var _ = duck.VerifyType(&appsv1.StatefulSet{}, &PodSpecable{}) -var _ = duck.VerifyType(&appsv1.DaemonSet{}, &PodSpecable{}) -var _ = duck.VerifyType(&batchv1.Job{}, &PodSpecable{}) - var _ duck.Populatable = (*WithPod)(nil) var _ duck.Implementable = (*PodSpecable)(nil) @@ -77,8 +67,18 @@ func (t *WithPod) Populate() { } } -func TestNothing(t *testing.T) { - // Don't test anything, this is simply a demonstration that we - // can Duck type core Kubernetes objects and initializing this - // package is sufficient to test that assertion. +func TestImplementsPodSpecable(t *testing.T) { + instances := []interface{}{ + &WithPod{}, + &appsv1.ReplicaSet{}, + &appsv1.Deployment{}, + &appsv1.StatefulSet{}, + &appsv1.DaemonSet{}, + &batchv1.Job{}, + } + for _, instance := range instances { + if err := duck.VerifyType(instance, &PodSpecable{}); err != nil { + t.Error(err) + } + } } diff --git a/apis/duck/v1alpha1/channelable_types.go b/apis/duck/v1alpha1/channelable_types.go index 2fcb12295..7e7ffd9ad 100644 --- a/apis/duck/v1alpha1/channelable_types.go +++ b/apis/duck/v1alpha1/channelable_types.go @@ -43,8 +43,6 @@ type ChannelSubscriberSpec struct { SinkableDomain string `json:"sinkableDomain,omitempty"` } -// Implementations can verify that they implement Channelable via: -var _ = duck.VerifyType(&Channel{}, &Channelable{}) // Channelable is an Implementable "duck type". var _ duck.Implementable = (*Channelable)(nil) diff --git a/apis/duck/v1alpha1/conditions_types.go b/apis/duck/v1alpha1/conditions_types.go index f069996b7..44b999210 100644 --- a/apis/duck/v1alpha1/conditions_types.go +++ b/apis/duck/v1alpha1/conditions_types.go @@ -93,8 +93,6 @@ func (c *Condition) IsUnknown() bool { return c.Status == corev1.ConditionUnknown } -// Implementations can verify that they implement Conditions via: -var _ = duck.VerifyType(&KResource{}, &Conditions{}) // Conditions is an Implementable "duck type". var _ duck.Implementable = (*Conditions)(nil) diff --git a/apis/duck/v1alpha1/generational_types.go b/apis/duck/v1alpha1/generational_types.go index a3198bdde..c8197f674 100644 --- a/apis/duck/v1alpha1/generational_types.go +++ b/apis/duck/v1alpha1/generational_types.go @@ -27,9 +27,6 @@ import ( // Generation is the schema for the generational portion of the payload type Generation int64 -// Implementations can verify that they implement Generation via: -var emptyGen Generation -var _ = duck.VerifyType(&Generational{}, &emptyGen) // Generation is an Implementable "duck type". var _ duck.Implementable = (*Generation)(nil) diff --git a/apis/duck/v1alpha1/implements_test.go b/apis/duck/v1alpha1/implements_test.go new file mode 100644 index 000000000..44a0be63a --- /dev/null +++ b/apis/duck/v1alpha1/implements_test.go @@ -0,0 +1,44 @@ +/* +Copyright 2018 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + "testing" + + "github.com/knative/pkg/apis/duck" +) + +func TestTypesImplements(t *testing.T) { + var emptyGen Generation + testCases := []struct { + instance interface{} + iface duck.Implementable + }{ + {instance: &Channel{}, iface: &Channelable{}}, + {instance: &KResource{}, iface: &Conditions{}}, + {instance: &Generational{}, iface: &emptyGen}, + {instance: &LegacyTarget{}, iface: &LegacyTargetable{}}, + {instance: &Sink{}, iface: &Sinkable{}}, + {instance: &Subscription{}, iface: &Subscribable{}}, + {instance: &Target{}, iface: &Targetable{}}, + } + for _, tc := range testCases { + if err := duck.VerifyType(tc.instance, tc.iface); err != nil { + t.Error(err) + } + } +} diff --git a/apis/duck/v1alpha1/legacy_targetable_types.go b/apis/duck/v1alpha1/legacy_targetable_types.go index d7c391f2a..6bfc08818 100644 --- a/apis/duck/v1alpha1/legacy_targetable_types.go +++ b/apis/duck/v1alpha1/legacy_targetable_types.go @@ -41,8 +41,6 @@ type LegacyTargetable struct { DomainInternal string `json:"domainInternal,omitempty"` } -// Implementations can verify that they implement LegacyTargetable via: -var _ = duck.VerifyType(&LegacyTarget{}, &LegacyTargetable{}) // LegacyTargetable is an Implementable "duck type". var _ duck.Implementable = (*LegacyTargetable)(nil) diff --git a/apis/duck/v1alpha1/sinkable_types.go b/apis/duck/v1alpha1/sinkable_types.go index 62434b2de..2ad0ab5e3 100644 --- a/apis/duck/v1alpha1/sinkable_types.go +++ b/apis/duck/v1alpha1/sinkable_types.go @@ -33,8 +33,6 @@ type Sinkable struct { DomainInternal string `json:"domainInternal,omitempty"` } -// Implementations can verify that they implement Sinkable via: -var _ = duck.VerifyType(&Sink{}, &Sinkable{}) // Sinkable is an Implementable "duck type". var _ duck.Implementable = (*Sinkable)(nil) diff --git a/apis/duck/v1alpha1/subscribable_types.go b/apis/duck/v1alpha1/subscribable_types.go index d73a75dc1..e8d03f890 100644 --- a/apis/duck/v1alpha1/subscribable_types.go +++ b/apis/duck/v1alpha1/subscribable_types.go @@ -36,8 +36,6 @@ type Subscribable struct { Channelable corev1.ObjectReference `json:"channelable,omitempty"` } -// Implementations can verify that they implement Subscribable via: -var _ = duck.VerifyType(&Subscription{}, &Subscribable{}) // Subscribable is an Implementable "duck type". var _ duck.Implementable = (*Subscribable)(nil) diff --git a/apis/duck/v1alpha1/targetable_types.go b/apis/duck/v1alpha1/targetable_types.go index 863e39f7d..5cd454ba5 100644 --- a/apis/duck/v1alpha1/targetable_types.go +++ b/apis/duck/v1alpha1/targetable_types.go @@ -33,8 +33,6 @@ type Targetable struct { DomainInternal string `json:"domainInternal,omitempty"` } -// Implementations can verify that they implement Targetable via: -var _ = duck.VerifyType(&Target{}, &Targetable{}) // Targetable is an Implementable "duck type". var _ duck.Implementable = (*Targetable)(nil) diff --git a/apis/duck/verify.go b/apis/duck/verify.go index 2be7976b2..b55efa093 100644 --- a/apis/duck/verify.go +++ b/apis/duck/verify.go @@ -48,11 +48,10 @@ type Populatable interface { // type ConcreteResource struct { ... } // // // Check that ConcreteResource properly implement Fooable. -// var _ = duck.VerifyType(&ConcreteResource{}, &something.Fooable{}) +// err := duck.VerifyType(&ConcreteResource{}, &something.Fooable{}) // -// This will panic on startup if the duck typing is not satisfied. The return -// value is purely cosmetic to enable the `var _ = ...` shorthand. -func VerifyType(instance interface{}, iface Implementable) (nothing interface{}) { +// This will return an error if the duck typing is not satisfied. +func VerifyType(instance interface{}, iface Implementable) error { // Create instances of the full resource for our input and ultimate result // that we will compare at the end. input, output := iface.GetFullType(), iface.GetFullType() @@ -63,24 +62,24 @@ func VerifyType(instance interface{}, iface Implementable) (nothing interface{}) // Serialize the input to JSON and deserialize that into the provided instance // of the type that we are checking. if before, err := json.Marshal(input); err != nil { - panic(fmt.Sprintf("Error serializing duck type %T", input)) + return fmt.Errorf("error serializing duck type %T", input) } else if err := json.Unmarshal(before, instance); err != nil { - panic(fmt.Sprintf("Error deserializing duck type %T into %T", input, instance)) + return fmt.Errorf("error deserializing duck type %T into %T", input, instance) } // Serialize the instance we are checking to JSON and deserialize that into the // output resource. if after, err := json.Marshal(instance); err != nil { - panic(fmt.Sprintf("Error serializing %T", instance)) + return fmt.Errorf("error serializing %T", instance) } else if err := json.Unmarshal(after, output); err != nil { - panic(fmt.Sprintf("Error deserializing %T into dock type %T", instance, output)) + return fmt.Errorf("error deserializing %T into dock type %T", instance, output) } // Now verify that we were able to roundtrip all of our fields through the type // we are checking. if diff := cmp.Diff(input, output); diff != "" { - panic(fmt.Sprintf("%T does not implement the duck type %T, the following fields were lost: %s", - instance, iface, diff)) + return fmt.Errorf("%T does not implement the duck type %T, the following fields were lost: %s", + instance, iface, diff) } - return + return nil } diff --git a/apis/duck/verify_test.go b/apis/duck/verify_test.go index 211b13f5c..f1bfba48a 100644 --- a/apis/duck/verify_test.go +++ b/apis/duck/verify_test.go @@ -97,14 +97,10 @@ func TestMatches(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - // If we panic, turn it into a test failure - defer func() { - if r := recover(); r != nil { - t.Errorf("panic: %v", r) - } - }() + if err := VerifyType(test.instance, test.iface); err != nil { + t.Error(err) + } - VerifyType(test.instance, test.iface) }) } } @@ -146,15 +142,9 @@ func TestMismatches(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - // Catch panics, they are the failure mode we expect. - defer func() { - if r := recover(); r != nil { - return - } + if err := VerifyType(test.instance, test.iface); err == nil { t.Errorf("Unexpected success %T implements %T", test.instance, test.iface) - }() - - VerifyType(test.instance, test.iface) + } }) } } diff --git a/testing/resource.go b/testing/resource.go index 498e88e69..8eb51f2c5 100644 --- a/testing/resource.go +++ b/testing/resource.go @@ -23,8 +23,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "github.com/knative/pkg/apis" - "github.com/knative/pkg/apis/duck" - duckv1alpha1 "github.com/knative/pkg/apis/duck/v1alpha1" ) // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -44,9 +42,6 @@ var _ apis.Immutable = (*Resource)(nil) var _ apis.Listable = (*Resource)(nil) // Check that we implement the Generation duck type. -var emptyGen duckv1alpha1.Generation -var _ = duck.VerifyType(&Resource{}, &emptyGen) - type ResourceSpec struct { Generation int64 `json:"generation,omitempty"` diff --git a/testing/resource_test.go b/testing/resource_test.go new file mode 100644 index 000000000..4f829b0e4 --- /dev/null +++ b/testing/resource_test.go @@ -0,0 +1,31 @@ +/* +Copyright 2018 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package testing + +import ( + "testing" + + "github.com/knative/pkg/apis/duck" + duckv1alpha1 "github.com/knative/pkg/apis/duck/v1alpha1" +) + +func TestResourceImplementsGenerational(t *testing.T) { + var emptyGen duckv1alpha1.Generation + if err := duck.VerifyType(&Resource{}, &emptyGen); err != nil { + t.Error(err) + } +} diff --git a/webhook/webhook.go b/webhook/webhook.go index a65289e33..8727dca77 100644 --- a/webhook/webhook.go +++ b/webhook/webhook.go @@ -275,7 +275,9 @@ func (ac *AdmissionController) Run(stop <-chan struct{}) error { for _, crd := range ac.Handlers { cp := crd.DeepCopyObject() var emptyGen duckv1alpha1.Generation - duck.VerifyType(cp, &emptyGen) + if err := duck.VerifyType(cp, &emptyGen); err != nil { + return err + } } select {