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

increase code cov for sources #1343

Merged
merged 11 commits into from
Jun 25, 2021
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions pkg/kn/commands/source/apiserver/apiserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,15 @@ func cleanupAPIServerMockClient() {
apiServerSourceClientFactory = nil
}

func createAPIServerSource(name, resourceKind, resourceVersion, serviceAccount, mode string, ceOverrides map[string]string, sink duckv1.Destination) *v1.ApiServerSource {
resources := []v1.APIVersionKindSelector{{
APIVersion: resourceVersion,
Kind: resourceKind,
}}
func createAPIServerSource(name, serviceAccount, mode string, resourceKind, resourceVersion []string, ceOverrides map[string]string, sink duckv1.Destination) *v1.ApiServerSource {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whenever I see a function with so many parameters (in code or test) I wonder whether a struct should be created to hold the related params and the struct passed to the func or be a method?

resources := make([]v1.APIVersionKindSelector, len(resourceKind))

for i, r := range resourceKind {
resources[i] = v1.APIVersionKindSelector{
APIVersion: resourceVersion[i],
Kind: r,
}
}

return clientv1.NewAPIServerSourceBuilder(name).
Resources(resources).
Expand Down
6 changes: 0 additions & 6 deletions pkg/kn/commands/source/apiserver/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,6 @@ func NewAPIServerCreateCommand(p *commands.KnParams) *cobra.Command {

err = apiSourceClient.CreateAPIServerSource(cmd.Context(), b.Build())

if err != nil {
return fmt.Errorf(
"cannot create ApiServerSource '%s' in namespace '%s' "+
"because: %s", name, namespace, err)
}

if err == nil {
fmt.Fprintf(cmd.OutOrStdout(), "ApiServer source '%s' created in namespace '%s'.\n", args[0], namespace)
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/kn/commands/source/apiserver/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func TestCreateApiServerSource(t *testing.T) {
apiServerClient := v1.NewMockKnAPIServerSourceClient(t)

apiServerRecorder := apiServerClient.Recorder()
apiServerRecorder.CreateAPIServerSource(createAPIServerSource("testsource", "Event", "v1", "testsa", "Reference", map[string]string{"bla": "blub", "foo": "bar"}, createSinkv1("testsvc", "default")), nil)
apiServerRecorder.CreateAPIServerSource(createAPIServerSource("testsource", "testsa", "Reference", []string{"Event"}, []string{"v1"}, map[string]string{"bla": "blub", "foo": "bar"}, createSinkv1("testsvc", "default")), nil)

out, err := executeAPIServerSourceCommand(apiServerClient, dynamicClient, "create", "testsource", "--resource", "Event:v1", "--service-account", "testsa", "--sink", "ksvc:testsvc", "--mode", "Reference", "--ce-override", "bla=blub", "--ce-override", "foo=bar")
assert.NilError(t, err, "ApiServer source should be created")
Expand All @@ -48,6 +48,9 @@ func TestSinkNotFoundError(t *testing.T) {
dynamicClient := dynamicfake.CreateFakeKnDynamicClient("default")
apiServerClient := v1.NewMockKnAPIServerSourceClient(t)
errorMsg := "cannot create ApiServerSource 'testsource' in namespace 'default' because: services.serving.knative.dev \"testsvc\" not found"
argMissingMsg := "requires the name of the source to create as single argument"
_, err := executeAPIServerSourceCommand(apiServerClient, dynamicClient, "create", "--resource", "Event:v1:key1=value1", "--service-account", "testsa", "--sink", "ksvc:testsvc", "--mode", "Reference")
assert.Error(t, err, argMissingMsg)
out, err := executeAPIServerSourceCommand(apiServerClient, dynamicClient, "create", "testsource", "--resource", "Event:v1:key1=value1", "--service-account", "testsa", "--sink", "ksvc:testsvc", "--mode", "Reference")
assert.Error(t, err, errorMsg)
assert.Assert(t, util.ContainsAll(out, errorMsg, "Usage"))
Expand Down
9 changes: 8 additions & 1 deletion pkg/kn/commands/source/apiserver/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ func TestApiServerSourceDelete(t *testing.T) {
}

func TestDeleteWithError(t *testing.T) {

apiServerClient := v1.NewMockKnAPIServerSourceClient(t, "mynamespace")
apiServerRecorder := apiServerClient.Recorder()

Expand All @@ -51,3 +50,11 @@ func TestDeleteWithError(t *testing.T) {

apiServerRecorder.Validate()
}

func TestDeleteWithNoArgError(t *testing.T) {
apiServerClient := v1.NewMockKnAPIServerSourceClient(t, "mynamespace")
argMissingMsg := "requires the name of the source as single argument"

_, err := executeAPIServerSourceCommand(apiServerClient, nil, "delete")
assert.Error(t, err, argMissingMsg)
}
14 changes: 11 additions & 3 deletions pkg/kn/commands/source/apiserver/describe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func TestSimpleDescribe(t *testing.T) {
apiServerClient := v1.NewMockKnAPIServerSourceClient(t, "mynamespace")

apiServerRecorder := apiServerClient.Recorder()
sampleSource := createAPIServerSource("testsource", "Event", "v1", "testsa", "Reference", map[string]string{"foo": "bar"}, createSinkv1("testsvc", "default"))
sampleSource := createAPIServerSource("testsource", "testsa", "Reference", []string{"Event"}, []string{"v1"}, map[string]string{"foo": "bar"}, createSinkv1("testsvc", "default"))
sampleSource.Namespace = "mynamespace"
apiServerRecorder.GetAPIServerSource("testsource", sampleSource, nil)

Expand All @@ -54,7 +54,7 @@ func TestDescribeMachineReadable(t *testing.T) {
apiServerClient := v1.NewMockKnAPIServerSourceClient(t, "mynamespace")

apiServerRecorder := apiServerClient.Recorder()
sampleSource := createAPIServerSource("testsource", "Event", "v1", "testsa", "Reference", map[string]string{"foo": "bar"}, createSinkv1("testsvc", "default"))
sampleSource := createAPIServerSource("testsource", "testsa", "Reference", []string{"Event"}, []string{"v1"}, map[string]string{"foo": "bar"}, createSinkv1("testsvc", "default"))
sampleSource.APIVersion = "sources.knative.dev/v1"
sampleSource.Kind = "ApiServerSource"
sampleSource.Namespace = "mynamespace"
Expand Down Expand Up @@ -84,7 +84,7 @@ func TestDescribeWithSinkURI(t *testing.T) {
apiServerClient := v1.NewMockKnAPIServerSourceClient(t, "mynamespace")

apiServerRecorder := apiServerClient.Recorder()
sampleSource := createAPIServerSource("testsource", "Event", "v1", "testsa", "Reference", map[string]string{"foo": "bar"}, sinkURI)
sampleSource := createAPIServerSource("testsource", "testsa", "Reference", []string{"Event"}, []string{"v1"}, map[string]string{"foo": "bar"}, sinkURI)
sampleSource.Namespace = "mynamespace"
apiServerRecorder.GetAPIServerSource("testsource", sampleSource, nil)

Expand All @@ -94,3 +94,11 @@ func TestDescribeWithSinkURI(t *testing.T) {

apiServerRecorder.Validate()
}

func TestDescribeNoArgError(t *testing.T) {
apiServerClient := v1.NewMockKnAPIServerSourceClient(t, "mynamespace")
argMissingMsg := "'kn source apiserver describe' requires name of the source as single argument"

_, err := executeAPIServerSourceCommand(apiServerClient, nil, "describe")
assert.Error(t, err, argMissingMsg)
}
2 changes: 1 addition & 1 deletion pkg/kn/commands/source/apiserver/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestListAPIServerSource(t *testing.T) {
apiServerClient := v12.NewMockKnAPIServerSourceClient(t)

apiServerRecorder := apiServerClient.Recorder()
sampleSource := createAPIServerSource("testsource", "Event", "v1", "testsa", "Reference", nil, createSinkv1("testsvc", "default"))
sampleSource := createAPIServerSource("testsource", "testsa", "Reference", []string{"Event"}, []string{"v1"}, nil, createSinkv1("testsvc", "default"))
sampleSourceList := v1.ApiServerSourceList{}
sampleSourceList.Items = []v1.ApiServerSource{*sampleSource}

Expand Down
35 changes: 31 additions & 4 deletions pkg/kn/commands/source/apiserver/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package apiserver

import (
"errors"
"testing"
"time"

Expand All @@ -37,13 +38,13 @@ func TestApiServerSourceUpdate(t *testing.T) {

apiServerRecorder := apiServerClient.Recorder()

present := createAPIServerSource("testsource", "Event", "v1", "testsa1", "Reference", map[string]string{"bla": "blub", "foo": "bar"}, createSinkv1("svc1", "default"))
present := createAPIServerSource("testsource", "testsa1", "Reference", []string{"Event"}, []string{"v1"}, map[string]string{"bla": "blub", "foo": "bar"}, createSinkv1("svc1", "default"))
apiServerRecorder.GetAPIServerSource("testsource", present, nil)

updated := createAPIServerSource("testsource", "Event", "v1", "testsa2", "Reference", map[string]string{"foo": "baz"}, createSinkv1("svc2", "default"))
updated := createAPIServerSource("testsource", "testsa2", "Reference", []string{"Event", "Pod"}, []string{"v1", "v1"}, map[string]string{"foo": "baz"}, createSinkv1("svc2", "default"))
apiServerRecorder.UpdateAPIServerSource(updated, nil)

output, err := executeAPIServerSourceCommand(apiServerClient, dynamicClient, "update", "testsource", "--service-account", "testsa2", "--sink", "ksvc:svc2", "--ce-override", "bla-", "--ce-override", "foo=baz")
output, err := executeAPIServerSourceCommand(apiServerClient, dynamicClient, "update", "testsource", "--service-account", "testsa2", "--sink", "ksvc:svc2", "--ce-override", "bla-", "--ce-override", "foo=baz", "--mode", "Reference", "--resource", "Pod:v1")
assert.NilError(t, err)
assert.Assert(t, util.ContainsAll(output, "testsource", "updated", "default"))

Expand All @@ -54,7 +55,7 @@ func TestApiServerSourceUpdateDeletionTimestampNotNil(t *testing.T) {
apiServerClient := v1.NewMockKnAPIServerSourceClient(t)
apiServerRecorder := apiServerClient.Recorder()

present := createAPIServerSource("testsource", "Event", "v1", "testsa1", "Ref", nil, createSinkv1("svc1", "default"))
present := createAPIServerSource("testsource", "testsa1", "Ref", []string{"Event"}, []string{"v1"}, nil, createSinkv1("svc1", "default"))
present.DeletionTimestamp = &metav1.Time{Time: time.Now()}
apiServerRecorder.GetAPIServerSource("testsource", present, nil)

Expand All @@ -63,3 +64,29 @@ func TestApiServerSourceUpdateDeletionTimestampNotNil(t *testing.T) {
assert.ErrorContains(t, err, "deletion")
assert.ErrorContains(t, err, "apiserver")
}

func TestApiServerUpdateErrorForNoArgs(t *testing.T) {
apiServerClient := v1.NewMockKnAPIServerSourceClient(t)
argMissingMsg := "requires the name of the source as single argument"
_, err := executeAPIServerSourceCommand(apiServerClient, nil, "update")
assert.Error(t, err, argMissingMsg)
}

func TestApiServerUpdateSinkMissingError(t *testing.T) {
apiServerClient := v1.NewMockKnAPIServerSourceClient(t)
apiServerRecorder := apiServerClient.Recorder()

//check if sink is missing
dynamicClient := dynamicfake.CreateFakeKnDynamicClient("default", &servingv1.Service{
TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: "serving.knative.dev/v1"},
ObjectMeta: metav1.ObjectMeta{Name: "svc2", Namespace: "default"},
})

present := createAPIServerSource("testsource", "testsa1", "Reference", []string{"Event"}, []string{"v1"}, map[string]string{"bla": "blub", "foo": "bar"}, createSinkv1("svc1", "default"))
apiServerRecorder.GetAPIServerSource("testsource", present, nil)
sinkMissingMsg := "services.serving.knative.dev \"svc3\" not found"
apiServerRecorder.UpdateAPIServerSource("", errors.New(sinkMissingMsg))

_, err := executeAPIServerSourceCommand(apiServerClient, dynamicClient, "update", "testsource", "--service-account", "testsa2", "--sink", "ksvc:svc3", "--ce-override", "bla-", "--ce-override", "foo=baz")
assert.Error(t, err, sinkMissingMsg)
}
20 changes: 18 additions & 2 deletions pkg/kn/commands/source/container/container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package container

import (
"bytes"
"strings"

corev1 "k8s.io/api/core/v1"
"k8s.io/client-go/tools/clientcmd"
Expand Down Expand Up @@ -82,8 +83,8 @@ func cleanupContainerServerMockClient() {
containerSourceClientFactory = nil
}

func createContainerSource(name, image string, sink duckv1.Destination) *v1.ContainerSource {
return clientv1.NewContainerSourceBuilder(name).
func createContainerSource(name, image string, sink duckv1.Destination, ceo map[string]string, envs, args []string) *v1.ContainerSource {
cs := clientv1.NewContainerSourceBuilder(name).
PodSpec(corev1.PodSpec{
Containers: []corev1.Container{{
Image: image,
Expand All @@ -94,6 +95,21 @@ func createContainerSource(name, image string, sink duckv1.Destination) *v1.Cont
}}}).
Sink(sink).
Build()

if args != nil {
cs.Spec.Template.Spec.Containers[0].Args = args
}

if ceo != nil {
cs.Spec.CloudEventOverrides = &duckv1.CloudEventOverrides{Extensions: ceo}
}

for _, env := range envs {
e := strings.Split(env, "=")
cs.Spec.Template.Spec.Containers[0].Env = append(cs.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{Name: e[0], Value: e[1]})
}

return cs
}

func createSinkv1(serviceName, namespace string) duckv1.Destination {
Expand Down
10 changes: 3 additions & 7 deletions pkg/kn/commands/source/container/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,10 @@ func NewContainerCreateCommand(p *commands.KnParams) *cobra.Command {

b := v1.NewContainerSourceBuilder(name).Sink(*objectRef).PodSpec(*podSpec)
err = srcClient.CreateContainerSource(cmd.Context(), b.Build())
if err != nil {
return fmt.Errorf(
"cannot create ContainerSource '%s' in namespace '%s' "+
"because: %s", name, namespace, err)
itsmurugappan marked this conversation as resolved.
Show resolved Hide resolved
if err == nil {
fmt.Fprintf(cmd.OutOrStdout(), "ContainerSource '%s' created in namespace '%s'.\n", args[0], namespace)
}

fmt.Fprintf(cmd.OutOrStdout(), "ContainerSource '%s' created in namespace '%s'.\n", args[0], namespace)
return nil
return err
},
}
commands.AddNamespaceFlags(cmd.Flags(), false)
Expand Down
21 changes: 20 additions & 1 deletion pkg/kn/commands/source/container/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestCreateContainerSource(t *testing.T) {
containerClient := v1.NewMockKnContainerSourceClient(t)

containerRecorder := containerClient.Recorder()
containerRecorder.CreateContainerSource(createContainerSource("testsource", "docker.io/test/testimg", createSinkv1("testsvc", "default")), nil)
containerRecorder.CreateContainerSource(createContainerSource("testsource", "docker.io/test/testimg", createSinkv1("testsvc", "default"), nil, nil, nil), nil)

out, err := executeContainerSourceCommand(containerClient, dynamicClient, "create", "testsource", "--image", "docker.io/test/testimg", "--sink", "ksvc:testsvc")
assert.NilError(t, err, "Container source should be created")
Expand All @@ -60,3 +60,22 @@ func TestNoSinkError(t *testing.T) {
_, err := executeContainerSourceCommand(containerClient, nil, "create", "testsource", "--image", "docker.io/test/testimg")
assert.ErrorContains(t, err, "required flag(s)", "sink", "not set")
}

func TestContainerCreateErrorForNoArgs(t *testing.T) {
containerClient := v1.NewMockKnContainerSourceClient(t, "mynamespace")
argMissingMsg := "requires the name of the source to create as single argument"
_, err := executeContainerSourceCommand(containerClient, nil, "create", "--sink", "ksvc:testsvc", "--image", "docker.io/test/testimg")
assert.Error(t, err, argMissingMsg)
}

func TestContainerCreatePSError(t *testing.T) {
testsvc := &servingv1.Service{
TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: "serving.knative.dev/v1"},
ObjectMeta: metav1.ObjectMeta{Name: "testsvc", Namespace: "default"},
}
dynamicClient := dynamicfake.CreateFakeKnDynamicClient("default", testsvc)
containerClient := v1.NewMockKnContainerSourceClient(t)

_, err := executeContainerSourceCommand(containerClient, dynamicClient, "create", "testsource", "--sink", "ksvc:testsvc", "--image", "docker.io/test/testimg", "--mount", "123456")
assert.Error(t, err, "cannot create ContainerSource 'testsource' in namespace 'default' because: Invalid --mount: argument requires a value that contains the \"=\" character; got \"123456\"")
itsmurugappan marked this conversation as resolved.
Show resolved Hide resolved
}
7 changes: 7 additions & 0 deletions pkg/kn/commands/source/container/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,10 @@ func TestDeleteWithError(t *testing.T) {

containerRecorder.Validate()
}

func TestContainerDeleteErrorForNoArgs(t *testing.T) {
containerClient := v1.NewMockKnContainerSourceClient(t, "mynamespace")
argMissingMsg := "requires the name of the source as single argument"
_, err := executeContainerSourceCommand(containerClient, nil, "delete")
assert.Error(t, err, argMissingMsg)
}
6 changes: 3 additions & 3 deletions pkg/kn/commands/source/container/describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (

// NewContainerDescribeCommand to describe an Container source object
func NewContainerDescribeCommand(p *commands.KnParams) *cobra.Command {
apiServerDescribe := &cobra.Command{
containerDescribe := &cobra.Command{
Use: "describe NAME",
Short: "Show details of a container source",
Example: `
Expand Down Expand Up @@ -90,11 +90,11 @@ func NewContainerDescribeCommand(p *commands.KnParams) *cobra.Command {
return nil
},
}
flags := apiServerDescribe.Flags()
flags := containerDescribe.Flags()
commands.AddNamespaceFlags(flags, false)
flags.BoolP("verbose", "v", false, "More output.")

return apiServerDescribe
return containerDescribe
}

func writeContainerSource(dw printers.PrefixWriter, source *v1.ContainerSource, printDetails bool) {
Expand Down
17 changes: 15 additions & 2 deletions pkg/kn/commands/source/container/describe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,27 @@ func TestSimpleDescribe(t *testing.T) {
containerClient := v1.NewMockKnContainerSourceClient(t, "mynamespace")

containerRecorder := containerClient.Recorder()
sampleSource := createContainerSource("testsource", "docker.io/test/testimg", createSinkv1("testsvc", "default"))
sampleSource := createContainerSource(
"testsource", "docker.io/test/testimg",
createSinkv1("testsvc", "default"),
map[string]string{"bla": "blub", "foo": "bar"},
[]string{"env1=evalue1"},
[]string{"baz"},
)
sampleSource.Namespace = "mynamespace"
containerRecorder.GetContainerSource("testsource", sampleSource, nil)

out, err := executeContainerSourceCommand(containerClient, nil, "describe", "testsource")
assert.NilError(t, err)
assert.Assert(t, util.ContainsAll(out, "testsource", "docker.io/test/testimg", "testsvc"))
assert.Assert(t, util.ContainsAll(out, "testsource", "docker.io/test/testimg", "testsvc", "bla", "foo", "env1", "baz"))
assert.Assert(t, util.ContainsNone(out, "URI"))

containerRecorder.Validate()
}

func TestContainerDescribeErrorForNoArgs(t *testing.T) {
containerClient := v1.NewMockKnContainerSourceClient(t, "mynamespace")
argMissingMsg := "'kn source container describe' requires name of the source as single argument"
_, err := executeContainerSourceCommand(containerClient, nil, "describe")
assert.Error(t, err, argMissingMsg)
}
2 changes: 1 addition & 1 deletion pkg/kn/commands/source/container/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestListContainerSource(t *testing.T) {
containerClient := v12.NewMockKnContainerSourceClient(t)

containerRecorder := containerClient.Recorder()
sampleSource := createContainerSource("testsource", "docker.io/test/newimg", createSinkv1("svc2", "default"))
sampleSource := createContainerSource("testsource", "docker.io/test/newimg", createSinkv1("svc2", "default"), nil, nil, nil)
sampleSourceList := v1.ContainerSourceList{}
sampleSourceList.Items = []v1.ContainerSource{*sampleSource}

Expand Down
8 changes: 2 additions & 6 deletions pkg/kn/commands/source/container/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,9 @@ func NewContainerUpdateCommand(p *commands.KnParams) *cobra.Command {
}

err = srcClient.UpdateContainerSource(cmd.Context(), b.Build())
if err != nil {
return fmt.Errorf(
"cannot update ContainerSource '%s' in namespace '%s' "+
"because: %s", name, namespace, err)
if err == nil {
fmt.Fprintf(cmd.OutOrStdout(), "Container source '%s' updated in namespace '%s'.\n", args[0], namespace)
}

fmt.Fprintf(cmd.OutOrStdout(), "Container source '%s' updated in namespace '%s'.\n", args[0], namespace)
return err
},
}
Expand Down
Loading