Shell executor logs through testing.T in upgrade tests (#14495)

* Use shell.TestingTStreams in upgrade tests

* TMP: Use mgencurs fork of knative.dev/pkg

* Induce error

* Revert "Induce error"

This reverts commit f5a5b50.

* Update deps

* Induce error

* Update deps

* Update usage of shell.NewExecutor

* Update deps

* Try printing each line separately

* Update deps

* Revert "Try printing each line separately"

This reverts commit 52e151a.

* Revert "Induce error"

This reverts commit bf60222.

test/upgrade/installation/shell.go

@@ -17,10 +17,10 @@ limitations under the License.
 package installation
 
 import (
-	"os"
+	"testing"
 
-	"knative.dev/hack/shell"
 	pkgupgrade "knative.dev/pkg/test/upgrade"
+	"knative.dev/pkg/test/upgrade/shell"
 )
 
 // Head installs Knative Serving from the HEAD of the default branch.
@@ -36,21 +36,18 @@ func LatestRelease() pkgupgrade.Operation {
 func install(installName, shellFunc string) pkgupgrade.Operation {
 	return pkgupgrade.NewOperation(installName, func(c pkgupgrade.Context) {
 		c.Log.Info("Running shell function: ", shellFunc)
-		if err := callShellFunction(shellFunc); err != nil {
+		if err := callShellFunction(shellFunc, c.T); err != nil {
 			c.T.Error(err)
 		}
 	})
 }
 
-func callShellFunction(funcName string) error {
+func callShellFunction(funcName string, t *testing.T) error {
 	loc, err := shell.NewProjectLocation("../../..")
 	if err != nil {
 		return err
 	}
-	exec := shell.NewExecutor(shell.ExecutorConfig{
-		ProjectLocation: loc,
-		Environ:         os.Environ(),
-	})
+	exec := shell.NewExecutor(t, loc)
 	fn := shell.Function{
 		Script: shell.Script{
 			Label:      funcName,

vendor/knative.dev/hack/shell/executor.go → vendor/knative.dev/pkg/test/upgrade/shell/executor.go

@@ -5,7 +5,7 @@ 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
 
-    https://www.apache.org/licenses/LICENSE-2.0
+    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,
@@ -17,7 +17,7 @@ limitations under the License.
 package shell
 
 import (
-	"errors"
+	"bytes"
 	"fmt"
 	"os"
 	"os/exec"
@@ -31,23 +31,32 @@ const (
 	executeMode     = 0700
 )
 
-// ErrNoProjectLocation is returned if user didnt provided the project location.
-var ErrNoProjectLocation = errors.New("project location isn't provided")
-
-// NewExecutor creates a new executor from given config.
-func NewExecutor(config ExecutorConfig) Executor {
-	configureDefaultValues(&config)
+// NewExecutor creates a new executor.
+func NewExecutor(t TestingT, loc ProjectLocation, opts ...Option) Executor {
+	config := &ExecutorConfig{
+		ProjectLocation: loc,
+		Streams:         testingTStreams(t),
+	}
+	for _, opt := range opts {
+		opt(config)
+	}
+	configureDefaultValues(config)
 	return &streamingExecutor{
-		ExecutorConfig: config,
+		ExecutorConfig: *config,
+	}
+}
+
+// testingTStreams returns Streams which writes to test log.
+func testingTStreams(t TestingT) Streams {
+	tWriter := testingWriter{t: t}
+	return Streams{
+		Out: tWriter,
+		Err: tWriter,
 	}
 }
 
 // RunScript executes a shell script with args.
 func (s *streamingExecutor) RunScript(script Script, args ...string) error {
-	err := validate(s.ExecutorConfig)
-	if err != nil {
-		return err
-	}
 	cnt := script.scriptContent(s.ProjectLocation, args)
 	return withTempScript(cnt, func(bin string) error {
 		return stream(bin, s.ExecutorConfig, script.Label)
@@ -56,10 +65,6 @@ func (s *streamingExecutor) RunScript(script Script, args ...string) error {
 
 // RunFunction executes a shell function with args.
 func (s *streamingExecutor) RunFunction(fn Function, args ...string) error {
-	err := validate(s.ExecutorConfig)
-	if err != nil {
-		return err
-	}
 	cnt := fn.scriptContent(s.ProjectLocation, args)
 	return withTempScript(cnt, func(bin string) error {
 		return stream(bin, s.ExecutorConfig, fn.Label)
@@ -70,20 +75,7 @@ type streamingExecutor struct {
 	ExecutorConfig
 }
 
-func validate(config ExecutorConfig) error {
-	if config.ProjectLocation == nil {
-		return ErrNoProjectLocation
-	}
-	return nil
-}
-
 func configureDefaultValues(config *ExecutorConfig) {
-	if config.Out == nil {
-		config.Out = os.Stdout
-	}
-	if config.Err == nil {
-		config.Err = os.Stderr
-	}
 	if config.LabelOut == "" {
 		config.LabelOut = defaultLabelOut
 	}
@@ -186,3 +178,14 @@ func quoteArgs(args []string) string {
 	}
 	return strings.Join(quoted, " ")
 }
+
+func (w testingWriter) Write(p []byte) (n int, err error) {
+	n = len(p)
+
+	// Strip trailing newline because t.Log always adds one.
+	p = bytes.TrimRight(p, "\n")
+
+	w.t.Logf("%s", p)
+
+	return n, nil
+}

vendor/knative.dev/hack/shell/fail-example.sh → vendor/knative.dev/pkg/test/upgrade/shell/fail-example.sh

@@ -6,7 +6,7 @@
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
 #
-#     https://www.apache.org/licenses/LICENSE-2.0
+#     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,

vendor/knative.dev/hack/shell/prefixer.go → vendor/knative.dev/pkg/test/upgrade/shell/prefixer.go

@@ -5,7 +5,7 @@ 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
 
-    https://www.apache.org/licenses/LICENSE-2.0
+    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,

vendor/knative.dev/hack/shell/project.go → vendor/knative.dev/pkg/test/upgrade/shell/project.go

@@ -5,7 +5,7 @@ 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
 
-    https://www.apache.org/licenses/LICENSE-2.0
+    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,
@@ -31,7 +31,7 @@ var (
 	// ErrCallerNotAllowed is raised when user tries to use this shell-out package
 	// outside of allowed places. This package is deprecated from start and was
 	// introduced to allow rewriting of shell code to Golang in small chunks.
-	ErrCallerNotAllowed = errors.New("don't try use knative.dev/hack/shell package outside of allowed places")
+	ErrCallerNotAllowed = errors.New("don't try use knative.dev/pkg/test/upgrade/shell package outside of allowed places")
 )
 
 // NewProjectLocation creates a ProjectLocation that is used to calculate
@@ -68,7 +68,7 @@ type callerLocation struct {
 func isCallsiteAllowed(funcName string) error {
 	validPaths := []string{
 		"knative.+/test/upgrade",
-		"knative(:?\\.dev/|-)hack/shell",
+		"knative(:?\\.dev/|-)pkg/test/upgrade/shell",
 	}
 	for _, validPath := range validPaths {
 		r := regexp.MustCompile(validPath)