refactor(service): Removed io.writer from of KnClient.WaitForService() (#248)

This puts all the console output to the command, where it belongs too.
One could add a ProgressHandler for more fine granular feedback (like suggested in #234) but for now this is not needed.

docs/cmd/kn.md

@@ -6,8 +6,9 @@ Knative client
 
 Manage your Knative building blocks:
 
-* [Serving](https://github.com/knative/serving/tree/master): Manage your services and release new software to them.
-* [Eventing](https://github.com/knative/eventing/tree/master): Manage event subscriptions and channels. Connect event sources.
+Serving: Manage your services and release new software to them.
+Build: Create builds and keep track of their results.
+Eventing: Manage event subscriptions and channels. Connect up event sources.
 
 ### Options
 

pkg/kn/commands/service/service_create.go

@@ -104,10 +104,16 @@ func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command {
 			}
 
 			if !waitFlags.Async {
-				err := client.WaitForService(name, time.Duration(waitFlags.TimeoutInSeconds)*time.Second, cmd.OutOrStdout())
+				out := cmd.OutOrStdout()
+				fmt.Fprintf(out, "Waiting for service '%s' to become ready ... ", name)
+				flush(out)
+
+				err := client.WaitForService(name, time.Duration(waitFlags.TimeoutInSeconds)*time.Second)
 				if err != nil {
+					fmt.Fprintln(out)
 					return err
 				}
+				fmt.Fprintln(out, "OK")
 				return showUrl(client, name, namespace, cmd.OutOrStdout())
 			}
 
@@ -120,6 +126,17 @@ func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command {
 	return serviceCreateCommand
 }
 
+// Duck type for writers having a flush
+type flusher interface {
+	Flush() error
+}
+
+func flush(out io.Writer) {
+	if flusher, ok := out.(flusher); ok {
+		flusher.Flush()
+	}
+}
+
 func createService(client v1alpha1.KnClient, service *serving_v1alpha1_api.Service, namespace string, out io.Writer) error {
 	err := client.CreateService(service)
 	if err != nil {

pkg/serving/v1alpha1/client.go

@@ -16,7 +16,6 @@ package v1alpha1
 
 import (
 	"fmt"
-	"io"
 	"time"
 
 	"github.com/knative/pkg/apis"
@@ -53,7 +52,7 @@ type KnClient interface {
 	DeleteService(name string) error
 
 	// Wait for a service to become ready, but not longer than provided timeout
-	WaitForService(name string, timeout time.Duration, out io.Writer) error
+	WaitForService(name string, timeout time.Duration) error
 
 	// Get a revision by name
 	GetRevision(name string) (*v1alpha1.Revision, error)
@@ -188,9 +187,9 @@ func (cl *knClient) DeleteService(serviceName string) error {
 }
 
 // Wait for a service to become ready, but not longer than provided timeout
-func (cl *knClient) WaitForService(name string, timeout time.Duration, out io.Writer) error {
+func (cl *knClient) WaitForService(name string, timeout time.Duration) error {
 	waitForReady := newServiceWaitForReady(cl.client.Services(cl.namespace).Watch)
-	return waitForReady.Wait(name, timeout, out)
+	return waitForReady.Wait(name, timeout)
 }
 
 // Get a revision by name

pkg/serving/v1alpha1/client_test.go

@@ -15,7 +15,6 @@
 package v1alpha1
 
 import (
-	"bytes"
 	"fmt"
 	"testing"
 	"time"
@@ -357,8 +356,7 @@ func TestWaitForService(t *testing.T) {
 		})
 
 	t.Run("wait on a service to become ready with success", func(t *testing.T) {
-		buf := new(bytes.Buffer)
-		err := client.WaitForService(serviceName, 60*time.Second, buf)
+		err := client.WaitForService(serviceName, 60*time.Second)
 		assert.NilError(t, err)
 	})
 }

pkg/wait/wait_for_ready.go

@@ -16,7 +16,6 @@ package wait
 
 import (
 	"fmt"
-	"io"
 	"time"
 
 	"github.com/knative/pkg/apis"
@@ -40,7 +39,7 @@ type WaitForReady interface {
 
 	// Wait on resource the resource with this name until a given timeout
 	// and write status out on writer
-	Wait(name string, timeout time.Duration, out io.Writer) error
+	Wait(name string, timeout time.Duration) error
 }
 
 // Create watch which is used when waiting for Ready condition
@@ -62,21 +61,17 @@ func NewWaitForReady(kind string, watchFunc WatchFunc, extractor ConditionsExtra
 // `watchFunc` creates the actual watch, `kind` is the type what your are watching for
 // (e.g. "service"), `timeout` is a timeout after which the watch should be cancelled if no
 // target state has been entered yet and `out` is used for printing out status messages
-func (w *waitForReadyConfig) Wait(name string, timeout time.Duration, out io.Writer) error {
+func (w *waitForReadyConfig) Wait(name string, timeout time.Duration) error {
 	opts := v1.ListOptions{
 		FieldSelector: fields.OneTermEqualSelector("metadata.name", name).String(),
 	}
 	addWatchTimeout(&opts, timeout)
 
-	fmt.Fprintf(out, "Waiting for %s '%s' to become ready ... ", w.kind, name)
-	flush(out)
-
 	floatingTimeout := timeout
 	for {
 		start := time.Now()
 		retry, timeoutReached, err := w.waitForReadyCondition(opts, name, floatingTimeout)
 		if err != nil {
-			fmt.Fprintln(out)
 			return err
 		}
 		floatingTimeout = floatingTimeout - time.Since(start)
@@ -88,8 +83,6 @@ func (w *waitForReadyConfig) Wait(name string, timeout time.Duration, out io.Wri
 			// restart loop
 			continue
 		}
-
-		fmt.Fprintln(out, "OK")
 		return nil
 	}
 }
@@ -105,17 +98,6 @@ func addWatchTimeout(opts *v1.ListOptions, timeout time.Duration) {
 	opts.TimeoutSeconds = &timeOutWatchSeconds
 }
 
-// Duck type for writers having a flush
-type flusher interface {
-	Flush() error
-}
-
-func flush(out io.Writer) {
-	if flusher, ok := out.(flusher); ok {
-		flusher.Flush()
-	}
-}
-
 func (w *waitForReadyConfig) waitForReadyCondition(opts v1.ListOptions, name string, timeout time.Duration) (bool, bool, error) {
 
 	watcher, err := w.watchFunc(opts)

pkg/wait/wait_for_ready_test.go

@@ -15,8 +15,6 @@
 package wait
 
 import (
-	"bytes"
-	"strings"
 	"testing"
 	"time"
 
@@ -29,17 +27,15 @@ import (
 )
 
 type waitForReadyTestCase struct {
-	events         []watch.Event
-	timeout        time.Duration
-	errorExpected  bool
-	messageContent []string
+	events        []watch.Event
+	timeout       time.Duration
+	errorExpected bool
 }
 
 func TestAddWaitForReady(t *testing.T) {
 
 	for i, tc := range prepareTestCases("test-service") {
 		fakeWatchApi := NewFakeWatch(tc.events)
-		outBuffer := new(bytes.Buffer)
 
 		waitForReady := NewWaitForReady(
 			"blub",
@@ -50,7 +46,7 @@ func TestAddWaitForReady(t *testing.T) {
 				return apis.Conditions(obj.(*v1alpha1.Service).Status.Conditions), nil
 			})
 		fakeWatchApi.Start()
-		err := waitForReady.Wait("foobar", tc.timeout, outBuffer)
+		err := waitForReady.Wait("foobar", tc.timeout)
 		close(fakeWatchApi.eventChan)
 
 		if !tc.errorExpected && err != nil {
@@ -60,16 +56,6 @@ func TestAddWaitForReady(t *testing.T) {
 		if tc.errorExpected && err == nil {
 			t.Errorf("%d: No error but expected one", i)
 		}
-		txtToCheck := outBuffer.String()
-		if err != nil {
-			txtToCheck = err.Error()
-		}
-
-		for _, msg := range tc.messageContent {
-			if !strings.Contains(txtToCheck, msg) {
-				t.Errorf("%d: '%s' does not contain expected part %s", i, txtToCheck, msg)
-			}
-		}
 
 		if fakeWatchApi.StopCalled != 1 {
 			t.Errorf("%d: Exactly one 'stop' should be called, but got %d", i, fakeWatchApi.StopCalled)
@@ -81,10 +67,10 @@ func TestAddWaitForReady(t *testing.T) {
 // Test cases which consists of a series of events to send and the expected behaviour.
 func prepareTestCases(name string) []waitForReadyTestCase {
 	return []waitForReadyTestCase{
-		{peNormal(name), time.Second, false, []string{"OK", "foobar", "blub"}},
-		{peError(name), time.Second, true, []string{"FakeError"}},
-		{peTimeout(name), time.Second, true, []string{"timeout"}},
-		{peWrongGeneration(name), time.Second, true, []string{"timeout"}},
+		{peNormal(name), time.Second, false},
+		{peError(name), time.Second, true},
+		{peTimeout(name), time.Second, true},
+		{peWrongGeneration(name), time.Second, true},
 	}
 }