From 88698c018ca7958e3095334c96a89efc6eb6ec86 Mon Sep 17 00:00:00 2001
From: Martin Gencur <mgencur@redhat.com>
Date: Tue, 10 Oct 2023 11:02:01 +0200
Subject: [PATCH 01/12] Move shell package from knative.dev/hack under
 test/shell

---
 test/shell/assertions_test.go |  45 ++++++++
 test/shell/executor.go        | 188 ++++++++++++++++++++++++++++++++++
 test/shell/executor_test.go   | 160 +++++++++++++++++++++++++++++
 test/shell/fail-example.sh    |  17 +++
 test/shell/prefixer.go        |  68 ++++++++++++
 test/shell/prefixer_test.go   |  70 +++++++++++++
 test/shell/project.go         |  81 +++++++++++++++
 test/shell/project_test.go    |  35 +++++++
 test/shell/types.go           |  82 +++++++++++++++
 9 files changed, 746 insertions(+)
 create mode 100644 test/shell/assertions_test.go
 create mode 100644 test/shell/executor.go
 create mode 100644 test/shell/executor_test.go
 create mode 100755 test/shell/fail-example.sh
 create mode 100644 test/shell/prefixer.go
 create mode 100644 test/shell/prefixer_test.go
 create mode 100644 test/shell/project.go
 create mode 100644 test/shell/project_test.go
 create mode 100644 test/shell/types.go

diff --git a/test/shell/assertions_test.go b/test/shell/assertions_test.go
new file mode 100644
index 0000000000..814910571b
--- /dev/null
+++ b/test/shell/assertions_test.go
@@ -0,0 +1,45 @@
+/*
+Copyright 2020 The Knative 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
+
+    https://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 shell_test
+
+import (
+	"strings"
+	"testing"
+)
+
+type assertions struct {
+	t *testing.T
+}
+
+func (a assertions) NoError(err error) {
+	if err != nil {
+		a.t.Error(err)
+	}
+}
+
+func (a assertions) Contains(haystack, needle string) {
+	if !strings.Contains(haystack, needle) {
+		a.t.Errorf("wanted to \ncontain: %#v\n     in: %#v",
+			needle, haystack)
+	}
+}
+
+func (a assertions) Equal(want, got string) {
+	if got != want {
+		a.t.Errorf("want: %#v\n got:%#v", want, got)
+	}
+}
diff --git a/test/shell/executor.go b/test/shell/executor.go
new file mode 100644
index 0000000000..1486c6a8ca
--- /dev/null
+++ b/test/shell/executor.go
@@ -0,0 +1,188 @@
+/*
+Copyright 2020 The Knative 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
+
+    https://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 shell
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"os/exec"
+	"strings"
+	"time"
+)
+
+const (
+	defaultLabelOut = "[OUT]"
+	defaultLabelErr = "[ERR]"
+	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)
+	return &streamingExecutor{
+		ExecutorConfig: config,
+	}
+}
+
+// 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)
+	})
+}
+
+// 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)
+	})
+}
+
+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
+	}
+	if config.LabelErr == "" {
+		config.LabelErr = defaultLabelErr
+	}
+	if config.Environ == nil {
+		config.Environ = os.Environ()
+	}
+	if !config.SkipDate && config.DateFormat == "" {
+		config.DateFormat = time.StampMilli
+	}
+	if config.PrefixFunc == nil {
+		config.PrefixFunc = defaultPrefixFunc
+	}
+}
+
+func stream(bin string, cfg ExecutorConfig, label string) error {
+	c := exec.Command(bin)
+	c.Env = cfg.Environ
+	c.Stdout = NewPrefixer(cfg.Out, prefixFunc(StreamTypeOut, label, cfg))
+	c.Stderr = NewPrefixer(cfg.Err, prefixFunc(StreamTypeErr, label, cfg))
+	return c.Run()
+}
+
+func prefixFunc(st StreamType, label string, cfg ExecutorConfig) func() string {
+	return func() string {
+		return cfg.PrefixFunc(st, label, cfg)
+	}
+}
+
+func defaultPrefixFunc(st StreamType, label string, cfg ExecutorConfig) string {
+	sep := " "
+	var buf []string
+	if !cfg.SkipDate {
+		dt := time.Now().Format(cfg.DateFormat)
+		buf = append(buf, dt)
+	}
+	buf = append(buf, label)
+	switch st {
+	case StreamTypeOut:
+		buf = append(buf, cfg.LabelOut)
+	case StreamTypeErr:
+		buf = append(buf, cfg.LabelErr)
+	}
+	return strings.Join(buf, sep) + sep
+}
+
+func withTempScript(contents string, fn func(bin string) error) error {
+	tmpfile, err := os.CreateTemp("", "shellout-*.sh")
+	if err != nil {
+		return err
+	}
+	_, err = tmpfile.WriteString(contents)
+	if err != nil {
+		return err
+	}
+	err = tmpfile.Chmod(executeMode)
+	if err != nil {
+		return err
+	}
+	err = tmpfile.Close()
+	if err != nil {
+		return err
+	}
+	//defer func() {
+	//	// clean up
+	//	_ = os.Remove(tmpfile.Name())
+	//}()
+
+	return fn(tmpfile.Name())
+}
+
+func (fn *Function) scriptContent(location ProjectLocation, args []string) string {
+	return fmt.Sprintf(`#!/usr/bin/env bash
+
+set -Eeuo pipefail
+
+cd "%s"
+source %s
+
+%s %s
+`, location.RootPath(), fn.ScriptPath, fn.FunctionName, quoteArgs(args))
+}
+
+func (sc *Script) scriptContent(location ProjectLocation, args []string) string {
+	return fmt.Sprintf(`#!/usr/bin/env bash
+
+set -Eeuo pipefail
+
+cd "%s"
+%s %s
+`, location.RootPath(), sc.ScriptPath, quoteArgs(args))
+}
+
+func quoteArgs(args []string) string {
+	quoted := make([]string, len(args))
+	for i, arg := range args {
+		quoted[i] = "\"" + strings.ReplaceAll(arg, "\"", "\\\"") + "\""
+	}
+	return strings.Join(quoted, " ")
+}
diff --git a/test/shell/executor_test.go b/test/shell/executor_test.go
new file mode 100644
index 0000000000..27a948a4b2
--- /dev/null
+++ b/test/shell/executor_test.go
@@ -0,0 +1,160 @@
+/*
+Copyright 2020 The Knative 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
+
+    https://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 shell_test
+
+import (
+	"bytes"
+	"testing"
+
+	"knative.dev/pkg/test/shell"
+)
+
+func TestNewExecutor(t *testing.T) {
+	assert := assertions{t: t}
+	tests := []testcase{
+		helloWorldTestCase(t),
+		abortTestCase(t),
+		failExampleCase(t),
+		missingProjectLocationCase(),
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var outB, errB bytes.Buffer
+			tt.config.Out = &outB
+			tt.config.Err = &errB
+			executor := shell.NewExecutor(tt.config)
+			err := tt.op(executor)
+			if err != nil && !tt.wants.failed {
+				t.Errorf("%s: \n got: %#v\nfailed: %#v", tt.name, err, tt.failed)
+			}
+
+			for _, wantOut := range tt.wants.outs {
+				assert.Contains(outB.String(), wantOut)
+			}
+			for _, wantErr := range tt.wants.errs {
+				assert.Contains(errB.String(), wantErr)
+			}
+		})
+	}
+}
+
+func TestExecutorDefaults(t *testing.T) {
+	assert := assertions{t: t}
+	loc, err := shell.NewProjectLocation("../..")
+	assert.NoError(err)
+	exec := shell.NewExecutor(shell.ExecutorConfig{
+		ProjectLocation: loc,
+	})
+	err = exec.RunFunction(fn("true"))
+	assert.NoError(err)
+}
+
+func helloWorldTestCase(t *testing.T) testcase {
+	return testcase{
+		"echo Hello, World!",
+		config(t, func(cfg *shell.ExecutorConfig) {
+			cfg.SkipDate = true
+		}),
+		func(exec shell.Executor) error {
+			return exec.RunFunction(fn("echo"), "Hello, World!")
+		},
+		wants{
+			outs: []string{
+				"echo [OUT] Hello, World!",
+			},
+		},
+	}
+}
+
+func abortTestCase(t *testing.T) testcase {
+	return testcase{
+		"abort function",
+		config(t, func(cfg *shell.ExecutorConfig) {}),
+		func(exec shell.Executor) error {
+			return exec.RunFunction(fn("abort"), "reason")
+		},
+		wants{
+			failed: true,
+		},
+	}
+}
+
+func failExampleCase(t *testing.T) testcase {
+	return testcase{
+		"fail-example.sh",
+		config(t, func(cfg *shell.ExecutorConfig) {}),
+		func(exec shell.Executor) error {
+			return exec.RunScript(shell.Script{
+				Label:      "fail-example.sh",
+				ScriptPath: "test/shell/fail-example.sh",
+			}, "expected err")
+		},
+		wants{
+			failed: true,
+			errs: []string{
+				"expected err",
+			},
+		},
+	}
+}
+
+func missingProjectLocationCase() testcase {
+	return testcase{
+		"missing project location",
+		shell.ExecutorConfig{},
+		func(exec shell.Executor) error {
+			return exec.RunFunction(fn("id"))
+		},
+		wants{
+			failed: true,
+		},
+	}
+}
+
+type wants struct {
+	failed bool
+	outs   []string
+	errs   []string
+}
+
+type testcase struct {
+	name   string
+	config shell.ExecutorConfig
+	op     func(exec shell.Executor) error
+	wants
+}
+
+func config(t *testing.T, customize func(cfg *shell.ExecutorConfig)) shell.ExecutorConfig {
+	assert := assertions{t: t}
+	loc, err := shell.NewProjectLocation("../..")
+	assert.NoError(err)
+	cfg := shell.ExecutorConfig{
+		ProjectLocation: loc,
+	}
+	customize(&cfg)
+	return cfg
+}
+
+func fn(name string) shell.Function {
+	return shell.Function{
+		Script: shell.Script{
+			Label:      name,
+			ScriptPath: "vendor/knative.dev/hack/library.sh",
+		},
+		FunctionName: name,
+	}
+}
diff --git a/test/shell/fail-example.sh b/test/shell/fail-example.sh
new file mode 100755
index 0000000000..551ce662a3
--- /dev/null
+++ b/test/shell/fail-example.sh
@@ -0,0 +1,17 @@
+#!/usr/bin/env bash
+
+# Copyright 2020 The Knative 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
+#
+#     https://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.
+
+echo "$*" >&2
diff --git a/test/shell/prefixer.go b/test/shell/prefixer.go
new file mode 100644
index 0000000000..273ee1cc4f
--- /dev/null
+++ b/test/shell/prefixer.go
@@ -0,0 +1,68 @@
+/*
+Copyright 2020 The Knative 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
+
+    https://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 shell
+
+import (
+	"bytes"
+	"io"
+)
+
+// NewPrefixer creates a new prefixer that forwards all calls to Write() to
+// writer.Write() with all lines prefixed with the value of prefix. Having a
+// function instead of a static prefix allows to print timestamps or other
+// changing information.
+func NewPrefixer(writer io.Writer, prefix func() string) io.Writer {
+	return &prefixer{prefix: prefix, writer: writer, trailingNewline: true}
+}
+
+type prefixer struct {
+	prefix          func() string
+	writer          io.Writer
+	trailingNewline bool
+	buf             bytes.Buffer // reuse buffer to save allocations
+}
+
+func (pf *prefixer) Write(payload []byte) (int, error) {
+	pf.buf.Reset() // clear the buffer
+
+	for _, b := range payload {
+		if pf.trailingNewline {
+			pf.buf.WriteString(pf.prefix())
+			pf.trailingNewline = false
+		}
+
+		pf.buf.WriteByte(b)
+
+		if b == '\n' {
+			// do not print the prefix right after the newline character as this might
+			// be the very last character of the stream and we want to avoid a trailing prefix.
+			pf.trailingNewline = true
+		}
+	}
+
+	n, err := pf.writer.Write(pf.buf.Bytes())
+	if err != nil {
+		// never return more than original length to satisfy io.Writer interface
+		if n > len(payload) {
+			n = len(payload)
+		}
+		return n, err
+	}
+
+	// return original length to satisfy io.Writer interface
+	return len(payload), nil
+}
diff --git a/test/shell/prefixer_test.go b/test/shell/prefixer_test.go
new file mode 100644
index 0000000000..52168b9e5c
--- /dev/null
+++ b/test/shell/prefixer_test.go
@@ -0,0 +1,70 @@
+/*
+Copyright 2020 The Knative 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
+
+    https://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 shell_test
+
+import (
+	"bytes"
+	"strconv"
+	"testing"
+
+	"knative.dev/pkg/test/shell"
+)
+
+func TestNewPrefixer(t *testing.T) {
+	assert := assertions{t: t}
+	var lineno int64 = 0
+	tests := []struct {
+		name   string
+		prefix func() string
+		want   string
+	}{{
+		"static",
+		func() string {
+			return "[prefix] "
+		},
+		`[prefix] test string 1
+[prefix] test string 2
+`,
+	}, {
+		"empty",
+		func() string {
+			return ""
+		},
+		`test string 1
+test string 2
+`,
+	}, {
+		"dynamic",
+		func() string {
+			lineno++
+			return strconv.FormatInt(lineno, 10) + ") "
+		},
+		`1) test string 1
+2) test string 2
+`,
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			writer := &bytes.Buffer{}
+			wr := shell.NewPrefixer(writer, tt.prefix)
+			_, err := wr.Write([]byte("test string 1\ntest string 2\n"))
+			assert.NoError(err)
+			got := writer.String()
+			assert.Equal(tt.want, got)
+		})
+	}
+}
diff --git a/test/shell/project.go b/test/shell/project.go
new file mode 100644
index 0000000000..498cc34903
--- /dev/null
+++ b/test/shell/project.go
@@ -0,0 +1,81 @@
+/*
+Copyright 2020 The Knative 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
+
+    https://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 shell
+
+import (
+	"errors"
+	"fmt"
+	"path"
+	"regexp"
+	"runtime"
+)
+
+var (
+	// ErrCantGetCaller is raised when we can't calculate a caller of NewProjectLocation.
+	ErrCantGetCaller = errors.New("can't get caller")
+
+	// 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/pkg/test/shell package outside of allowed places")
+)
+
+// NewProjectLocation creates a ProjectLocation that is used to calculate
+// relative paths within the project.
+func NewProjectLocation(pathToRoot string) (ProjectLocation, error) {
+	pc, filename, _, ok := runtime.Caller(1)
+	if !ok {
+		return nil, ErrCantGetCaller
+	}
+	funcName := runtime.FuncForPC(pc).Name()
+	err := isCallsiteAllowed(funcName)
+	if err != nil {
+		return nil, err
+	}
+	return &callerLocation{
+		caller:     filename,
+		pathToRoot: pathToRoot,
+	}, nil
+}
+
+// RootPath return a path to root of the project.
+func (c *callerLocation) RootPath() string {
+	return path.Join(path.Dir(c.caller), c.pathToRoot)
+}
+
+// callerLocation holds a caller Go file, and a relative location to a project
+// root directory. This information can be used to calculate relative paths and
+// properly source shell scripts.
+type callerLocation struct {
+	caller     string
+	pathToRoot string
+}
+
+func isCallsiteAllowed(funcName string) error {
+	validPaths := []string{
+		"knative.+/test/upgrade",
+		"knative(:?\\.dev/|-)pkg/test/shell",
+	}
+	for _, validPath := range validPaths {
+		r := regexp.MustCompile(validPath)
+		if loc := r.FindStringIndex(funcName); loc != nil {
+			return nil
+		}
+	}
+	return fmt.Errorf("%w, tried using from: %s",
+		ErrCallerNotAllowed, funcName)
+}
diff --git a/test/shell/project_test.go b/test/shell/project_test.go
new file mode 100644
index 0000000000..60944db198
--- /dev/null
+++ b/test/shell/project_test.go
@@ -0,0 +1,35 @@
+/*
+Copyright 2020 The Knative 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
+
+    https://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 shell_test
+
+import (
+	"os"
+	"path"
+	"testing"
+
+	"knative.dev/pkg/test/shell"
+)
+
+func TestNewProjectLocation(t *testing.T) {
+	assert := assertions{t: t}
+	loc, err := shell.NewProjectLocation("../..")
+	assert.NoError(err)
+	goModPath := path.Join(loc.RootPath(), "go.mod")
+	bytes, err := os.ReadFile(goModPath)
+	assert.NoError(err)
+	assert.Contains(string(bytes), "module knative.dev/pkg")
+}
diff --git a/test/shell/types.go b/test/shell/types.go
new file mode 100644
index 0000000000..8e34515206
--- /dev/null
+++ b/test/shell/types.go
@@ -0,0 +1,82 @@
+/*
+Copyright 2020 The Knative 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
+
+    https://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 shell
+
+import "io"
+
+// ProjectLocation represents a project location on a file system.
+type ProjectLocation interface {
+	RootPath() string
+}
+
+// Script represents a script to be executed.
+type Script struct {
+	Label      string
+	ScriptPath string
+}
+
+// Function represents a function, whom will be sourced from Script file,
+// and executed.
+type Function struct {
+	Script
+	FunctionName string
+}
+
+// ExecutorConfig holds a executor configuration options.
+type ExecutorConfig struct {
+	ProjectLocation
+	Streams
+	Labels
+	Environ []string
+}
+
+// StreamType represets either output or error stream.
+type StreamType int
+
+const (
+	// StreamTypeOut represents process output stream.
+	StreamTypeOut StreamType = iota
+	// StreamTypeErr represents process error stream.
+	StreamTypeErr
+)
+
+// PrefixFunc is used to build a prefix that will be added to each line of the
+// script/function output or error stream.
+type PrefixFunc func(st StreamType, label string, config ExecutorConfig) string
+
+// Labels holds a labels to be used to prefix Out and Err streams of executed
+// shells scripts/functions.
+type Labels struct {
+	LabelOut   string
+	LabelErr   string
+	SkipDate   bool
+	DateFormat string
+	PrefixFunc
+}
+
+// Streams holds a streams of a shell scripts/functions.
+type Streams struct {
+	Out io.Writer
+	Err io.Writer
+}
+
+// Executor represents a executor that can execute shell scripts and call
+// functions directly.
+type Executor interface {
+	RunScript(script Script, args ...string) error
+	RunFunction(fn Function, args ...string) error
+}

From 974d18ba37f9d9aa0692279d237621aed7abcc47 Mon Sep 17 00:00:00 2001
From: Martin Gencur <mgencur@redhat.com>
Date: Tue, 10 Oct 2023 11:17:20 +0200
Subject: [PATCH 02/12] Provide Streams for shell executor that writes to t.Log

---
 test/shell/executor.go | 25 +++++++++++++++++++++++++
 test/shell/types.go    | 14 +++++++++++++-
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/test/shell/executor.go b/test/shell/executor.go
index 1486c6a8ca..04c76401c4 100644
--- a/test/shell/executor.go
+++ b/test/shell/executor.go
@@ -17,11 +17,13 @@ limitations under the License.
 package shell
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
 	"os"
 	"os/exec"
 	"strings"
+	"testing"
 	"time"
 )
 
@@ -42,6 +44,15 @@ func NewExecutor(config ExecutorConfig) Executor {
 	}
 }
 
+// TestingTStreams returns Streams which writes to t.Log and marks
+// the test as failed if anything is written to Streams.Err.
+func TestingTStreams(t testing.TB) Streams {
+	return Streams{
+		Out: testingWriter{t: t},
+		Err: testingWriter{t: t, markFailed: true},
+	}
+}
+
 // RunScript executes a shell script with args.
 func (s *streamingExecutor) RunScript(script Script, args ...string) error {
 	err := validate(s.ExecutorConfig)
@@ -186,3 +197,17 @@ 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)
+	if w.markFailed {
+		w.t.Fail()
+	}
+
+	return n, nil
+}
diff --git a/test/shell/types.go b/test/shell/types.go
index 8e34515206..58aa745e23 100644
--- a/test/shell/types.go
+++ b/test/shell/types.go
@@ -16,7 +16,10 @@ limitations under the License.
 
 package shell
 
-import "io"
+import (
+	"io"
+	"testing"
+)
 
 // ProjectLocation represents a project location on a file system.
 type ProjectLocation interface {
@@ -44,6 +47,15 @@ type ExecutorConfig struct {
 	Environ []string
 }
 
+// testingWriter writes to the given testing.TB.
+type testingWriter struct {
+	t testing.TB
+
+	// If true, the test will be marked as failed if this testingWriter is
+	// ever used.
+	markFailed bool
+}
+
 // StreamType represets either output or error stream.
 type StreamType int
 

From 3cfcbc6e2b698434961527ac7fe886164539ea4f Mon Sep 17 00:00:00 2001
From: Martin Gencur <mgencur@redhat.com>
Date: Tue, 10 Oct 2023 11:37:51 +0200
Subject: [PATCH 03/12] Remove temporary comment

---
 test/shell/executor.go | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/test/shell/executor.go b/test/shell/executor.go
index 04c76401c4..57503b634b 100644
--- a/test/shell/executor.go
+++ b/test/shell/executor.go
@@ -160,10 +160,10 @@ func withTempScript(contents string, fn func(bin string) error) error {
 	if err != nil {
 		return err
 	}
-	//defer func() {
-	//	// clean up
-	//	_ = os.Remove(tmpfile.Name())
-	//}()
+	defer func() {
+		// clean up
+		_ = os.Remove(tmpfile.Name())
+	}()
 
 	return fn(tmpfile.Name())
 }

From f6e0044ceda21b0df9f6c256b8112ac592dc3f7f Mon Sep 17 00:00:00 2001
From: Martin Gencur <mgencur@redhat.com>
Date: Tue, 10 Oct 2023 13:04:21 +0200
Subject: [PATCH 04/12] Fix style

---
 test/shell/assertions_test.go | 2 +-
 test/shell/executor.go        | 2 +-
 test/shell/executor_test.go   | 2 +-
 test/shell/fail-example.sh    | 2 +-
 test/shell/prefixer.go        | 2 +-
 test/shell/prefixer_test.go   | 2 +-
 test/shell/project.go         | 2 +-
 test/shell/project_test.go    | 2 +-
 test/shell/types.go           | 2 +-
 9 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/test/shell/assertions_test.go b/test/shell/assertions_test.go
index 814910571b..4800d7ac0e 100644
--- a/test/shell/assertions_test.go
+++ b/test/shell/assertions_test.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,
diff --git a/test/shell/executor.go b/test/shell/executor.go
index 57503b634b..8aca394d9f 100644
--- a/test/shell/executor.go
+++ b/test/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,
diff --git a/test/shell/executor_test.go b/test/shell/executor_test.go
index 27a948a4b2..33cd283152 100644
--- a/test/shell/executor_test.go
+++ b/test/shell/executor_test.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,
diff --git a/test/shell/fail-example.sh b/test/shell/fail-example.sh
index 551ce662a3..4815aef9de 100755
--- a/test/shell/fail-example.sh
+++ b/test/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,
diff --git a/test/shell/prefixer.go b/test/shell/prefixer.go
index 273ee1cc4f..f267c1e3b4 100644
--- a/test/shell/prefixer.go
+++ b/test/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,
diff --git a/test/shell/prefixer_test.go b/test/shell/prefixer_test.go
index 52168b9e5c..3853e7c15e 100644
--- a/test/shell/prefixer_test.go
+++ b/test/shell/prefixer_test.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,
diff --git a/test/shell/project.go b/test/shell/project.go
index 498cc34903..2d5ded547c 100644
--- a/test/shell/project.go
+++ b/test/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,
diff --git a/test/shell/project_test.go b/test/shell/project_test.go
index 60944db198..78dedbade1 100644
--- a/test/shell/project_test.go
+++ b/test/shell/project_test.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,
diff --git a/test/shell/types.go b/test/shell/types.go
index 58aa745e23..f8adae28b3 100644
--- a/test/shell/types.go
+++ b/test/shell/types.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,

From 25f0adf7487e6dd493a1e5951c03b16ef94b43e3 Mon Sep 17 00:00:00 2001
From: Martin Gencur <mgencur@redhat.com>
Date: Tue, 10 Oct 2023 13:43:24 +0200
Subject: [PATCH 05/12] Fix lint

---
 test/shell/prefixer_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/shell/prefixer_test.go b/test/shell/prefixer_test.go
index 3853e7c15e..824bf375de 100644
--- a/test/shell/prefixer_test.go
+++ b/test/shell/prefixer_test.go
@@ -26,7 +26,7 @@ import (
 
 func TestNewPrefixer(t *testing.T) {
 	assert := assertions{t: t}
-	var lineno int64 = 0
+	var lineno int64
 	tests := []struct {
 		name   string
 		prefix func() string

From 64dda1efdfa8b9c011c5aaa90b55822b3440c4b7 Mon Sep 17 00:00:00 2001
From: Martin Gencur <mgencur@redhat.com>
Date: Wed, 11 Oct 2023 07:46:11 +0200
Subject: [PATCH 06/12] Do not fail when writing to stderr

* use own interface TestingT
---
 test/shell/executor.go | 11 ++++-------
 test/shell/types.go    | 12 ++++++------
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/test/shell/executor.go b/test/shell/executor.go
index 8aca394d9f..f347321ad7 100644
--- a/test/shell/executor.go
+++ b/test/shell/executor.go
@@ -23,7 +23,6 @@ import (
 	"os"
 	"os/exec"
 	"strings"
-	"testing"
 	"time"
 )
 
@@ -46,10 +45,11 @@ func NewExecutor(config ExecutorConfig) Executor {
 
 // TestingTStreams returns Streams which writes to t.Log and marks
 // the test as failed if anything is written to Streams.Err.
-func TestingTStreams(t testing.TB) Streams {
+func TestingTStreams(t TestingT) Streams {
+	tWriter := testingWriter{t: t}
 	return Streams{
-		Out: testingWriter{t: t},
-		Err: testingWriter{t: t, markFailed: true},
+		Out: tWriter,
+		Err: tWriter,
 	}
 }
 
@@ -205,9 +205,6 @@ func (w testingWriter) Write(p []byte) (n int, err error) {
 	p = bytes.TrimRight(p, "\n")
 
 	w.t.Logf("%s", p)
-	if w.markFailed {
-		w.t.Fail()
-	}
 
 	return n, nil
 }
diff --git a/test/shell/types.go b/test/shell/types.go
index f8adae28b3..36e9af3af9 100644
--- a/test/shell/types.go
+++ b/test/shell/types.go
@@ -18,7 +18,6 @@ package shell
 
 import (
 	"io"
-	"testing"
 )
 
 // ProjectLocation represents a project location on a file system.
@@ -47,13 +46,14 @@ type ExecutorConfig struct {
 	Environ []string
 }
 
+// TestingT is used by testingWriter and allows passing testing.T.
+type TestingT interface {
+	Logf(args ...any)
+}
+
 // testingWriter writes to the given testing.TB.
 type testingWriter struct {
-	t testing.TB
-
-	// If true, the test will be marked as failed if this testingWriter is
-	// ever used.
-	markFailed bool
+	t TestingT
 }
 
 // StreamType represets either output or error stream.

From 551d95b46beb947eba4b37596c7c0da75641c986 Mon Sep 17 00:00:00 2001
From: Martin Gencur <mgencur@redhat.com>
Date: Wed, 11 Oct 2023 08:57:41 +0200
Subject: [PATCH 07/12] Fix arguments for Logf

---
 test/shell/types.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/shell/types.go b/test/shell/types.go
index 36e9af3af9..fb6e2a19ac 100644
--- a/test/shell/types.go
+++ b/test/shell/types.go
@@ -48,7 +48,7 @@ type ExecutorConfig struct {
 
 // TestingT is used by testingWriter and allows passing testing.T.
 type TestingT interface {
-	Logf(args ...any)
+	Logf(format string, args ...any)
 }
 
 // testingWriter writes to the given testing.TB.

From 7e238a11a09e7be19ccc251ed184c44f29970d86 Mon Sep 17 00:00:00 2001
From: Martin Gencur <mgencur@redhat.com>
Date: Wed, 11 Oct 2023 09:33:33 +0200
Subject: [PATCH 08/12] Fix comments

---
 test/shell/executor.go | 3 +--
 test/shell/types.go    | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/test/shell/executor.go b/test/shell/executor.go
index f347321ad7..bc2213f83f 100644
--- a/test/shell/executor.go
+++ b/test/shell/executor.go
@@ -43,8 +43,7 @@ func NewExecutor(config ExecutorConfig) Executor {
 	}
 }
 
-// TestingTStreams returns Streams which writes to t.Log and marks
-// the test as failed if anything is written to Streams.Err.
+// TestingTStreams returns Streams which writes to test log.
 func TestingTStreams(t TestingT) Streams {
 	tWriter := testingWriter{t: t}
 	return Streams{
diff --git a/test/shell/types.go b/test/shell/types.go
index fb6e2a19ac..94a4a792d0 100644
--- a/test/shell/types.go
+++ b/test/shell/types.go
@@ -51,7 +51,7 @@ type TestingT interface {
 	Logf(format string, args ...any)
 }
 
-// testingWriter writes to the given testing.TB.
+// testingWriter implements io.Writer and writes to given testing.T log.
 type testingWriter struct {
 	t TestingT
 }

From ae893654ed91c0d6f68efaa473e29b73ec3ce1ae Mon Sep 17 00:00:00 2001
From: Martin Gencur <mgencur@redhat.com>
Date: Thu, 12 Oct 2023 08:35:24 +0200
Subject: [PATCH 09/12] NewExecutor function requires TestingT and
 ProjectLocation

---
 test/shell/executor.go      | 25 +++++++++++++------------
 test/shell/executor_test.go | 11 +++++------
 test/shell/types.go         |  5 ++++-
 3 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/test/shell/executor.go b/test/shell/executor.go
index bc2213f83f..df5739586c 100644
--- a/test/shell/executor.go
+++ b/test/shell/executor.go
@@ -35,16 +35,23 @@ const (
 // 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 {
+// testingTStreams returns Streams which writes to test log.
+func testingTStreams(t TestingT) Streams {
 	tWriter := testingWriter{t: t}
 	return Streams{
 		Out: tWriter,
@@ -88,12 +95,6 @@ func validate(config ExecutorConfig) error {
 }
 
 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
 	}
diff --git a/test/shell/executor_test.go b/test/shell/executor_test.go
index 33cd283152..10d7d7114b 100644
--- a/test/shell/executor_test.go
+++ b/test/shell/executor_test.go
@@ -34,9 +34,10 @@ func TestNewExecutor(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			var outB, errB bytes.Buffer
-			tt.config.Out = &outB
-			tt.config.Err = &errB
-			executor := shell.NewExecutor(tt.config)
+			executor := shell.NewExecutor(t, tt.config.ProjectLocation, func(cfg *shell.ExecutorConfig) {
+				cfg.Streams.Out = &outB
+				cfg.Streams.Err = &errB
+			})
 			err := tt.op(executor)
 			if err != nil && !tt.wants.failed {
 				t.Errorf("%s: \n got: %#v\nfailed: %#v", tt.name, err, tt.failed)
@@ -56,9 +57,7 @@ func TestExecutorDefaults(t *testing.T) {
 	assert := assertions{t: t}
 	loc, err := shell.NewProjectLocation("../..")
 	assert.NoError(err)
-	exec := shell.NewExecutor(shell.ExecutorConfig{
-		ProjectLocation: loc,
-	})
+	exec := shell.NewExecutor(t, loc)
 	err = exec.RunFunction(fn("true"))
 	assert.NoError(err)
 }
diff --git a/test/shell/types.go b/test/shell/types.go
index 94a4a792d0..14bbbac05a 100644
--- a/test/shell/types.go
+++ b/test/shell/types.go
@@ -20,6 +20,9 @@ import (
 	"io"
 )
 
+// Option overrides configuration options in ExecutorConfig.
+type Option func(*ExecutorConfig)
+
 // ProjectLocation represents a project location on a file system.
 type ProjectLocation interface {
 	RootPath() string
@@ -38,7 +41,7 @@ type Function struct {
 	FunctionName string
 }
 
-// ExecutorConfig holds a executor configuration options.
+// ExecutorConfig holds executor configuration options.
 type ExecutorConfig struct {
 	ProjectLocation
 	Streams

From b0c1be67cae4de1502d3d57a646fb055d70b2e4f Mon Sep 17 00:00:00 2001
From: Martin Gencur <mgencur@redhat.com>
Date: Thu, 12 Oct 2023 08:45:52 +0200
Subject: [PATCH 10/12] Move pkg/test/shell under pkg/test/upgrade/shell

---
 test/{ => upgrade}/shell/assertions_test.go |  0
 test/{ => upgrade}/shell/executor.go        |  0
 test/{ => upgrade}/shell/executor_test.go   |  8 +--
 test/{ => upgrade}/shell/fail-example.sh    |  0
 test/upgrade/shell/logstream.diff           | 79 +++++++++++++++++++++
 test/{ => upgrade}/shell/prefixer.go        |  0
 test/{ => upgrade}/shell/prefixer_test.go   |  2 +-
 test/{ => upgrade}/shell/project.go         |  4 +-
 test/{ => upgrade}/shell/project_test.go    |  4 +-
 test/{ => upgrade}/shell/types.go           |  0
 10 files changed, 88 insertions(+), 9 deletions(-)
 rename test/{ => upgrade}/shell/assertions_test.go (100%)
 rename test/{ => upgrade}/shell/executor.go (100%)
 rename test/{ => upgrade}/shell/executor_test.go (94%)
 rename test/{ => upgrade}/shell/fail-example.sh (100%)
 create mode 100644 test/upgrade/shell/logstream.diff
 rename test/{ => upgrade}/shell/prefixer.go (100%)
 rename test/{ => upgrade}/shell/prefixer_test.go (97%)
 rename test/{ => upgrade}/shell/project.go (95%)
 rename test/{ => upgrade}/shell/project_test.go (90%)
 rename test/{ => upgrade}/shell/types.go (100%)

diff --git a/test/shell/assertions_test.go b/test/upgrade/shell/assertions_test.go
similarity index 100%
rename from test/shell/assertions_test.go
rename to test/upgrade/shell/assertions_test.go
diff --git a/test/shell/executor.go b/test/upgrade/shell/executor.go
similarity index 100%
rename from test/shell/executor.go
rename to test/upgrade/shell/executor.go
diff --git a/test/shell/executor_test.go b/test/upgrade/shell/executor_test.go
similarity index 94%
rename from test/shell/executor_test.go
rename to test/upgrade/shell/executor_test.go
index 10d7d7114b..f9f8ef3a8f 100644
--- a/test/shell/executor_test.go
+++ b/test/upgrade/shell/executor_test.go
@@ -20,7 +20,7 @@ import (
 	"bytes"
 	"testing"
 
-	"knative.dev/pkg/test/shell"
+	"knative.dev/pkg/test/upgrade/shell"
 )
 
 func TestNewExecutor(t *testing.T) {
@@ -55,7 +55,7 @@ func TestNewExecutor(t *testing.T) {
 
 func TestExecutorDefaults(t *testing.T) {
 	assert := assertions{t: t}
-	loc, err := shell.NewProjectLocation("../..")
+	loc, err := shell.NewProjectLocation("../../..")
 	assert.NoError(err)
 	exec := shell.NewExecutor(t, loc)
 	err = exec.RunFunction(fn("true"))
@@ -99,7 +99,7 @@ func failExampleCase(t *testing.T) testcase {
 		func(exec shell.Executor) error {
 			return exec.RunScript(shell.Script{
 				Label:      "fail-example.sh",
-				ScriptPath: "test/shell/fail-example.sh",
+				ScriptPath: "test/upgrade/shell/fail-example.sh",
 			}, "expected err")
 		},
 		wants{
@@ -139,7 +139,7 @@ type testcase struct {
 
 func config(t *testing.T, customize func(cfg *shell.ExecutorConfig)) shell.ExecutorConfig {
 	assert := assertions{t: t}
-	loc, err := shell.NewProjectLocation("../..")
+	loc, err := shell.NewProjectLocation("../../..")
 	assert.NoError(err)
 	cfg := shell.ExecutorConfig{
 		ProjectLocation: loc,
diff --git a/test/shell/fail-example.sh b/test/upgrade/shell/fail-example.sh
similarity index 100%
rename from test/shell/fail-example.sh
rename to test/upgrade/shell/fail-example.sh
diff --git a/test/upgrade/shell/logstream.diff b/test/upgrade/shell/logstream.diff
new file mode 100644
index 0000000000..76306f8534
--- /dev/null
+++ b/test/upgrade/shell/logstream.diff
@@ -0,0 +1,79 @@
+diff --git a/test/shell/executor.go b/test/shell/executor.go
+index f347321a..03da568d 100644
+--- a/test/shell/executor.go
++++ b/test/shell/executor.go
+@@ -24,6 +24,8 @@ import (
+ 	"os/exec"
+ 	"strings"
+ 	"time"
++
++	"knative.dev/pkg/test/logging"
+ )
+ 
+ const (
+@@ -43,13 +45,12 @@ func NewExecutor(config ExecutorConfig) Executor {
+ 	}
+ }
+ 
+-// TestingTStreams returns Streams which writes to t.Log and marks
+-// the test as failed if anything is written to Streams.Err.
+-func TestingTStreams(t TestingT) Streams {
+-	tWriter := testingWriter{t: t}
++// LogStreams returns Streams which writes to given FormatLogger.
++func LogStreams(logFn logging.FormatLogger) Streams {
++	writer := logWriter{logf: logFn}
+ 	return Streams{
+-		Out: tWriter,
+-		Err: tWriter,
++		Out: writer,
++		Err: writer,
+ 	}
+ }
+ 
+@@ -198,13 +199,13 @@ func quoteArgs(args []string) string {
+ 	return strings.Join(quoted, " ")
+ }
+ 
+-func (w testingWriter) Write(p []byte) (n int, err error) {
++func (w logWriter) 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)
++	w.logf("%s", p)
+ 
+ 	return n, nil
+ }
+diff --git a/test/shell/types.go b/test/shell/types.go
+index fb6e2a19..2dda1996 100644
+--- a/test/shell/types.go
++++ b/test/shell/types.go
+@@ -18,6 +18,8 @@ package shell
+ 
+ import (
+ 	"io"
++
++	"knative.dev/pkg/test/logging"
+ )
+ 
+ // ProjectLocation represents a project location on a file system.
+@@ -46,14 +48,9 @@ type ExecutorConfig struct {
+ 	Environ []string
+ }
+ 
+-// TestingT is used by testingWriter and allows passing testing.T.
+-type TestingT interface {
+-	Logf(format string, args ...any)
+-}
+-
+-// testingWriter writes to the given testing.TB.
+-type testingWriter struct {
+-	t TestingT
++// logWriter implements io.Writer and writes to the given logf.
++type logWriter struct {
++	logf logging.FormatLogger
+ }
+ 
+ // StreamType represets either output or error stream.
diff --git a/test/shell/prefixer.go b/test/upgrade/shell/prefixer.go
similarity index 100%
rename from test/shell/prefixer.go
rename to test/upgrade/shell/prefixer.go
diff --git a/test/shell/prefixer_test.go b/test/upgrade/shell/prefixer_test.go
similarity index 97%
rename from test/shell/prefixer_test.go
rename to test/upgrade/shell/prefixer_test.go
index 824bf375de..184b340d02 100644
--- a/test/shell/prefixer_test.go
+++ b/test/upgrade/shell/prefixer_test.go
@@ -21,7 +21,7 @@ import (
 	"strconv"
 	"testing"
 
-	"knative.dev/pkg/test/shell"
+	"knative.dev/pkg/test/upgrade/shell"
 )
 
 func TestNewPrefixer(t *testing.T) {
diff --git a/test/shell/project.go b/test/upgrade/shell/project.go
similarity index 95%
rename from test/shell/project.go
rename to test/upgrade/shell/project.go
index 2d5ded547c..c58bc713c3 100644
--- a/test/shell/project.go
+++ b/test/upgrade/shell/project.go
@@ -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/pkg/test/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/|-)pkg/test/shell",
+		"knative(:?\\.dev/|-)pkg/test/upgrade/shell",
 	}
 	for _, validPath := range validPaths {
 		r := regexp.MustCompile(validPath)
diff --git a/test/shell/project_test.go b/test/upgrade/shell/project_test.go
similarity index 90%
rename from test/shell/project_test.go
rename to test/upgrade/shell/project_test.go
index 78dedbade1..93df1cf088 100644
--- a/test/shell/project_test.go
+++ b/test/upgrade/shell/project_test.go
@@ -21,12 +21,12 @@ import (
 	"path"
 	"testing"
 
-	"knative.dev/pkg/test/shell"
+	"knative.dev/pkg/test/upgrade/shell"
 )
 
 func TestNewProjectLocation(t *testing.T) {
 	assert := assertions{t: t}
-	loc, err := shell.NewProjectLocation("../..")
+	loc, err := shell.NewProjectLocation("../../..")
 	assert.NoError(err)
 	goModPath := path.Join(loc.RootPath(), "go.mod")
 	bytes, err := os.ReadFile(goModPath)
diff --git a/test/shell/types.go b/test/upgrade/shell/types.go
similarity index 100%
rename from test/shell/types.go
rename to test/upgrade/shell/types.go

From 3abb74389b04e8b53327410a40a070e3f1308f36 Mon Sep 17 00:00:00 2001
From: Martin Gencur <mgencur@redhat.com>
Date: Thu, 12 Oct 2023 09:01:36 +0200
Subject: [PATCH 11/12] ProjectLocation is always passed

It is now a required argument for NewExecutor function
---
 test/upgrade/shell/executor.go      | 19 -------------------
 test/upgrade/shell/executor_test.go | 14 --------------
 2 files changed, 33 deletions(-)

diff --git a/test/upgrade/shell/executor.go b/test/upgrade/shell/executor.go
index df5739586c..d41ccc765a 100644
--- a/test/upgrade/shell/executor.go
+++ b/test/upgrade/shell/executor.go
@@ -18,7 +18,6 @@ package shell
 
 import (
 	"bytes"
-	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -32,9 +31,6 @@ 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.
 func NewExecutor(t TestingT, loc ProjectLocation, opts ...Option) Executor {
 	config := &ExecutorConfig{
@@ -61,10 +57,6 @@ func testingTStreams(t TestingT) Streams {
 
 // 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)
@@ -73,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)
@@ -87,13 +75,6 @@ type streamingExecutor struct {
 	ExecutorConfig
 }
 
-func validate(config ExecutorConfig) error {
-	if config.ProjectLocation == nil {
-		return ErrNoProjectLocation
-	}
-	return nil
-}
-
 func configureDefaultValues(config *ExecutorConfig) {
 	if config.LabelOut == "" {
 		config.LabelOut = defaultLabelOut
diff --git a/test/upgrade/shell/executor_test.go b/test/upgrade/shell/executor_test.go
index f9f8ef3a8f..b0332278b1 100644
--- a/test/upgrade/shell/executor_test.go
+++ b/test/upgrade/shell/executor_test.go
@@ -29,7 +29,6 @@ func TestNewExecutor(t *testing.T) {
 		helloWorldTestCase(t),
 		abortTestCase(t),
 		failExampleCase(t),
-		missingProjectLocationCase(),
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -111,19 +110,6 @@ func failExampleCase(t *testing.T) testcase {
 	}
 }
 
-func missingProjectLocationCase() testcase {
-	return testcase{
-		"missing project location",
-		shell.ExecutorConfig{},
-		func(exec shell.Executor) error {
-			return exec.RunFunction(fn("id"))
-		},
-		wants{
-			failed: true,
-		},
-	}
-}
-
 type wants struct {
 	failed bool
 	outs   []string

From 270ad2005cbd48f2fc718144712549d706ce9410 Mon Sep 17 00:00:00 2001
From: Martin Gencur <mgencur@redhat.com>
Date: Thu, 12 Oct 2023 09:17:36 +0200
Subject: [PATCH 12/12] Remove redundant file

---
 test/upgrade/shell/logstream.diff | 79 -------------------------------
 1 file changed, 79 deletions(-)
 delete mode 100644 test/upgrade/shell/logstream.diff

diff --git a/test/upgrade/shell/logstream.diff b/test/upgrade/shell/logstream.diff
deleted file mode 100644
index 76306f8534..0000000000
--- a/test/upgrade/shell/logstream.diff
+++ /dev/null
@@ -1,79 +0,0 @@
-diff --git a/test/shell/executor.go b/test/shell/executor.go
-index f347321a..03da568d 100644
---- a/test/shell/executor.go
-+++ b/test/shell/executor.go
-@@ -24,6 +24,8 @@ import (
- 	"os/exec"
- 	"strings"
- 	"time"
-+
-+	"knative.dev/pkg/test/logging"
- )
- 
- const (
-@@ -43,13 +45,12 @@ func NewExecutor(config ExecutorConfig) Executor {
- 	}
- }
- 
--// TestingTStreams returns Streams which writes to t.Log and marks
--// the test as failed if anything is written to Streams.Err.
--func TestingTStreams(t TestingT) Streams {
--	tWriter := testingWriter{t: t}
-+// LogStreams returns Streams which writes to given FormatLogger.
-+func LogStreams(logFn logging.FormatLogger) Streams {
-+	writer := logWriter{logf: logFn}
- 	return Streams{
--		Out: tWriter,
--		Err: tWriter,
-+		Out: writer,
-+		Err: writer,
- 	}
- }
- 
-@@ -198,13 +199,13 @@ func quoteArgs(args []string) string {
- 	return strings.Join(quoted, " ")
- }
- 
--func (w testingWriter) Write(p []byte) (n int, err error) {
-+func (w logWriter) 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)
-+	w.logf("%s", p)
- 
- 	return n, nil
- }
-diff --git a/test/shell/types.go b/test/shell/types.go
-index fb6e2a19..2dda1996 100644
---- a/test/shell/types.go
-+++ b/test/shell/types.go
-@@ -18,6 +18,8 @@ package shell
- 
- import (
- 	"io"
-+
-+	"knative.dev/pkg/test/logging"
- )
- 
- // ProjectLocation represents a project location on a file system.
-@@ -46,14 +48,9 @@ type ExecutorConfig struct {
- 	Environ []string
- }
- 
--// TestingT is used by testingWriter and allows passing testing.T.
--type TestingT interface {
--	Logf(format string, args ...any)
--}
--
--// testingWriter writes to the given testing.TB.
--type testingWriter struct {
--	t TestingT
-+// logWriter implements io.Writer and writes to the given logf.
-+type logWriter struct {
-+	logf logging.FormatLogger
- }
- 
- // StreamType represets either output or error stream.