Repository: knative/client (go)

Issue:
Misleading error message with multiple names
### Bug report

```
$ kn service update echo echo1 --image duglin/echo
requires the service name
```

### Expected behavior

I would have expected either something like:
```
$ kn service create echo echo1 --image duglin/echo
'service create' requires the service name given as single argument
```
or for it to do the update on both services (I kind of like this option).

### Steps to reproduce the problem

See above

### kn version

master/head as of 2020/03/26


Pull request:
fix(issue #762): correct error message when updating service
## Description

Correct error message when calling `service update` with more than one arg

## Changes

* fix issue with TestServiceUpdateWithMultipleImages running create vs update
* correct message when updating service and passing many names
* added TestServiceDescribeWithMultipleNames and fix to describe command
* added TestServiceCreateWithMultipleNames
* fix error message for service delete since many names can be passed

## Reference

Fixes #762

/lint

Patch:
diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
index 298a26f3..08f7a34e 100644
--- a/CHANGELOG.adoc
+++ b/CHANGELOG.adoc
@@ -26,6 +26,10 @@
 | Fix plugin lookup with file ext on Windows
 | https://github.com/knative/client/pull/774[#774]
 
+| 🐛
+| Correct error message when updating service
+| https://github.com/knative/client/pull/778[#778]
+
 |===
 
 ## v0.13.1 (2020-03-25)
diff --git a/pkg/eventing/v1alpha1/client.go b/pkg/eventing/v1alpha1/client.go
index 16037fe7..d77f2d52 100644
--- a/pkg/eventing/v1alpha1/client.go
+++ b/pkg/eventing/v1alpha1/client.go
@@ -62,7 +62,7 @@ func NewKnEventingClient(client client_v1alpha1.EventingV1alpha1Interface, names
 
 //CreateTrigger is used to create an instance of trigger
 func (c *knEventingClient) CreateTrigger(trigger *v1alpha1.Trigger) error {
-	trigger, err := c.client.Triggers(c.namespace).Create(trigger)
+	_, err := c.client.Triggers(c.namespace).Create(trigger)
 	if err != nil {
 		return kn_errors.GetError(err)
 	}
@@ -112,7 +112,7 @@ func (c *knEventingClient) ListTriggers() (*v1alpha1.TriggerList, error) {
 
 //CreateTrigger is used to create an instance of trigger
 func (c *knEventingClient) UpdateTrigger(trigger *v1alpha1.Trigger) error {
-	trigger, err := c.client.Triggers(c.namespace).Update(trigger)
+	_, err := c.client.Triggers(c.namespace).Update(trigger)
 	if err != nil {
 		return kn_errors.GetError(err)
 	}
diff --git a/pkg/kn/commands/describe.go b/pkg/kn/commands/describe.go
index 476cf724..28aee7ac 100644
--- a/pkg/kn/commands/describe.go
+++ b/pkg/kn/commands/describe.go
@@ -158,7 +158,6 @@ func sortConditions(conditions []apis.Condition) []apis.Condition {
 		default:
 			return false
 		}
-		return false
 	})
 	return ret
 }
diff --git a/pkg/kn/commands/service/delete.go b/pkg/kn/commands/service/delete.go
index 81dc8f96..413c7aea 100644
--- a/pkg/kn/commands/service/delete.go
+++ b/pkg/kn/commands/service/delete.go
@@ -40,7 +40,7 @@ func NewServiceDeleteCommand(p *commands.KnParams) *cobra.Command {
 
 		RunE: func(cmd *cobra.Command, args []string) error {
 			if len(args) < 1 {
-				return errors.New("requires the service name")
+				return errors.New("'service delete' requires the service name(s)")
 			}
 
 			namespace, err := p.GetNamespace(cmd)
diff --git a/pkg/kn/commands/service/describe.go b/pkg/kn/commands/service/describe.go
index 866c0827..d3041248 100644
--- a/pkg/kn/commands/service/describe.go
+++ b/pkg/kn/commands/service/describe.go
@@ -81,7 +81,7 @@ func NewServiceDescribeCommand(p *commands.KnParams) *cobra.Command {
 		Short: "Show details of a service",
 		RunE: func(cmd *cobra.Command, args []string) error {
 			if len(args) != 1 {
-				return errors.New("'kn service describe' requires name of the service as single argument")
+				return errors.New("'service describe' requires the service name given as single argument")
 			}
 			serviceName := args[0]
 
diff --git a/pkg/kn/commands/service/update.go b/pkg/kn/commands/service/update.go
index b0589e0a..fe82ce23 100644
--- a/pkg/kn/commands/service/update.go
+++ b/pkg/kn/commands/service/update.go
@@ -60,7 +60,7 @@ func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command {
 		Example: updateExample,
 		RunE: func(cmd *cobra.Command, args []string) (err error) {
 			if len(args) != 1 {
-				return errors.New("requires the service name")
+				return errors.New("'service update' requires the service name given as single argument")
 			}
 
 			namespace, err := p.GetNamespace(cmd)
diff --git a/pkg/sources/v1alpha2/binding_client.go b/pkg/sources/v1alpha2/binding_client.go
index 406c0cad..d004246f 100644
--- a/pkg/sources/v1alpha2/binding_client.go
+++ b/pkg/sources/v1alpha2/binding_client.go
@@ -66,7 +66,7 @@ func newKnSinkBindingClient(client clientv1alpha2.SinkBindingInterface, namespac
 
 //CreateSinkBinding is used to create an instance of binding
 func (c *knBindingClient) CreateSinkBinding(binding *v1alpha2.SinkBinding) error {
-	binding, err := c.client.Create(binding)
+	_, err := c.client.Create(binding)
 	if err != nil {
 		return knerrors.GetError(err)
 	}
@@ -116,7 +116,7 @@ func (c *knBindingClient) ListSinkBindings() (*v1alpha2.SinkBindingList, error)
 
 //CreateSinkBinding is used to create an instance of binding
 func (c *knBindingClient) UpdateSinkBinding(binding *v1alpha2.SinkBinding) error {
-	binding, err := c.client.Update(binding)
+	_, err := c.client.Update(binding)
 	if err != nil {
 		return knerrors.GetError(err)
 	}


Test patch:
diff --git a/pkg/kn/commands/service/create_test.go b/pkg/kn/commands/service/create_test.go
index 0ade9c32..548018f1 100644
--- a/pkg/kn/commands/service/create_test.go
+++ b/pkg/kn/commands/service/create_test.go
@@ -143,6 +143,13 @@ func TestServiceCreateWithMultipleImages(t *testing.T) {
 	assert.Assert(t, util.ContainsAll(err.Error(), "\"--image\"", "\"gcr.io/bar/foo:baz\"", "flag", "once"))
 }
 
+func TestServiceCreateWithMultipleNames(t *testing.T) {
+	_, _, _, err := fakeServiceCreate([]string{
+		"service", "create", "foo", "foo1", "--image", "gcr.io/foo/bar:baz", "--no-wait"}, false)
+
+	assert.Assert(t, util.ContainsAll(err.Error(), "'service create' requires the service name given as single argument"))
+}
+
 func TestServiceCreateImageSync(t *testing.T) {
 	action, created, output, err := fakeServiceCreate([]string{
 		"service", "create", "foo", "--image", "gcr.io/foo/bar:baz"}, false)
diff --git a/pkg/kn/commands/service/describe_test.go b/pkg/kn/commands/service/describe_test.go
index 1c45b2e0..0c2bceb8 100644
--- a/pkg/kn/commands/service/describe_test.go
+++ b/pkg/kn/commands/service/describe_test.go
@@ -75,6 +75,16 @@ func TestServiceDescribeBasic(t *testing.T) {
 	r.Validate()
 }
 
+func TestServiceDescribeWithMultipleNames(t *testing.T) {
+	client := knclient.NewMockKnServiceClient(t)
+	r := client.Recorder()
+	createTestService("foo", []string{"rev1"}, goodConditions())
+	_, err := executeServiceCommand(client, "describe", "foo", "foo1")
+
+	assert.Assert(t, util.ContainsAll(err.Error(), "'service describe' requires the service name given as single argument"))
+	r.Validate()
+}
+
 func TestServiceDescribeSad(t *testing.T) {
 	client := knclient.NewMockKnServiceClient(t)
 	r := client.Recorder()
diff --git a/pkg/kn/commands/service/update_test.go b/pkg/kn/commands/service/update_test.go
index bb48aad8..e197684c 100644
--- a/pkg/kn/commands/service/update_test.go
+++ b/pkg/kn/commands/service/update_test.go
@@ -195,11 +195,19 @@ func TestServiceUpdateImage(t *testing.T) {
 func TestServiceUpdateWithMultipleImages(t *testing.T) {
 	orig := newEmptyService()
 	_, _, _, err := fakeServiceUpdate(orig, []string{
-		"service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--image", "gcr.io/bar/foo:baz", "--no-wait"})
+		"service", "update", "foo", "--image", "gcr.io/foo/bar:baz", "--image", "gcr.io/bar/foo:baz", "--no-wait"})
 
 	assert.Assert(t, util.ContainsAll(err.Error(), "\"--image\"", "\"gcr.io/bar/foo:baz\"", "flag", "once"))
 }
 
+func TestServiceUpdateWithMultipleNames(t *testing.T) {
+	orig := newEmptyService()
+	_, _, _, err := fakeServiceUpdate(orig, []string{
+		"service", "update", "foo", "foo1", "--image", "gcr.io/foo/bar:baz", "--no-wait"})
+
+	assert.Assert(t, util.ContainsAll(err.Error(), "'service update' requires the service name given as single argument"))
+}
+
 func TestServiceUpdateCommand(t *testing.T) {
 	orig := newEmptyService()