Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move the use of VerifyType in consumer packages to unit tests #97

Closed
dprotaso opened this issue Sep 26, 2018 · 6 comments
Closed

Move the use of VerifyType in consumer packages to unit tests #97

dprotaso opened this issue Sep 26, 2018 · 6 comments
Labels
area/API kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.

Comments

@dprotaso
Copy link
Member

dprotaso commented Sep 26, 2018

We're using duck.VerifyType to enforce that serialization formats match. These statements are being placed in packages as follows:

var _ = duck.VerifyType(&Target{}, &Targetable{})

This technique means verify is invoked at runtime during package initialization.

As we add more potential types that need to be verified this package initialization technique has an affect on a program's startup.

To avoid this startup time cost, we can simply move these assertions to unit tests that invoke VerifyType

func TestTargetImplementsTargetable(t *testing.T) {
  duck.VerifyType(&Target{}, &Targetable{}) 
}
@knative-prow-robot knative-prow-robot added area/API kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. labels Sep 26, 2018
@dprotaso
Copy link
Member Author

Similarly the use of

var _ duck.Implementable = (*Targetable)(nil)

is not an issue as it's a compile time check

@dprotaso
Copy link
Member Author

/kind good-first-issue

@dprotaso
Copy link
Member Author

/assign @vdemeester

@knative-prow-robot
Copy link
Contributor

@dprotaso: GitHub didn't allow me to assign the following users: vdemeester.

Note that only knative members and repo collaborators can be assigned.
For more information please see the contributor guide

In response to this:

/assign @vdemeester

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@dprotaso
Copy link
Member Author

@mattmoor
Copy link
Member

This is done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/API kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.
Projects
None yet
Development

No branches or pull requests

3 participants