Fix bug with STDOUT and STDERR in the "group" output mode

Took the oportunity to refactor a bit how we handle closing of the streams.

Fixes #779

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+- Fixed bug when using the `output: group` mode where STDOUT and STDERR were
+  being print in separated blocks instead of in the right order
+  ([#779](https://github.com/go-task/task/issues/779)).
 - Starting on this release, ARM architecture binaries are been released to Snap
   as well
   ([#795](https://github.com/go-task/task/issues/795)).

internal/output/group.go

@@ -5,19 +5,19 @@ import (
 	"io"
 )
 
-type Group struct{
+type Group struct {
 	Begin, End string
 }
 
-func (g Group) WrapWriter(w io.Writer, _ string, tmpl Templater) io.Writer {
+func (g Group) WrapWriter(stdOut, _ io.Writer, _ string, tmpl Templater) (io.Writer, io.Writer, CloseFunc) {
-	gw := &groupWriter{writer: w}
+	gw := &groupWriter{writer: stdOut}
 	if g.Begin != "" {
 		gw.begin = tmpl.Replace(g.Begin) + "\n"
 	}
 	if g.End != "" {
 		gw.end = tmpl.Replace(g.End) + "\n"
 	}
-	return gw
+	return gw, gw, func() error { return gw.close() }
 }
 
 type groupWriter struct {
@@ -30,7 +30,7 @@ func (gw *groupWriter) Write(p []byte) (int, error) {
 	return gw.buff.Write(p)
 }
 
-func (gw *groupWriter) Close() error {
+func (gw *groupWriter) close() error {
 	if gw.buff.Len() == 0 {
 		// don't print begin/end messages if there's no buffered entries
 		return nil

internal/output/interleaved.go

@@ -6,6 +6,6 @@ import (
 
 type Interleaved struct{}
 
-func (Interleaved) WrapWriter(w io.Writer, _ string, _ Templater) io.Writer {
+func (Interleaved) WrapWriter(stdOut, stdErr io.Writer, _ string, _ Templater) (io.Writer, io.Writer, CloseFunc) {
-	return w
+	return stdOut, stdErr, func() error { return nil }
 }

internal/output/output.go

@@ -15,9 +15,11 @@ type Templater interface {
 }
 
 type Output interface {
-	WrapWriter(w io.Writer, prefix string, tmpl Templater) io.Writer
+	WrapWriter(stdOut, stdErr io.Writer, prefix string, tmpl Templater) (io.Writer, io.Writer, CloseFunc)
 }
 
+type CloseFunc func() error
+
 // Build the Output for the requested taskfile.Output.
 func BuildFor(o *taskfile.Output) (Output, error) {
 	switch o.Name {

internal/output/output_test.go

@@ -16,7 +16,7 @@ import (
 func TestInterleaved(t *testing.T) {
 	var b bytes.Buffer
 	var o output.Output = output.Interleaved{}
-	var w = o.WrapWriter(&b, "", nil)
+	var w, _, _ = o.WrapWriter(&b, io.Discard, "", nil)
 
 	fmt.Fprintln(w, "foo\nbar")
 	assert.Equal(t, "foo\nbar\n", b.String())
@@ -27,14 +27,19 @@ func TestInterleaved(t *testing.T) {
 func TestGroup(t *testing.T) {
 	var b bytes.Buffer
 	var o output.Output = output.Group{}
-	var w = o.WrapWriter(&b, "", nil).(io.WriteCloser)
+	var stdOut, stdErr, cleanup = o.WrapWriter(&b, io.Discard, "", nil)
 
-	fmt.Fprintln(w, "foo\nbar")
+	fmt.Fprintln(stdOut, "out\nout")
 	assert.Equal(t, "", b.String())
-	fmt.Fprintln(w, "baz")
+	fmt.Fprintln(stdErr, "err\nerr")
 	assert.Equal(t, "", b.String())
-	assert.NoError(t, w.Close())
+	fmt.Fprintln(stdOut, "out")
-	assert.Equal(t, "foo\nbar\nbaz\n", b.String())
+	assert.Equal(t, "", b.String())
+	fmt.Fprintln(stdErr, "err")
+	assert.Equal(t, "", b.String())
+
+	assert.NoError(t, cleanup())
+	assert.Equal(t, "out\nout\nerr\nerr\nout\nerr\n", b.String())
 }
 
 func TestGroupWithBeginEnd(t *testing.T) {
@@ -53,19 +58,19 @@ func TestGroupWithBeginEnd(t *testing.T) {
 	}
 	t.Run("simple", func(t *testing.T) {
 		var b bytes.Buffer
-		var w = o.WrapWriter(&b, "", &tmpl).(io.WriteCloser)
+		var w, _, cleanup = o.WrapWriter(&b, io.Discard, "", &tmpl)
 
 		fmt.Fprintln(w, "foo\nbar")
 		assert.Equal(t, "", b.String())
 		fmt.Fprintln(w, "baz")
 		assert.Equal(t, "", b.String())
-		assert.NoError(t, w.Close())
+		assert.NoError(t, cleanup())
 		assert.Equal(t, "::group::example-value\nfoo\nbar\nbaz\n::endgroup::\n", b.String())
 	})
 	t.Run("no output", func(t *testing.T) {
 		var b bytes.Buffer
-		var w = o.WrapWriter(&b, "", &tmpl).(io.WriteCloser)
+		var _, _, cleanup = o.WrapWriter(&b, io.Discard, "", &tmpl)
-		assert.NoError(t, w.Close())
+		assert.NoError(t, cleanup())
 		assert.Equal(t, "", b.String())
 	})
 }
@@ -73,7 +78,7 @@ func TestGroupWithBeginEnd(t *testing.T) {
 func TestPrefixed(t *testing.T) {
 	var b bytes.Buffer
 	var o output.Output = output.Prefixed{}
-	var w = o.WrapWriter(&b, "prefix", nil).(io.WriteCloser)
+	var w, _, cleanup = o.WrapWriter(&b, io.Discard, "prefix", nil)
 
 	t.Run("simple use cases", func(t *testing.T) {
 		b.Reset()
@@ -82,6 +87,7 @@ func TestPrefixed(t *testing.T) {
 		assert.Equal(t, "[prefix] foo\n[prefix] bar\n", b.String())
 		fmt.Fprintln(w, "baz")
 		assert.Equal(t, "[prefix] foo\n[prefix] bar\n[prefix] baz\n", b.String())
+		assert.NoError(t, cleanup())
 	})
 
 	t.Run("multiple writes for a single line", func(t *testing.T) {
@@ -92,7 +98,7 @@ func TestPrefixed(t *testing.T) {
 			assert.Equal(t, "", b.String())
 		}
 
-		assert.NoError(t, w.Close())
+		assert.NoError(t, cleanup())
 		assert.Equal(t, "[prefix] Test!\n", b.String())
 	})
 }

internal/output/prefixed.go

@@ -9,8 +9,9 @@ import (
 
 type Prefixed struct{}
 
-func (Prefixed) WrapWriter(w io.Writer, prefix string, _ Templater) io.Writer {
+func (Prefixed) WrapWriter(stdOut, _ io.Writer, prefix string, _ Templater) (io.Writer, io.Writer, CloseFunc) {
-	return &prefixWriter{writer: w, prefix: prefix}
+	pw := &prefixWriter{writer: stdOut, prefix: prefix}
+	return pw, pw, func() error { return pw.close() }
 }
 
 type prefixWriter struct {
@@ -28,7 +29,7 @@ func (pw *prefixWriter) Write(p []byte) (int, error) {
 	return n, pw.writeOutputLines(false)
 }
 
-func (pw *prefixWriter) Close() error {
+func (pw *prefixWriter) close() error {
 	return pw.writeOutputLines(true)
 }
 

task.go

@@ -449,19 +449,10 @@ func (e *Executor) runCommand(ctx context.Context, t *taskfile.Task, call taskfi
 		if err != nil {
 			return fmt.Errorf("task: failed to get variables: %w", err)
 		}
-		stdOut := outputWrapper.WrapWriter(e.Stdout, t.Prefix, outputTemplater)
+		stdOut, stdErr, close := outputWrapper.WrapWriter(e.Stdout, e.Stderr, t.Prefix, outputTemplater)
-		stdErr := outputWrapper.WrapWriter(e.Stderr, t.Prefix, outputTemplater)
-
 		defer func() {
-			if _, ok := stdOut.(*os.File); !ok {
+			if err := close(); err != nil {
-				if closer, ok := stdOut.(io.Closer); ok {
+				e.Logger.Errf(logger.Red, "task: unable to close writter: %v", err)
-					closer.Close()
-				}
-			}
-			if _, ok := stdErr.(*os.File); !ok {
-				if closer, ok := stdErr.(io.Closer); ok {
-					closer.Close()
-				}
 			}
 		}()