Refactor to use new SkaffoldWriter type

cmd/skaffold/app/cmd/cmd.go

@@ -79,7 +79,7 @@ func NewSkaffoldCommand(out, errOut io.Writer) *cobra.Command {
 
 			opts.Command = cmd.Use
 			instrumentation.SetCommand(cmd.Use)
-			out := output.SetupColors(out, defaultColor, forceColors)
+			out := output.SetupOutput(out, defaultColor, forceColors)
 			if timestamps {
 				l := logrus.New()
 				l.SetOutput(out)

cmd/skaffold/skaffold.go

@@ -41,7 +41,7 @@ func main() {
 			// As we allow some color setup using CLI flags for the main run, we can't run SetupColors()
 			// for the entire skaffold run here. It's possible SetupColors() was never called, so call it again
 			// before we print an error to get the right coloring.
-			errOut := output.SetupColors(os.Stderr, output.DefaultColorCode, false)
+			errOut := output.SetupOutput(os.Stderr, output.DefaultColorCode, false)
 			output.Red.Fprintln(errOut, err)
 			code = exitCode(err)
 		}

pkg/skaffold/build/result.go

@@ -63,7 +63,7 @@ func (l *logAggregatorImpl) GetWriter() (io.Writer, func(), error) {
 
 	writer := io.Writer(w)
 	if output.IsColorable(l.out) {
-		writer = output.NewWriter(writer)
+		writer = output.NewColorWriter(writer)
 	}
 	ch := make(chan string, buffSize)
 	l.messages <- ch

pkg/skaffold/output/color.go

@@ -19,7 +19,6 @@ package output
 import (
 	"fmt"
 	"io"
-	"os"
 	"strings"
 
 	colors "github.com/heroku/color"
@@ -71,7 +70,7 @@ func SetupColors(out io.Writer, defaultColor int, forceColors bool) io.Writer {
 	}[defaultColor]
 
 	if useColors {
-		return NewWriter(out)
+		return NewColorWriter(out)
 	}
 	return out
 }
@@ -137,32 +136,17 @@ func (c Color) Fprintf(out io.Writer, format string, a ...interface{}) {
 	fmt.Fprint(out, c.color.Sprintf(format, a...))
 }
 
-func NewWriter(out io.Writer) io.Writer {
+func NewColorWriter(out io.Writer) io.Writer {
 	return colorableWriter{out}
 }
 
 func IsColorable(out io.Writer) bool {
-	switch out.(type) {
+	switch w := out.(type) {
 	case colorableWriter:
 		return true
+	case SkaffoldWriter:
+		return IsColorable(w.MainWriter)
 	default:
 		return false
 	}
 }
-
-func IsStdout(out io.Writer) bool {
-	o, ok := out.(colorableWriter)
-	if ok {
-		return o.Writer == os.Stdout
-	}
-	return out == os.Stdout
-}
-
-// GetWriter returns the underlying writer if out is a colorableWriter
-func GetWriter(out io.Writer) io.Writer {
-	o, ok := out.(colorableWriter)
-	if ok {
-		return o.Writer
-	}
-	return out
-}

pkg/skaffold/output/color_test.go

@@ -18,11 +18,7 @@ package output
 
 import (
 	"bytes"
-	"io"
-	"os"
 	"testing"
-
-	"github.com/GoogleContainerTools/skaffold/testutil"
 )
 
 func compareText(t *testing.T, expected, actual string) {
@@ -93,59 +89,3 @@ func TestFprintlnChangeDefaultToUnknown(t *testing.T) {
 	Default.Fprintln(cw, "2", "less", "chars!")
 	compareText(t, "2 less chars!\n", b.String())
 }
-
-func TestIsStdOut(t *testing.T) {
-	tests := []struct {
-		description string
-		out         io.Writer
-		expected    bool
-	}{
-		{
-			description: "std out passed",
-			out:         os.Stdout,
-			expected:    true,
-		},
-		{
-			description: "out nil",
-			out:         nil,
-		},
-		{
-			description: "out bytes buffer",
-			out:         new(bytes.Buffer),
-		},
-		{
-			description: "colorable std out passed",
-			out:         NewWriter(os.Stdout),
-			expected:    true,
-		},
-	}
-	for _, test := range tests {
-		testutil.Run(t, test.description, func(t *testutil.T) {
-			t.CheckDeepEqual(test.expected, IsStdout(test.out))
-		})
-	}
-}
-
-func TestGetWriter(t *testing.T) {
-	tests := []struct {
-		description string
-		out         io.Writer
-		expected    io.Writer
-	}{
-		{
-			description: "colorable os.Stdout returns os.Stdout",
-			out:         colorableWriter{os.Stdout},
-			expected:    os.Stdout,
-		},
-		{
-			description: "GetWriter returns original writer if not colorable",
-			out:         os.Stdout,
-			expected:    os.Stdout,
-		},
-	}
-	for _, test := range tests {
-		testutil.Run(t, test.description, func(t *testutil.T) {
-			t.CheckDeepEqual(true, test.expected == GetWriter(test.out))
-		})
-	}
-}

pkg/skaffold/output/output.go

@@ -0,0 +1,75 @@
+/*
+Copyright 2021 The Skaffold 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 output
+
+import (
+	"io"
+	"io/ioutil"
+	"os"
+)
+
+type SkaffoldWriter struct {
+	MainWriter  io.Writer
+	EventWriter io.Writer
+}
+
+func (s SkaffoldWriter) Write(p []byte) (int, error) {
+	n, err := s.MainWriter.Write(p)
+	if err != nil {
+		return n, err
+	}
+	if n != len(p) {
+		return n, io.ErrShortWrite
+	}
+
+	s.EventWriter.Write(p)
+
+	return len(p), nil
+}
+
+func SetupOutput(out io.Writer, defaultColor int, forceColors bool) io.Writer {
+	return SkaffoldWriter{
+		MainWriter: SetupColors(out, defaultColor, forceColors),
+		// TODO(marlongamez): Replace this once event writer is implemented
+		EventWriter: ioutil.Discard,
+	}
+}
+
+func IsStdout(out io.Writer) bool {
+	sw, isSW := out.(SkaffoldWriter)
+	if isSW {
+		out = sw.MainWriter
+	}
+	cw, isCW := out.(colorableWriter)
+	if isCW {
+		out = cw.Writer
+	}
+	return out == os.Stdout
+}
+
+// GetWriter returns the underlying writer if out is a colorableWriter
+func GetWriter(out io.Writer) io.Writer {
+	sw, isSW := out.(SkaffoldWriter)
+	if isSW {
+		out = sw.MainWriter
+	}
+	cw, isCW := out.(colorableWriter)
+	if isCW {
+		out = cw.Writer
+	}
+	return out
+}

pkg/skaffold/output/output_test.go

@@ -0,0 +1,118 @@
+/*
+Copyright 2021 The Skaffold 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 output
+
+import (
+	"bytes"
+	"io"
+	"io/ioutil"
+	"os"
+	"testing"
+
+	"github.com/GoogleContainerTools/skaffold/testutil"
+)
+
+func TestIsStdOut(t *testing.T) {
+	tests := []struct {
+		description string
+		out         io.Writer
+		expected    bool
+	}{
+		{
+			description: "std out passed",
+			out:         os.Stdout,
+			expected:    true,
+		},
+		{
+			description: "out nil",
+			out:         nil,
+		},
+		{
+			description: "out bytes buffer",
+			out:         new(bytes.Buffer),
+		},
+		{
+			description: "colorable std out passed",
+			out: SkaffoldWriter{
+				MainWriter: NewColorWriter(os.Stdout),
+			},
+			expected: true,
+		},
+		{
+			description: "colorableWriter passed",
+			out:         NewColorWriter(os.Stdout),
+			expected:    true,
+		},
+		{
+			description: "invalid colorableWriter passed",
+			out: SkaffoldWriter{
+				MainWriter: NewColorWriter(ioutil.Discard),
+			},
+			expected: false,
+		},
+	}
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			t.CheckDeepEqual(test.expected, IsStdout(test.out))
+		})
+	}
+}
+
+func TestGetWriter(t *testing.T) {
+	tests := []struct {
+		description string
+		out         io.Writer
+		expected    io.Writer
+	}{
+		{
+			description: "colorable os.Stdout returns os.Stdout",
+			out: SkaffoldWriter{
+				MainWriter: colorableWriter{os.Stdout},
+			},
+			expected: os.Stdout,
+		},
+		{
+			description: "skaffold writer returns os.Stdout without colorableWriter",
+			out: SkaffoldWriter{
+				MainWriter: os.Stdout,
+			},
+			expected: os.Stdout,
+		},
+		{
+			description: "return ioutil.Discard from SkaffoldWriter",
+			out: SkaffoldWriter{
+				MainWriter: NewColorWriter(ioutil.Discard),
+			},
+			expected: ioutil.Discard,
+		},
+		{
+			description: "os.Stdout returned from colorableWriter",
+			out:         NewColorWriter(os.Stdout),
+			expected:    os.Stdout,
+		},
+		{
+			description: "GetWriter returns original writer if not colorable",
+			out:         os.Stdout,
+			expected:    os.Stdout,
+		},
+	}
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			t.CheckDeepEqual(true, test.expected == GetWriter(test.out))
+		})
+	}
+}