src: exported client constructor with options (#883)

* update root and version structure and help text

* fix: limit openshift int test with tag

* refactor: commands to use simplifed, unified constructor

* fix ineffectual assignment lint error

* cleanup

* add repository to run command

* callout for forthcoming s2i builder impl

* lint errors

* re-add the deferred client factory

* remove setNamespaceFlag now that it is persistent

* avoid side-effect of global-mutating deploy tests

* reduce line-by-line difference for PR ease

* simplificaiton of tests and comment lines for further PR ease purposes

* reduce inconsequential differences for ease of PR

* tests to RootCommandConfig

* review comment updates

* fix lint errors

* replace stdlib Setenv in tests

Using t.Setenv will require an update to go1.17, which is out of scope
for this PR.

* pass ClientFactory throughout

* explicitly empty test command args

See https://github.com/spf13/cobra/pull/155

Errors can still be encountered when, for example, using precomiled
tests.  Explicitly setting constructed command args to the empty slice
ensures we avoid hitting any futher edge cases.

client_int_test.go

@@ -204,7 +204,7 @@ func TestRemoteRepositories(t *testing.T) {
 // match those created by the kn func plugin CLI.
 func newClient(verbose bool) *fn.Client {
 	builder := buildpacks.NewBuilder(verbose)
-	pusher := docker.NewPusher(verbose)
+	pusher := docker.NewPusher(docker.WithVerbose(verbose))
 	deployer := knative.NewDeployer(DefaultNamespace, verbose)
 	remover := knative.NewRemover(DefaultNamespace, verbose)
 	lister := knative.NewLister(DefaultNamespace, verbose)

client_test.go

@@ -63,7 +63,7 @@ func TestClient_InstantiationCreatesRepositoriesPath(t *testing.T) {
 	}
 
 	// Instruct the system to use the above test root directory as the home dir.
-	os.Setenv("XDG_CONFIG_HOME", rootAbs)
+	defer WithEnvVar(t, "XDG_CONFIG_HOME", rootAbs)()
 
 	// The expected full path to the repositories should be:
 	expected := filepath.Join(rootAbs, "func", "repositories")

cmd/build.go

@@ -12,13 +12,6 @@ import (
 	fn "knative.dev/kn-plugin-func"
 )
 
-func buildConfigToClientOptions(cfg buildConfig) ClientOptions {
-	return ClientOptions{
-		Registry: cfg.Registry,
-		Verbose:  cfg.Verbose,
-	}
-}
-
 func NewBuildCmd(newClient ClientFactory) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "build",
@@ -87,7 +80,7 @@ func ValidNamespaceAndRegistry(path string) survey.Validator {
 	}
 }
 
-func runBuild(cmd *cobra.Command, _ []string, clientFn ClientFactory) (err error) {
+func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err error) {
 	config, err := newBuildConfig().Prompt()
 	if err != nil {
 		if errors.Is(err, terminal.InterruptErr) {
@@ -151,7 +144,16 @@ func runBuild(cmd *cobra.Command, _ []string, clientFn ClientFactory) (err error
 		config.Registry = ""
 	}
 
-	client := clientFn(buildConfigToClientOptions(config))
+	// TODO(lkingland): The below deferred options gathering is what will
+	// re-enable the addition of alternative implementations of the Builder,
+	// unblocking PR https://github.com/knative-sandbox/kn-plugin-func/pull/842
+	// the implementation of which will be inserted here.
+
+	// Create a client using the registry defined in config plus any additional
+	// options provided (such as mocks for testing)
+	client, done := newClient(ClientConfig{Verbose: config.Verbose},
+		fn.WithRegistry(config.Registry))
+	defer done()
 
 	err = client.Build(cmd.Context(), config.Path)
 	if err == nil && config.Push {

cmd/build_test.go

@@ -8,6 +8,7 @@ import (
 
 	fn "knative.dev/kn-plugin-func"
 	"knative.dev/kn-plugin-func/mock"
+	. "knative.dev/kn-plugin-func/testing"
 )
 
 // TestBuild_InvalidRegistry ensures that running build specifying the name of the
@@ -19,7 +20,7 @@ func TestBuild_InvalidRegistry(t *testing.T) {
 	)
 
 	// Run this test in a temporary directory
-	defer fromTempDir(t)()
+	defer Fromtemp(t)()
 	// Write a func.yaml config which does not specify an image
 	funcYaml := `name: testymctestface
 namespace: ""
@@ -38,11 +39,10 @@ created: 2021-01-01T00:00:00+00:00
 		t.Fatal(err)
 	}
 
-	// Create a command with a client constructor fn that instantiates a client
+	// Create build command that will use a mock builder.
-	// with a the mocked builder.
+	cmd := NewBuildCmd(NewClientFactory(func() *fn.Client {
-	cmd := NewBuildCmd(func(options ClientOptions) *fn.Client {
 		return fn.New(fn.WithBuilder(builder))
-	})
+	}))
 
 	// Execute the command
 	cmd.SetArgs(args)
@@ -99,7 +99,7 @@ created: 2009-11-10 23:00:00`,
 				},
 			}
 			mockBuilder := mock.NewBuilder()
-			cmd := NewBuildCmd(func(options ClientOptions) *fn.Client {
+			cmd := NewBuildCmd(NewClientFactory(func() *fn.Client {
 				pusher := mockPusher
 				if tt.wantErr {
 					pusher = failPusher
@@ -108,7 +108,7 @@ created: 2009-11-10 23:00:00`,
 					fn.WithBuilder(mockBuilder),
 					fn.WithPusher(pusher),
 				)
-			})
+			}))
 
 			tempDir, err := os.MkdirTemp("", "func-tests")
 			if err != nil {

cmd/client.go

@@ -1,6 +1,10 @@
 package cmd
 
 import (
+	"fmt"
+	"net/http"
+	"os"
+
 	fn "knative.dev/kn-plugin-func"
 	"knative.dev/kn-plugin-func/buildpacks"
 	"knative.dev/kn-plugin-func/docker"
@@ -12,101 +16,120 @@ import (
 	"knative.dev/kn-plugin-func/progress"
 )
 
-type ClientOptions struct {
+// ClientConfig settings for use with NewClient
+// These are the minimum settings necessary to create the default client
+// instance which has most subsystems initialized.
+type ClientConfig struct {
+	// Namespace in the remote cluster to use for any client commands which
+	// touch the remote.  Optional.  Empty namespace indicates the namespace
+	// currently configured in the client's connection should be used.
 	Namespace string
-	Registry         string
+
-	Repository       string
+	// Verbose logging.  By default logging output is kept to the bare minimum.
-	RepositoriesPath string
+	// Use this flag to configure verbose logging throughout.
 	Verbose bool
 }
 
-type ClientFactory func(opts ClientOptions) *fn.Client
+// ClientFactory defines a constructor which assists in the creation of a Client
+// for use by commands.
+// See the NewClient constructor which is the fully populated ClientFactory used
+// by commands by default.
+// See NewClientFactory which constructs a minimal CientFactory for use
+// during testing.
+type ClientFactory func(ClientConfig, ...fn.Option) (*fn.Client, func())
 
-// NewDefaultClientFactory returns a function that creates instances of function.Client and a cleanup routine.
+// NewClientFactory enables simple instantiation of an fn.Client, such as
-//
+// for mocking during tests or for minimal api usage.
-// This function may allocate resources that are used by produced instances of function.Client.
+// Given is a minimal Client constructor, Returned is full ClientFactory
-//
+// with the aspects of normal (full) Client construction (namespace, verbosity
-// To free these resources (after instances of function.Client are no longer in use)
+// level, additional options and the returned cleanup function) ignored.
-// caller of this function has to invoke the cleanup routine.
+func NewClientFactory(n func() *fn.Client) ClientFactory {
-//
+	return func(_ ClientConfig, _ ...fn.Option) (*fn.Client, func()) {
-// Usage:
+		return n(), func() {}
-//   newClient, cleanUp := NewDefaultClientFactory()
+	}
-//   defer cleanUp()
+}
-//   fnClient := newClient()
-//   // use your fnClient here...
-func NewDefaultClientFactory() (newClient ClientFactory, cleanUp func() error) {
 
-	var transportOpts []fnhttp.Option
+// NewClient constructs an fn.Client with the majority of
-	var additionalCredLoaders []creds.CredentialsCallback
+// the concrete implementations set.  Provide additional Options to this constructor
+// to override or augment as needed, or override the ClientFactory passed to
+// commands entirely to mock for testing. Note the reutrned cleanup function.
+// 'Namespace' is optional.  If not provided (see DefaultNamespace commentary),
+// the currently configured is used.
+// 'Verbose' indicates the system should write out a higher amount of logging.
+// Example:
+//   client, done := NewClient("",false)
+//   defer done()
+func NewClient(cfg ClientConfig, options ...fn.Option) (*fn.Client, func()) {
+	var (
+		p = progress.New(cfg.Verbose) // updates the CLI
+		t = newTransport()            // may provide a custom impl which proxies
+		c = newCredentialsProvider(t) // for accessing registries
+		o = []fn.Option{              // standard (shared) options for all commands
+			fn.WithVerbose(cfg.Verbose),
+			fn.WithProgressListener(p),
+			fn.WithTransport(t),
+			fn.WithBuilder(buildpacks.NewBuilder(cfg.Verbose)),
+			fn.WithRemover(knative.NewRemover(cfg.Namespace, cfg.Verbose)),
+			fn.WithDescriber(knative.NewDescriber(cfg.Namespace, cfg.Verbose)),
+			fn.WithLister(knative.NewLister(cfg.Namespace, cfg.Verbose)),
+			fn.WithRunner(docker.NewRunner(cfg.Verbose)),
+			fn.WithDeployer(knative.NewDeployer(cfg.Namespace, cfg.Verbose)),
+			fn.WithPipelinesProvider(tekton.NewPipelinesProvider(
+				tekton.WithNamespace(cfg.Namespace),
+				tekton.WithProgressListener(p),
+				tekton.WithCredentialsProvider(c),
+				tekton.WithVerbose(cfg.Verbose))),
+			fn.WithPusher(docker.NewPusher(
+				docker.WithCredentialsProvider(c),
+				docker.WithProgressListener(p),
+				docker.WithTransport(t),
+				docker.WithVerbose(cfg.Verbose))),
+		}
+	)
 
-	switch {
+	// Client is constructed with standard options plus any additional options
-	case openshift.IsOpenShift():
+	// which either augment or override the defaults.
-		transportOpts = append(transportOpts, openshift.WithOpenShiftServiceCA())
+	client := fn.New(append(o, options...)...)
-		additionalCredLoaders = openshift.GetDockerCredentialLoaders()
+
-	default:
+	// A deferrable cleanup function which is used to perform any cleanup, such
+	// as closing the transport
+	cleanup := func() {
+		if err := t.Close(); err != nil {
+			fmt.Fprintf(os.Stderr, "error closing http transport. %v", err)
+		}
 	}
 
-	transport := fnhttp.NewRoundTripper(transportOpts...)
+	return client, cleanup
-	cleanUp = func() error {
+}
-		return transport.Close()
+
+// newTransport returns a transport with cluster-flavor-specific variations
+// which take advantage of additional features offered by cluster variants.
+func newTransport() fnhttp.RoundTripCloser {
+	if openshift.IsOpenShift() {
+		return fnhttp.NewRoundTripper(openshift.WithOpenShiftServiceCA())
 	}
 
-	newClient = func(clientOptions ClientOptions) *fn.Client {
+	// Other cluster variants ...
-		verbose := clientOptions.Verbose
-		builder := buildpacks.NewBuilder(verbose)
 
-		progressListener := progress.New(verbose)
+	return fnhttp.NewRoundTripper() // Default (vanilla k8s)
+}
 
-		credentialsProvider := creds.NewCredentialsProvider(
+// newCredentialsProvider returns a credentials provider which possibly
+// has cluster-flavor specific additional credential loaders to take advantage
+// of features or configuration nuances of cluster variants.
+func newCredentialsProvider(t http.RoundTripper) docker.CredentialsProvider {
+	options := []creds.Opt{
 		creds.WithPromptForCredentials(newPromptForCredentials()),
 		creds.WithPromptForCredentialStore(newPromptForCredentialStore()),
-			creds.WithTransport(transport),
+		creds.WithTransport(t),
-			creds.WithAdditionalCredentialLoaders(additionalCredLoaders...))
-
-		pusher := docker.NewPusher(verbose,
-			docker.WithCredentialsProvider(credentialsProvider),
-			docker.WithProgressListener(progressListener),
-			docker.WithTransport(transport))
-
-		deployer := knative.NewDeployer(clientOptions.Namespace, verbose)
-
-		pipelinesProvider := tekton.NewPipelinesProvider(verbose,
-			tekton.WithNamespace(clientOptions.Namespace),
-			tekton.WithProgressListener(progressListener),
-			tekton.WithCredentialsProvider(credentialsProvider))
-
-		remover := knative.NewRemover(clientOptions.Namespace, verbose)
-
-		describer := knative.NewDescriber(clientOptions.Namespace, verbose)
-
-		lister := knative.NewLister(clientOptions.Namespace, verbose)
-
-		runner := docker.NewRunner(verbose)
-
-		opts := []fn.Option{
-			fn.WithRepository(clientOptions.Repository), // URI of repository override
-			fn.WithRegistry(clientOptions.Registry),
-			fn.WithVerbose(verbose),
-			fn.WithTransport(transport),
-			fn.WithProgressListener(progressListener),
-			fn.WithBuilder(builder),
-			fn.WithPipelinesProvider(pipelinesProvider),
-			fn.WithRemover(remover),
-			fn.WithDescriber(describer),
-			fn.WithLister(lister),
-			fn.WithRunner(runner),
-			fn.WithDeployer(deployer),
-			fn.WithPusher(pusher),
 	}
-
+	// The OpenShift variant has additional ways to load credentials
-		if clientOptions.RepositoriesPath != "" {
+	if openshift.IsOpenShift() {
-			opts = append(opts, fn.WithRepositoriesPath(clientOptions.RepositoriesPath)) // path to repositories in disk
+		options = append(options,
+			creds.WithAdditionalCredentialLoaders(openshift.GetDockerCredentialLoaders()...))
 	}
-
+	// Other cluster variants can be supported here
-		return fn.New(opts...)
+	return creds.NewCredentialsProvider(options...)
-	}
-
-	return newClient, cleanUp
 }
 
 func GetDefaultRegistry() string {

cmd/config_labels_test.go

@@ -106,6 +106,7 @@ func TestNewConfigLabelsCmd(t *testing.T) {
 	labels := &loaderSaver.f.Labels
 
 	cmd := NewConfigLabelsCmd(&loaderSaver)
+	cmd.SetArgs([]string{}) // Do not use test command args
 
 	run := createRunFunc(cmd, t)
 
@@ -143,6 +144,7 @@ func TestListLabels(t *testing.T) {
 	*labels = append(*labels, p("a", "b"), p("c", "d"))
 
 	cmd := NewConfigLabelsCmd(&loaderSaver)
+	cmd.SetArgs([]string{}) // Do not use test command args
 
 	ctx := context.Background()
 	c, _, err := vt10x.NewVT10XConsole()
@@ -151,8 +153,6 @@ func TestListLabels(t *testing.T) {
 	}
 	defer c.Close()
 
-	cmd.SetArgs(make([]string, 0))
-
 	errChan := make(chan error, 1)
 	func() {
 		var err error

cmd/create.go

@@ -25,14 +25,6 @@ type ErrInvalidRuntime error
 // ErrInvalidTemplate indicates that the passed template was invalid.
 type ErrInvalidTemplate error
 
-func createConfigToClientOptions(cfg createConfig) ClientOptions {
-	return ClientOptions{
-		RepositoriesPath: cfg.RepositoriesPath,
-		Repository:       cfg.Repository,
-		Verbose:          cfg.Verbose,
-	}
-}
-
 // NewCreateCmd creates a create command using the given client creator.
 func NewCreateCmd(newClient ClientFactory) *cobra.Command {
 	cmd := &cobra.Command{
@@ -89,16 +81,14 @@ EXAMPLES
 	cmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all options interactively (Env: $FUNC_CONFIRM)")
 
 	// Help Action
-	cmd.SetHelpFunc(func(cmd *cobra.Command, args []string) {
+	cmd.SetHelpFunc(func(cmd *cobra.Command, args []string) { runCreateHelp(cmd, args, newClient) })
-		runCreateHelp(cmd, args, newClient)
-	})
 
 	// Run Action
 	cmd.RunE = func(cmd *cobra.Command, args []string) error {
 		return runCreate(cmd, args, newClient)
 	}
 
-	// Tab Completion
+	// Tab completion
 	if err := cmd.RegisterFlagCompletionFunc("language", newRuntimeCompletionFunc(newClient)); err != nil {
 		fmt.Fprintf(os.Stderr, "unable to provide language runtime suggestions: %v", err)
 	}
@@ -122,7 +112,10 @@ func runCreate(cmd *cobra.Command, args []string, newClient ClientFactory) (err
 	// Client
 	// From environment variables, flags, arguments, and user prompts if --confirm
 	// (in increasing levels of precidence)
-	client := newClient(createConfigToClientOptions(cfg))
+	client, done := newClient(ClientConfig{Verbose: cfg.Verbose},
+		fn.WithRepository(cfg.Repository),             // Use exactly this repo OR
+		fn.WithRepositoriesPath(cfg.RepositoriesPath)) // Path on disk to installed repos
+	defer done()
 
 	// Validate - a deeper validation than that which is performed when
 	// instantiating the client with the raw config above.
@@ -163,7 +156,10 @@ func runCreateHelp(cmd *cobra.Command, args []string, newClient ClientFactory) {
 	cfg, err := newCreateConfig(cmd, args, newClient)
 	failSoft(err)
 
-	client := newClient(createConfigToClientOptions(cfg))
+	client, done := newClient(ClientConfig{Verbose: cfg.Verbose},
+		fn.WithRepositoriesPath(cfg.RepositoriesPath),
+		fn.WithRepository(cfg.Repository))
+	defer done()
 
 	options, err := runtimeTemplateOptions(client) // human-friendly
 	failSoft(err)
@@ -258,7 +254,8 @@ func newCreateConfig(cmd *cobra.Command, args []string, newClient ClientFactory)
 
 	// Create a tempoarary client for use by the following prompts to complete
 	// runtime/template suggestions etc
-	client := newClient(createConfigToClientOptions(cfg))
+	client, done := newClient(ClientConfig{Verbose: cfg.Verbose})
+	defer done()
 
 	// IN confirm mode.  If also in an interactive terminal, run prompts.
 	if interactiveTerminal() {
@@ -508,7 +505,8 @@ func newRuntimeCompletionFunc(newClient ClientFactory) flagCompletionFunc {
 		if err != nil {
 			fmt.Fprintf(os.Stderr, "error creating client config for flag completion: %v", err)
 		}
-		client := newClient(createConfigToClientOptions(cfg))
+		client, done := newClient(ClientConfig{Verbose: cfg.Verbose})
+		defer done()
 		return CompleteRuntimeList(cmd, args, toComplete, client)
 	}
 }
@@ -519,7 +517,8 @@ func newTemplateCompletionFunc(newClient ClientFactory) flagCompletionFunc {
 		if err != nil {
 			fmt.Fprintf(os.Stderr, "error creating client config for flag completion: %v", err)
 		}
-		client := newClient(createConfigToClientOptions(cfg))
+		client, done := newClient(ClientConfig{Verbose: cfg.Verbose})
+		defer done()
 		return CompleteTemplateList(cmd, args, toComplete, client)
 	}
 }

cmd/create_test.go

@@ -2,26 +2,20 @@ package cmd
 
 import (
 	"errors"
-	"fmt"
-	"io/ioutil"
-	"os"
 	"path/filepath"
 	"testing"
 
-	fn "knative.dev/kn-plugin-func"
+	. "knative.dev/kn-plugin-func/testing"
 	"knative.dev/kn-plugin-func/utils"
 )
 
 // TestCreate_Execute ensures that an invocation of create with minimal settings
 // and valid input completes without error; degenerate case.
 func TestCreate_Execute(t *testing.T) {
-	defer fromTempDir(t)()
+	defer Fromtemp(t)()
 
-	// command with a client factory which yields a fully default client.
+	cmd := NewCreateCmd(NewClient)
-	cmd := NewCreateCmd(func(ClientOptions) *fn.Client { return fn.New() })
+	cmd.SetArgs([]string{"--language", "go"})
-	cmd.SetArgs([]string{
-		fmt.Sprintf("--language=%s", "go"),
-	})
 
 	if err := cmd.Execute(); err != nil {
 		t.Fatal(err)
@@ -31,11 +25,10 @@ func TestCreate_Execute(t *testing.T) {
 // TestCreate_NoRuntime ensures that an invocation of create must be
 // done with a runtime.
 func TestCreate_NoRuntime(t *testing.T) {
-	defer fromTempDir(t)()
+	defer Fromtemp(t)()
 
-	// command with a client factory which yields a fully default client.
+	cmd := NewCreateCmd(NewClient)
-	cmd := NewCreateCmd(func(ClientOptions) *fn.Client { return fn.New() })
+	cmd.SetArgs([]string{}) // Do not use test command args
-	cmd.SetArgs([]string{})
 
 	err := cmd.Execute()
 	var e ErrNoRuntime
@@ -47,13 +40,10 @@ func TestCreate_NoRuntime(t *testing.T) {
 // TestCreate_WithNoRuntime ensures that an invocation of create must be
 // done with one of the valid runtimes only.
 func TestCreate_WithInvalidRuntime(t *testing.T) {
-	defer fromTempDir(t)()
+	defer Fromtemp(t)()
 
-	// command with a client factory which yields a fully default client.
+	cmd := NewCreateCmd(NewClient)
-	cmd := NewCreateCmd(func(ClientOptions) *fn.Client { return fn.New() })
+	cmd.SetArgs([]string{"--language", "invalid"})
-	cmd.SetArgs([]string{
-		fmt.Sprintf("--language=%s", "test"),
-	})
 
 	err := cmd.Execute()
 	var e ErrInvalidRuntime
@@ -65,14 +55,10 @@ func TestCreate_WithInvalidRuntime(t *testing.T) {
 // TestCreate_InvalidTemplate ensures that an invocation of create must be
 // done with one of the valid templates only.
 func TestCreate_InvalidTemplate(t *testing.T) {
-	defer fromTempDir(t)()
+	defer Fromtemp(t)()
 
-	// command with a client factory which yields a fully default client.
+	cmd := NewCreateCmd(NewClient)
-	cmd := NewCreateCmd(func(ClientOptions) *fn.Client { return fn.New() })
+	cmd.SetArgs([]string{"--language", "go", "--template", "invalid"})
-	cmd.SetArgs([]string{
-		fmt.Sprintf("--language=%s", "go"),
-		fmt.Sprintf("--template=%s", "events"),
-	})
 
 	err := cmd.Execute()
 	var e ErrInvalidTemplate
@@ -84,14 +70,11 @@ func TestCreate_InvalidTemplate(t *testing.T) {
 // TestCreate_ValidatesName ensures that the create command only accepts
 // DNS-1123 labels for Function name.
 func TestCreate_ValidatesName(t *testing.T) {
-	defer fromTempDir(t)()
+	defer Fromtemp(t)()
-
-	// Create a new Create command with a fn.Client construtor
-	// which returns a default (noop) client suitable for tests.
-	cmd := NewCreateCmd(func(ClientOptions) *fn.Client { return fn.New() })
 
 	// Execute the command with a function name containing invalid characters and
 	// confirm the expected error is returned
+	cmd := NewCreateCmd(NewClient)
 	cmd.SetArgs([]string{"invalid!"})
 	err := cmd.Execute()
 	var e utils.ErrInvalidFunctionName
@@ -100,78 +83,27 @@ func TestCreate_ValidatesName(t *testing.T) {
 	}
 }
 
-// TestCreate_RepositoriesPath ensures that the create command utilizes the
+// TestCreateConfig_RepositoriesPath ensures that the create command utilizes
-// expected repositories path, respecting the setting for XDG_CONFIG_PATH
+// the expected repositories path, respecting the setting for XDG_CONFIG_PATH
 // when deriving the default
-func TestCreate_RepositoriesPath(t *testing.T) {
+func TestCreateConfig_RepositoriesPath(t *testing.T) {
-	defer fromTempDir(t)()
+	defer Fromtemp(t)()
 
 	// Update XDG_CONFIG_HOME to point to some arbitrary location.
-	xdgConfigHome, err := ioutil.TempDir("", "alice")
+	xdgConfigHome := t.TempDir()
+	defer WithEnvVar(t, "XDG_CONFIG_HOME", xdgConfigHome)()
+
+	// The expected full path is XDG_CONFIG_HOME/func/repositories
+	expected := filepath.Join(xdgConfigHome, "func", "repositories")
+
+	cmd := NewCreateCmd(NewClient)
+	cmd.SetArgs([]string{}) // Do not use test command args
+	cfg, err := newCreateConfig(cmd, []string{}, NewClient)
 	if err != nil {
 		t.Fatal(err)
 	}
-	os.Setenv("XDG_CONFIG_HOME", xdgConfigHome)
 
-	// The expected full path to repositories:
-	expected := filepath.Join(xdgConfigHome, "func", "repositories")
-
-	// Create command takes a function which will be invoked with the final
-	// state of the createConfig, usually used to do fn.Client instantiation
-	// after flags, environment variables, etc. are calculated.  In this case it
-	// will validate the test condition:  that config reflects the value of
-	// XDG_CONFIG_HOME, and secondarily the path suffix `func/repositories`.
-	cmd := NewCreateCmd(func(cfg ClientOptions) *fn.Client {
 	if cfg.RepositoriesPath != expected {
 		t.Fatalf("expected repositories default path to be '%v', got '%v'", expected, cfg.RepositoriesPath)
 	}
-		return fn.New()
-	})
-	cmd.SetArgs([]string{
-		fmt.Sprintf("--language=%s", "go"),
-	})
-	// Invoke the command, which is an airball, but does invoke the client
-	// constructor, which which evaluates the aceptance condition of ensuring the
-	// default repositories path was updated based on XDG_CONFIG_HOME.
-	if err = cmd.Execute(); err != nil {
-		t.Fatalf("unexpected error running 'create' with a default (noop) client instance: %v", err)
-	}
-}
-
-// Helpers ----
-
-// change directory into a new temp directory.
-// returned is a closure which cleans up; intended to be run as a defer:
-//    defer within(t, /some/path)()
-func fromTempDir(t *testing.T) func() {
-	t.Helper()
-	tmp := mktmp(t) // create temp directory
-	owd := pwd(t)   // original working directory
-	cd(t, tmp)      // change to the temp directory
-	return func() { // return a deferable cleanup closure
-		os.RemoveAll(tmp) // remove temp directory
-		cd(t, owd)        // change director back to original
-	}
-}
-
-func mktmp(t *testing.T) string {
-	d, err := ioutil.TempDir("", "dir")
-	if err != nil {
-		t.Fatal(err)
-	}
-	return d
-}
-
-func pwd(t *testing.T) string {
-	d, err := os.Getwd()
-	if err != nil {
-		t.Fatal(err)
-	}
-	return d
-}
-
-func cd(t *testing.T, dir string) {
-	if err := os.Chdir(dir); err != nil {
-		t.Fatal(err)
-	}
 }

cmd/delete.go

@@ -32,12 +32,12 @@ No local files are deleted.
 `,
 		SuggestFor:        []string{"remove", "rm", "del"},
 		ValidArgsFunction: CompleteFunctionList,
-		PreRunE:           bindEnv("path", "confirm", "namespace", "all"),
+		PreRunE:           bindEnv("path", "confirm", "all"),
+		SilenceUsage:      true, // no usage dump on error
 	}
 
 	cmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all configuration options (Env: $FUNC_CONFIRM)")
 	cmd.Flags().StringP("all", "a", "true", "Delete all resources created for a function, eg. Pipelines, Secrets, etc. (Env: $FUNC_ALL) (allowed values: \"true\", \"false\")")
-	setNamespaceFlag(cmd)
 	setPathFlag(cmd)
 
 	cmd.SetHelpFunc(defaultTemplatedHelp)
@@ -49,7 +49,7 @@ No local files are deleted.
 	return cmd
 }
 
-func runDelete(cmd *cobra.Command, args []string, clientFn ClientFactory) (err error) {
+func runDelete(cmd *cobra.Command, args []string, newClient ClientFactory) (err error) {
 	config, err := newDeleteConfig(args).Prompt()
 	if err != nil {
 		if err == terminal.InterruptErr {
@@ -87,10 +87,8 @@ func runDelete(cmd *cobra.Command, args []string, clientFn ClientFactory) (err e
 	}
 
 	// Create a client instance from the now-final config
-	client := clientFn(ClientOptions{
+	client, done := newClient(ClientConfig{Namespace: config.Namespace, Verbose: config.Verbose})
-		Namespace: config.Namespace,
+	defer done()
-		Verbose:   config.Verbose,
-	})
 
 	// Invoke remove using the concrete client impl
 	return client.Remove(cmd.Context(), function, config.DeleteAll)

cmd/delete_test.go

@@ -6,6 +6,7 @@ import (
 
 	fn "knative.dev/kn-plugin-func"
 	"knative.dev/kn-plugin-func/mock"
+	. "knative.dev/kn-plugin-func/testing"
 )
 
 // TestDelete_ByName ensures that running delete specifying the name of the
@@ -13,7 +14,6 @@ import (
 func TestDelete_ByName(t *testing.T) {
 	var (
 		testname = "testname"        // explicit name for the Function
-		args     = []string{testname} // passed as the lone argument
 		remover  = mock.NewRemover() // with a mock remover
 	)
 
@@ -28,14 +28,12 @@ func TestDelete_ByName(t *testing.T) {
 
 	// Create a command with a client constructor fn that instantiates a client
 	// with a the mocked remover.
-	cmd := NewDeleteCmd(func(ClientOptions) *fn.Client {
+	cmd := NewDeleteCmd(NewClientFactory(func() *fn.Client {
 		return fn.New(fn.WithRemover(remover))
-	})
+	}))
 
-	// Execute the command
+	cmd.SetArgs([]string{testname})
-	cmd.SetArgs(args)
+	if err := cmd.Execute(); err != nil {
-	err := cmd.Execute()
-	if err != nil {
 		t.Fatal(err)
 	}
 
@@ -49,7 +47,7 @@ func TestDelete_ByName(t *testing.T) {
 // context invokes remove and with the correct name (reads name from func.yaml)
 func TestDelete_ByProject(t *testing.T) {
 	// from within a new temporary directory
-	defer fromTempDir(t)()
+	defer Fromtemp(t)()
 
 	// Write a func.yaml config which specifies a name
 	funcYaml := `name: bar
@@ -80,12 +78,12 @@ created: 2021-01-01T00:00:00+00:00
 
 	// Command with a Client constructor that returns  client with the
 	// mocked remover.
-	cmd := NewDeleteCmd(func(ClientOptions) *fn.Client {
+	cmd := NewDeleteCmd(NewClientFactory(func() *fn.Client {
 		return fn.New(fn.WithRemover(remover))
-	})
+	}))
+	cmd.SetArgs([]string{}) // Do not use test command args
 
 	// Execute the command simulating no arguments.
-	cmd.SetArgs([]string{})
 	err := cmd.Execute()
 	if err != nil {
 		t.Fatal(err)
@@ -108,9 +106,9 @@ func TestDelete_NameAndPathExclusivity(t *testing.T) {
 	remover := mock.NewRemover()
 
 	// Command with a Client constructor using the mock remover.
-	cmd := NewDeleteCmd(func(ClientOptions) *fn.Client {
+	cmd := NewDeleteCmd(NewClientFactory(func() *fn.Client {
 		return fn.New(fn.WithRemover(remover))
-	})
+	}))
 
 	// Execute the command simulating the invalid argument combination of both
 	// a path and an explicit name.

cmd/deploy.go

@@ -17,14 +17,6 @@ import (
 	"knative.dev/kn-plugin-func/docker/creds"
 )
 
-func deployConfigToClientOptions(cfg deployConfig) ClientOptions {
-	return ClientOptions{
-		Namespace: cfg.Namespace,
-		Verbose:   cfg.Verbose,
-		Registry:  cfg.Registry,
-	}
-}
-
 func NewDeployCmd(newClient ClientFactory) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "deploy",
@@ -51,7 +43,7 @@ that is pushed to an image registry, and finally the function's Knative service
 {{.Name}} deploy --image quay.io/myuser/myfunc -n myns
 `,
 		SuggestFor: []string{"delpoy", "deplyo"},
-		PreRunE:    bindEnv("image", "namespace", "path", "registry", "confirm", "build", "push", "git-url", "git-branch", "git-dir"),
+		PreRunE:    bindEnv("image", "path", "registry", "confirm", "build", "push", "git-url", "git-branch", "git-dir"),
 	}
 
 	cmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all configuration options (Env: $FUNC_CONFIRM)")
@@ -66,7 +58,6 @@ that is pushed to an image registry, and finally the function's Knative service
 	cmd.Flags().StringP("build", "b", fn.DefaultBuildType, fmt.Sprintf("Build specifies the way the function should be built. Supported types are %s (Env: $FUNC_BUILD)", fn.SupportedBuildTypes(true)))
 	cmd.Flags().BoolP("push", "u", true, "Attempt to push the function image to registry before deploying (Env: $FUNC_PUSH)")
 	setPathFlag(cmd)
-	setNamespaceFlag(cmd)
 
 	if err := cmd.RegisterFlagCompletionFunc("build", CompleteDeployBuildType); err != nil {
 		fmt.Println("internal: error while calling RegisterFlagCompletionFunc: ", err)
@@ -163,7 +154,9 @@ func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err
 		config.Registry = ""
 	}
 
-	client := newClient(deployConfigToClientOptions(config))
+	client, done := newClient(ClientConfig{Namespace: config.Namespace, Verbose: config.Verbose},
+		fn.WithRegistry(config.Registry))
+	defer done()
 
 	switch currentBuildType {
 	case fn.BuildTypeLocal, "":

cmd/deploy_test.go

@@ -9,6 +9,7 @@ import (
 	"k8s.io/utils/pointer"
 	fn "knative.dev/kn-plugin-func"
 	"knative.dev/kn-plugin-func/mock"
+	. "knative.dev/kn-plugin-func/testing"
 )
 
 func Test_runDeploy(t *testing.T) {
@@ -109,19 +110,27 @@ created: 2009-11-10 23:00:00`,
 				},
 			}
 			deployer := mock.NewDeployer()
-			defer fromTempDir(t)()
+			defer Fromtemp(t)()
-			cmd := NewDeployCmd(func(opts ClientOptions) *fn.Client {
+			cmd := NewDeployCmd(NewClientFactory(func() *fn.Client {
 				return fn.New(
 					fn.WithPipelinesProvider(pipeline),
-					fn.WithDeployer(deployer),
+					fn.WithDeployer(deployer))
-				)
+			}))
-			})
+			cmd.SetArgs([]string{}) // Do not use test command args
 
+			// TODO: the below viper.SetDefault calls appear to be altering
+			// the default values of flags as a way set various values of flags.
+			// This could perhaps be better achieved by constructing an array
+			// of flag arguments, set via cmd.SetArgs(...).  This would more directly
+			// test the use-case of flag values (as opposed to the indirect proxy
+			// of their defaults), and would avoid the need to call viper.Reset() to
+			// avoid affecting other tests.
 			viper.SetDefault("git-url", tt.gitURL)
 			viper.SetDefault("git-branch", tt.gitBranch)
 			viper.SetDefault("git-dir", tt.gitDir)
 			viper.SetDefault("build", tt.buildType)
 			viper.SetDefault("registry", "docker.io/tigerteam")
+			defer viper.Reset()
 
 			// set test case's func.yaml
 			if err := os.WriteFile("func.yaml", []byte(tt.funcFile), os.ModePerm); err != nil {

cmd/func/main.go

@@ -10,8 +10,7 @@ import (
 	"knative.dev/kn-plugin-func/cmd"
 )
 
-// Statically-populated build metadata set
+// Statically-populated build metadata set by `make build`.
-// by `make build`.
 var date, vers, hash string
 
 func main() {
@@ -20,34 +19,26 @@ func main() {
 
 	sigs := make(chan os.Signal, 1)
 	signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM)
-
 	go func() {
 		<-sigs
 		cancel()
-		// second sigint/sigterm is treated as sigkill
+		<-sigs // second sigint/sigterm is treated as sigkill
-		<-sigs
 		os.Exit(137)
 	}()
 
-	root, err := cmd.NewRootCmd(cmd.RootCommandConfig{
+	cfg := cmd.RootCommandConfig{
 		Name: "func",
+		Version: cmd.Version{
 			Date: date,
-		Version: vers,
+			Vers: vers,
 			Hash: hash,
-	})
+		}}
 
-	if err != nil {
+	if err := cmd.NewRootCmd(cfg).ExecuteContext(ctx); err != nil {
 		fmt.Fprintf(os.Stderr, "Error: %v\n", err)
-		os.Exit(1)
-	}
-
-	if err := root.ExecuteContext(ctx); err != nil {
 		if ctx.Err() != nil {
 			os.Exit(130)
-			return
 		}
-		// Errors are printed to STDERR output and the process exits with code of 1.
-		fmt.Fprintf(os.Stderr, "Error: %v\n", err)
 		os.Exit(1)
 	}
 }

cmd/info.go

@@ -32,10 +32,9 @@ the current directory or from the directory specified with --path.
 `,
 		SuggestFor:        []string{"ifno", "describe", "fino", "get"},
 		ValidArgsFunction: CompleteFunctionList,
-		PreRunE:           bindEnv("namespace", "output", "path"),
+		PreRunE:           bindEnv("output", "path"),
 	}
 
-	setNamespaceFlag(cmd)
 	cmd.Flags().StringP("output", "o", "human", "Output format (human|plain|json|xml|yaml|url) (Env: $FUNC_OUTPUT)")
 	setPathFlag(cmd)
 
@@ -66,10 +65,8 @@ func runInfo(cmd *cobra.Command, args []string, newClient ClientFactory) (err er
 	}
 
 	// Create a client
-	client := newClient(ClientOptions{
+	client, done := newClient(ClientConfig{Namespace: config.Namespace, Verbose: config.Verbose})
-		Namespace: config.Namespace,
+	defer done()
-		Verbose:   config.Verbose,
-	})
 
 	// Get the description
 	d, err := client.Info(cmd.Context(), config.Name, config.Path)

cmd/invoke.go

@@ -99,7 +99,7 @@ EXAMPLES
 
 `,
 		SuggestFor: []string{"emit", "emti", "send", "emit", "exec", "nivoke", "onvoke", "unvoke", "knvoke", "imvoke", "ihvoke", "ibvoke"},
-		PreRunE:    bindEnv("path", "format", "target", "id", "source", "type", "data", "content-type", "file", "confirm", "namespace"),
+		PreRunE:    bindEnv("path", "format", "target", "id", "source", "type", "data", "content-type", "file", "confirm"),
 	}
 
 	// Flags
@@ -113,11 +113,9 @@ EXAMPLES
 	cmd.Flags().StringP("data", "", fn.DefaultInvokeData, "Data to send in the request. (Env: $FUNC_DATA)")
 	cmd.Flags().StringP("file", "", "", "Path to a file to use as data. Overrides --data flag and should be sent with a correct --content-type. (Env: $FUNC_FILE)")
 	cmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all options interactively. (Env: $FUNC_CONFIRM)")
-	setNamespaceFlag(cmd)
 
 	cmd.SetHelpFunc(defaultTemplatedHelp)
 
-	// Run Action
 	cmd.RunE = func(cmd *cobra.Command, args []string) error {
 		return runInvoke(cmd, args, newClient)
 	}
@@ -134,10 +132,8 @@ func runInvoke(cmd *cobra.Command, args []string, newClient ClientFactory) (err
 	}
 
 	// Client instance from env vars, flags, args and user prompts (if --confirm)
-	client := newClient(ClientOptions{
+	client, done := newClient(ClientConfig{Namespace: cfg.Namespace, Verbose: cfg.Verbose})
-		Namespace: cfg.Namespace,
+	defer done()
-		Verbose:   cfg.Verbose,
-	})
 
 	// Message to send the running Function built from parameters gathered
 	// from the user (or defaults)
@@ -196,6 +192,7 @@ func newInvokeConfig(newClient ClientFactory) (cfg invokeConfig, err error) {
 		File:        viper.GetString("file"),
 		Confirm:     viper.GetBool("confirm"),
 		Verbose:     viper.GetBool("verbose"),
+		Namespace:   viper.GetString("namespace"),
 	}
 
 	// If file was passed, read it in as data
@@ -213,10 +210,8 @@ func newInvokeConfig(newClient ClientFactory) (cfg invokeConfig, err error) {
 	}
 
 	// Client instance for use during prompting.
-	client := newClient(ClientOptions{
+	client, done := newClient(ClientConfig{Namespace: cfg.Namespace, Verbose: cfg.Verbose})
-		Namespace: cfg.Namespace,
+	defer done()
-		Verbose:   cfg.Verbose,
-	})
 
 	// If in interactive terminal mode, prompt to modify defaults.
 	if interactiveTerminal() {

cmd/list.go

@@ -35,12 +35,11 @@ Lists all deployed functions in a given namespace.
 {{.Name}} list --all-namespaces --output json
 `,
 		SuggestFor: []string{"ls", "lsit"},
-		PreRunE:    bindEnv("namespace", "output"),
+		PreRunE:    bindEnv("all-namespaces", "output"),
 	}
 
 	cmd.Flags().BoolP("all-namespaces", "A", false, "List functions in all namespaces. If set, the --namespace flag is ignored.")
 	cmd.Flags().StringP("output", "o", "human", "Output format (human|plain|json|xml|yaml) (Env: $FUNC_OUTPUT)")
-	setNamespaceFlag(cmd)
 
 	if err := cmd.RegisterFlagCompletionFunc("output", CompleteOutputFormatList); err != nil {
 		fmt.Println("internal: error while calling RegisterFlagCompletionFunc: ", err)
@@ -62,10 +61,8 @@ func runList(cmd *cobra.Command, _ []string, newClient ClientFactory) (err error
 		return err
 	}
 
-	client := newClient(ClientOptions{
+	client, done := newClient(ClientConfig{Namespace: config.Namespace, Verbose: config.Verbose})
-		Namespace: config.Namespace,
+	defer done()
-		Verbose:   config.Verbose,
-	})
 
 	items, err := client.List(cmd.Context())
 	if err != nil {

cmd/repository.go

@@ -12,29 +12,10 @@ import (
 	fn "knative.dev/kn-plugin-func"
 )
 
-// repositoryClientFn is a function which yields both a client and the final
-// config used to instantiate.
-type repositoryClientFn func([]string) (repositoryConfig, RepositoryClient, error)
-
-// newRepositoryClient is the default repositoryClientFn.
-// It creates a config (which parses flags and environment variables) and uses
-// the config to intantiate a client.  This function is swapped out in tests
-// with one which returns a mock client.
-func newRepositoryClient(args []string) (repositoryConfig, RepositoryClient, error) {
-	cfg, err := newRepositoryConfig(args)
-	if err != nil {
-		return cfg, nil, err
-	}
-	client := repositoryClient{fn.New(
-		fn.WithRepositoriesPath(cfg.RepositoriesPath),
-		fn.WithVerbose(cfg.Verbose))}
-	return cfg, client, nil
-}
-
 // command constructors
 // --------------------
 
-func NewRepositoryCmd(clientFn repositoryClientFn) *cobra.Command {
+func NewRepositoryCmd(newClient ClientFactory) *cobra.Command {
 	cmd := &cobra.Command{
 		Short:   "Manage installed template repositories",
 		Use:     "repository",
@@ -164,30 +145,31 @@ EXAMPLES
 	cmd.SetHelpFunc(defaultTemplatedHelp)
 
 	cmd.RunE = func(cmd *cobra.Command, args []string) error {
-		return runRepository(cmd, args, clientFn)
+		return runRepository(cmd, args, newClient)
 	}
 
-	cmd.AddCommand(NewRepositoryListCmd(newRepositoryClient))
+	cmd.AddCommand(NewRepositoryListCmd(newClient))
-	cmd.AddCommand(NewRepositoryAddCmd(newRepositoryClient))
+	cmd.AddCommand(NewRepositoryAddCmd(newClient))
-	cmd.AddCommand(NewRepositoryRenameCmd(newRepositoryClient))
+	cmd.AddCommand(NewRepositoryRenameCmd(newClient))
-	cmd.AddCommand(NewRepositoryRemoveCmd(newRepositoryClient))
+	cmd.AddCommand(NewRepositoryRemoveCmd(newClient))
 
 	return cmd
 }
 
-func NewRepositoryListCmd(clientFn repositoryClientFn) *cobra.Command {
+func NewRepositoryListCmd(newClient ClientFactory) *cobra.Command {
 	cmd := &cobra.Command{
 		Short: "List repositories",
 		Use:   "list",
 	}
 
 	cmd.RunE = func(_ *cobra.Command, args []string) error {
-		return runRepositoryList(args, clientFn)
+		return runRepositoryList(cmd, args, newClient)
 	}
+
 	return cmd
 }
 
-func NewRepositoryAddCmd(clientFn repositoryClientFn) *cobra.Command {
+func NewRepositoryAddCmd(newClient ClientFactory) *cobra.Command {
 	cmd := &cobra.Command{
 		Short:      "Add a repository",
 		Use:        "add <name> <url>",
@@ -197,13 +179,14 @@ func NewRepositoryAddCmd(clientFn repositoryClientFn) *cobra.Command {
 
 	cmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all options interactively (Env: $FUNC_CONFIRM)")
 
-	cmd.RunE = func(_ *cobra.Command, args []string) error {
+	cmd.RunE = func(cmd *cobra.Command, args []string) error {
-		return runRepositoryAdd(args, clientFn)
+		return runRepositoryAdd(cmd, args, newClient)
 	}
+
 	return cmd
 }
 
-func NewRepositoryRenameCmd(clientFn repositoryClientFn) *cobra.Command {
+func NewRepositoryRenameCmd(newClient ClientFactory) *cobra.Command {
 	cmd := &cobra.Command{
 		Short:   "Rename a repository",
 		Use:     "rename <old> <new>",
@@ -212,13 +195,14 @@ func NewRepositoryRenameCmd(clientFn repositoryClientFn) *cobra.Command {
 
 	cmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all options interactively (Env: $FUNC_CONFIRM)")
 
-	cmd.RunE = func(_ *cobra.Command, args []string) error {
+	cmd.RunE = func(cmd *cobra.Command, args []string) error {
-		return runRepositoryRename(args, clientFn)
+		return runRepositoryRename(cmd, args, newClient)
 	}
+
 	return cmd
 }
 
-func NewRepositoryRemoveCmd(clientFn repositoryClientFn) *cobra.Command {
+func NewRepositoryRemoveCmd(newClient ClientFactory) *cobra.Command {
 	cmd := &cobra.Command{
 		Short:      "Remove a repository",
 		Use:        "remove <name>",
@@ -229,9 +213,10 @@ func NewRepositoryRemoveCmd(clientFn repositoryClientFn) *cobra.Command {
 
 	cmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all options interactively (Env: $FUNC_CONFIRM)")
 
-	cmd.RunE = func(_ *cobra.Command, args []string) error {
+	cmd.RunE = func(cmd *cobra.Command, args []string) error {
-		return runRepositoryRemove(args, clientFn)
+		return runRepositoryRemove(cmd, args, newClient)
 	}
+
 	return cmd
 }
 
@@ -240,10 +225,8 @@ func NewRepositoryRemoveCmd(clientFn repositoryClientFn) *cobra.Command {
 
 // Run
 // (list by default or interactive with -c|--confirm)
-func runRepository(cmd *cobra.Command, args []string, clientFn repositoryClientFn) (err error) {
+func runRepository(cmd *cobra.Command, args []string, newClient ClientFactory) (err error) {
-	// Config
+	cfg, err := newRepositoryConfig(args)
-	// Based on env, flags and (optionally) user pompting.
-	cfg, client, err := clientFn(args)
 	if err != nil {
 		return
 	}
@@ -253,6 +236,7 @@ func runRepository(cmd *cobra.Command, args []string, clientFn repositoryClientF
 		return cmd.Help()
 	}
 
+	// If in interactive mode, the user chan choose which subcommand to invoke
 	// Prompt for action to perform
 	question := &survey.Question{
 		Name: "Action",
@@ -266,34 +250,31 @@ func runRepository(cmd *cobra.Command, args []string, clientFn repositoryClientF
 		return
 	}
 
-	// Passthrough client constructor
-	// We already instantiated a client and a config using clientFn above
-	// (possibly prompting etc.), so the clientFn used for the subcommand
-	// delegation can be effectively a closure around those values so the
-	// user is not re-prompted:
-	c := func([]string) (repositoryConfig, RepositoryClient, error) { return cfg, client, nil }
-
 	// Run the command indicated
 	switch answer.Action {
 	case "list":
-		return runRepositoryList(args, c)
+		return runRepositoryList(cmd, args, newClient)
 	case "add":
-		return runRepositoryAdd(args, c)
+		return runRepositoryAdd(cmd, args, newClient)
 	case "rename":
-		return runRepositoryRename(args, c)
+		return runRepositoryRename(cmd, args, newClient)
 	case "remove":
-		return runRepositoryRemove(args, c)
+		return runRepositoryRemove(cmd, args, newClient)
 	}
 	return fmt.Errorf("invalid action '%v'", answer.Action) // Unreachable
 }
 
 // List
-func runRepositoryList(args []string, clientFn repositoryClientFn) (err error) {
+func runRepositoryList(_ *cobra.Command, args []string, newClient ClientFactory) (err error) {
-	cfg, client, err := clientFn(args)
+	cfg, err := newRepositoryConfig(args)
 	if err != nil {
 		return
 	}
 
+	client, done := newClient(ClientConfig{Verbose: cfg.Verbose},
+		fn.WithRepositoriesPath(cfg.RepositoriesPath))
+	defer done()
+
 	// List all repositories given a client instantiated about config.
 	rr, err := client.Repositories().All()
 	if err != nil {
@@ -313,19 +294,21 @@ func runRepositoryList(args []string, clientFn repositoryClientFn) (err error) {
 }
 
 // Add
-func runRepositoryAdd(args []string, clientFn repositoryClientFn) (err error) {
+func runRepositoryAdd(_ *cobra.Command, args []string, newClient ClientFactory) (err error) {
 	// Supports both composable, discrete CLI commands or prompt-based "config"
 	// by setting the argument values (name and ulr) to value of positional args,
 	// but only requires them if not prompting.  If prompting, those values
 	// become the prompt defaults.
 
-	// Client
+	cfg, err := newRepositoryConfig(args)
-	// (repositories location, verbosity, confirm)
-	cfg, client, err := clientFn(args)
 	if err != nil {
 		return
 	}
 
+	client, done := newClient(ClientConfig{Verbose: cfg.Verbose},
+		fn.WithRepositoriesPath(cfg.RepositoriesPath))
+	defer done()
+
 	// Preconditions
 	// If not confirming/prompting, assert the args were both provided.
 	if len(args) != 2 && !cfg.Confirm {
@@ -398,11 +381,14 @@ func runRepositoryAdd(args []string, clientFn repositoryClientFn) (err error) {
 }
 
 // Rename
-func runRepositoryRename(args []string, clientFn repositoryClientFn) (err error) {
+func runRepositoryRename(_ *cobra.Command, args []string, newClient ClientFactory) (err error) {
-	cfg, client, err := clientFn(args)
+	cfg, err := newRepositoryConfig(args)
 	if err != nil {
 		return
 	}
+	client, done := newClient(ClientConfig{Verbose: cfg.Verbose},
+		fn.WithRepositoriesPath(cfg.RepositoriesPath))
+	defer done()
 
 	// Preconditions
 	if len(args) != 2 && !cfg.Confirm {
@@ -466,11 +452,14 @@ func runRepositoryRename(args []string, clientFn repositoryClientFn) (err error)
 }
 
 // Remove
-func runRepositoryRemove(args []string, clientFn repositoryClientFn) (err error) {
+func runRepositoryRemove(_ *cobra.Command, args []string, newClient ClientFactory) (err error) {
-	cfg, client, err := clientFn(args)
+	cfg, err := newRepositoryConfig(args)
 	if err != nil {
 		return
 	}
+	client, done := newClient(ClientConfig{Verbose: cfg.Verbose},
+		fn.WithRepositoriesPath(cfg.RepositoriesPath))
+	defer done()
 
 	// Preconditions
 	if len(args) != 1 && !cfg.Confirm {
@@ -558,7 +547,7 @@ func runRepositoryRemove(args []string, clientFn repositoryClientFn) (err error)
 
 // Installed repositories
 // All repositories which have been installed (does not include builtin)
-func installedRepositories(client RepositoryClient) ([]string, error) {
+func installedRepositories(client *fn.Client) ([]string, error) {
 	// Client API contract stipulates the list always lists the defeault builtin
 	// repo, and always lists it at index 0
 	repositories, err := client.Repositories().List()
@@ -652,34 +641,3 @@ func (c repositoryConfig) prompt() (repositoryConfig, error) {
 	err := survey.Ask(qs, &c)
 	return c, err
 }
-
-// Type Gymnastics
-// ---------------
-
-// RepositoryClient enumerates the API required of a functions.Client to
-// implement the various repository commands.
-type RepositoryClient interface {
-	Repositories() Repositories
-}
-
-// Repositories enumerates the API required of the object returned from
-// client.Repositories.
-type Repositories interface {
-	All() ([]fn.Repository, error)
-	List() ([]string, error)
-	Add(name, url string) (string, error)
-	Rename(old, new string) error
-	Remove(name string) error
-}
-
-// repositoryClient implements RepositoryClient by embedding
-// a functions.Client and overriding the Repositories accessor
-// to return an interaface type.  This is because an instance
-// of functions.Client can not be directly treated as a RepositoryClient due
-// to the return value of Repositories being a concrete type.
-// This is hopefully not The Way.
-type repositoryClient struct{ *fn.Client }
-
-func (c repositoryClient) Repositories() Repositories {
-	return Repositories(c.Client.Repositories())
-}

cmd/repository_test.go

@@ -6,32 +6,23 @@ import (
 	"strings"
 	"testing"
 
-	"knative.dev/kn-plugin-func/mock"
+	. "knative.dev/kn-plugin-func/testing"
 )
 
 // TestRepository_List ensures that the 'list' subcommand shows the client's
-// set of repositories by name and prints the list as expected.
+// set of repositories by name for builtin repositories, by explicitly
+// setting the repositories path to a new path which includes no others.
 func TestRepository_List(t *testing.T) {
-	var (
+	defer WithEnvVar(t, "XDG_CONFIG_HOME", t.TempDir())()
-		client = mock.NewClient()
+	cmd := NewRepositoryListCmd(NewClient)
-		list   = NewRepositoryListCmd(testRepositoryClientFn(client))
+	cmd.SetArgs([]string{}) // Do not use test command args
-	)
-
-	// Set the repositories path, which will be passed to the client instance
-	// in the form of a config.
-	os.Setenv("FUNC_REPOSITORIES_PATH", "testpath")
 
 	// Execute the command, capturing the output sent to stdout
 	stdout := piped(t)
-	if err := list.Execute(); err != nil {
+	if err := cmd.Execute(); err != nil {
 		t.Fatal(err)
 	}
 
-	// Assert the repository flag setting was preserved during execution
-	if client.RepositoriesPath != "testpath" {
-		t.Fatal("repositories env not passed to client")
-	}
-
 	// Assert the output matches expectd (whitespace trimmed)
 	expect := "default"
 	output := stdout()
@@ -44,18 +35,20 @@ func TestRepository_List(t *testing.T) {
 // arguments, respects the repositories path flag, and the expected name is echoed
 // upon subsequent 'list'.
 func TestRepository_Add(t *testing.T) {
+	defer WithEnvVar(t, "XDG_CONFIG_HOME", t.TempDir())()
 	var (
-		client = mock.NewClient()
+		add    = NewRepositoryAddCmd(NewClient)
-		add    = NewRepositoryAddCmd(testRepositoryClientFn(client))
+		list   = NewRepositoryListCmd(NewClient)
-		list   = NewRepositoryListCmd(testRepositoryClientFn(client))
 		stdout = piped(t)
 	)
-	os.Setenv("FUNC_REPOSITORIES_PATH", "testpath")
+	// Do not use test command args
+	add.SetArgs([]string{})
+	list.SetArgs([]string{})
 
 	// add [flags] <old> <new>
 	add.SetArgs([]string{
 		"newrepo",
-		"https://git.example.com/user/repo",
+		TestRepoURI("repository", t),
 	})
 
 	// Parse flags and args, performing action
@@ -63,11 +56,6 @@ func TestRepository_Add(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// Assert the repositories flag was parsed and provided to client
-	if client.RepositoriesPath != "testpath" {
-		t.Fatal("repositories path not passed to client")
-	}
-
 	// List post-add, capturing output from stdout
 	if err := list.Execute(); err != nil {
 		t.Fatal(err)
@@ -85,17 +73,20 @@ func TestRepository_Add(t *testing.T) {
 // positional arguments, respects the repositories path flag, and the name is
 // reflected as having been reanamed upon subsequent 'list'.
 func TestRepository_Rename(t *testing.T) {
+	defer WithEnvVar(t, "XDG_CONFIG_HOME", t.TempDir())()
 	var (
-		client = mock.NewClient()
+		add    = NewRepositoryAddCmd(NewClient)
-		add    = NewRepositoryAddCmd(testRepositoryClientFn(client))
+		rename = NewRepositoryRenameCmd(NewClient)
-		rename = NewRepositoryRenameCmd(testRepositoryClientFn(client))
+		list   = NewRepositoryListCmd(NewClient)
-		list   = NewRepositoryListCmd(testRepositoryClientFn(client))
 		stdout = piped(t)
 	)
-	os.Setenv("FUNC_REPOSITORIES_PATH", "testpath")
+	// Do not use test command args
+	add.SetArgs([]string{})
+	rename.SetArgs([]string{})
+	list.SetArgs([]string{})
 
 	// add a repo which will be renamed
-	add.SetArgs([]string{"newrepo", "https://git.example.com/user/repo"})
+	add.SetArgs([]string{"newrepo", TestRepoURI("repository", t)})
 	if err := add.Execute(); err != nil {
 		t.Fatal(err)
 	}
@@ -111,11 +102,6 @@ func TestRepository_Rename(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// Assert the repositories flag was parsed and provided to client
-	if client.RepositoriesPath != "testpath" {
-		t.Fatal("repositories path not passed to client")
-	}
-
 	// List post-rename, capturing output from stdout
 	if err := list.Execute(); err != nil {
 		t.Fatal(err)
@@ -133,17 +119,20 @@ func TestRepository_Rename(t *testing.T) {
 // its argument, respects the repositorieis flag, and the entry is removed upon
 // subsequent 'list'.
 func TestRepository_Remove(t *testing.T) {
+	defer WithEnvVar(t, "XDG_CONFIG_HOME", t.TempDir())()
 	var (
-		client = mock.NewClient()
+		add    = NewRepositoryAddCmd(NewClient)
-		add    = NewRepositoryAddCmd(testRepositoryClientFn(client))
+		remove = NewRepositoryRemoveCmd(NewClient)
-		remove = NewRepositoryRemoveCmd(testRepositoryClientFn(client))
+		list   = NewRepositoryListCmd(NewClient)
-		list   = NewRepositoryListCmd(testRepositoryClientFn(client))
 		stdout = piped(t)
 	)
-	os.Setenv("FUNC_REPOSITORIES_PATH", "testpath")
+	// Do not use test command args
+	add.SetArgs([]string{})
+	remove.SetArgs([]string{})
+	list.SetArgs([]string{})
 
 	// add a repo which will be removed
-	add.SetArgs([]string{"newrepo", "https://git.example.com/user/repo"})
+	add.SetArgs([]string{"newrepo", TestRepoURI("repository", t)})
 	if err := add.Execute(); err != nil {
 		t.Fatal(err)
 	}
@@ -158,11 +147,6 @@ func TestRepository_Remove(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// Assert the repositories flag was parsed and provided to client
-	if client.RepositoriesPath != "testpath" {
-		t.Fatal("repositories flag not passed to client")
-	}
-
 	// List post-remove, capturing output from stdout
 	if err := list.Execute(); err != nil {
 		t.Fatal(err)
@@ -179,30 +163,6 @@ func TestRepository_Remove(t *testing.T) {
 // Helpers
 // -------
 
-// testClientFn returns a repositoryClientFn which always returns the provided
-// mock client.  The client may have various function implementations overridden
-// so as to test particular cases, and a config object is created from the
-// flags and environment variables in the same way as the actual commands, with
-// the effective value recorded on the mock as members for test assertions.
-func testRepositoryClientFn(client *mock.Client) repositoryClientFn {
-	c := testRepositoryClient{client} // type gymnastics
-	return func(args []string) (repositoryConfig, RepositoryClient, error) {
-		cfg, err := newRepositoryConfig(args)
-		client.Confirm = cfg.Confirm
-		client.RepositoriesPath = cfg.RepositoriesPath
-		if err != nil {
-			return cfg, c, err
-		}
-		return cfg, c, nil
-	}
-}
-
-type testRepositoryClient struct{ *mock.Client }
-
-func (c testRepositoryClient) Repositories() Repositories {
-	return Repositories(c.Client.Repositories())
-}
-
 // pipe the output of stdout to a buffer whose value is returned
 // from the returned function.  Call pipe() to start piping output
 // to the buffer, call the returned function to access the data in

cmd/root.go

@@ -11,22 +11,19 @@ import (
 	"github.com/spf13/cobra"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"knative.dev/client/pkg/util"
-
 	fn "knative.dev/kn-plugin-func"
 )
 
 type RootCommandConfig struct {
 	Name string // usually `func` or `kn func`
-	Date      string
+	Version
-	Version   string
-	Hash      string
 	NewClient ClientFactory
 }
 
 // NewRootCmd creates the root of the command tree defines the command name, description, globally
 // available flags, etc.  It has no action of its own, such that running the
 // resultant binary with no arguments prints the help/usage text.
-func NewRootCmd(config RootCommandConfig) (*cobra.Command, error) {
+func NewRootCmd(config RootCommandConfig) *cobra.Command {
 	cmd := &cobra.Command{
 		// Use must be set to exactly config.Name, as this field is overloaded to
 		// be used in subcommand help text as the command with possible prefix:
@@ -66,36 +63,29 @@ EXAMPLES
 
 	// Flags
 	// persistent flags are available to all subcommands implicitly
-	cmd.PersistentFlags().BoolP("verbose", "v", false, "print verbose logs ($FUNC_VERBOSE)")
+	// Note they are bound immediately here as opposed to other subcommands
-	err := viper.BindPFlag("verbose", cmd.PersistentFlags().Lookup("verbose"))
+	// because this root command is not actually executed during tests, and
-	if err != nil {
+	// therefore PreRunE and other event-based listeners are not invoked.
-		return cmd, err
+	cmd.PersistentFlags().BoolP("verbose", "v", false, "Print verbose logs ($FUNC_VERBOSE)")
+	if err := viper.BindPFlag("verbose", cmd.PersistentFlags().Lookup("verbose")); err != nil {
+		fmt.Fprintf(os.Stderr, "error binding flag: %v\n", err)
+	}
+	cmd.PersistentFlags().StringP("namespace", "n", "", "The namespace on the cluster used for remote commands. By default, the namespace func.yaml is used or the currently active namespace if not set in the configuration. (Env: $FUNC_NAMESPACE)")
+	if err := viper.BindPFlag("namespace", cmd.PersistentFlags().Lookup("namespace")); err != nil {
+		fmt.Fprintf(os.Stderr, "error binding flag: %v\n", err)
 	}
 
 	// Version
-	// Gather the statically-set version values (populated durin build) into
+	cmd.Version = config.Version.String()
-	// a version structure used by both --version flag and the `version` subcmd
-	// Overrides the --version template to match the output format from the
-	// version subcommand: nothing but the version.
-	version := Version{
-		Date: config.Date,
-		Vers: config.Version,
-		Hash: config.Hash,
-	}
-	cmd.Version = version.String()
 	cmd.SetVersionTemplate(`{{printf "%s\n" .Version}}`)
 
+	// Client
+	// Use the provided ClientFactory or default to NewClient
 	newClient := config.NewClient
-
 	if newClient == nil {
-		var cleanUp func() error
+		newClient = NewClient
-		newClient, cleanUp = NewDefaultClientFactory()
-		cmd.PersistentPostRunE = func(cmd *cobra.Command, args []string) error {
-			return cleanUp()
-		}
 	}
 
-	cmd.AddCommand(NewVersionCmd(version))
 	cmd.AddCommand(NewCreateCmd(newClient))
 	cmd.AddCommand(NewConfigCmd())
 	cmd.AddCommand(NewBuildCmd(newClient))
@@ -104,18 +94,19 @@ EXAMPLES
 	cmd.AddCommand(NewInfoCmd(newClient))
 	cmd.AddCommand(NewListCmd(newClient))
 	cmd.AddCommand(NewInvokeCmd(newClient))
-	cmd.AddCommand(NewRepositoryCmd(newRepositoryClient))
+	cmd.AddCommand(NewRepositoryCmd(newClient))
-	cmd.AddCommand(NewRunCmd(newRunClient))
+	cmd.AddCommand(NewRunCmd(newClient))
 	cmd.AddCommand(NewCompletionCmd())
+	cmd.AddCommand(NewVersionCmd(config.Version))
 
 	// Help
 	// Overridden to process the help text as a template and have
 	// access to the provided Client instance.
 	cmd.SetHelpFunc(func(cmd *cobra.Command, args []string) {
-		runRootHelp(cmd, args, version)
+		runRootHelp(cmd, args, config.Version)
 	})
 
-	return cmd, nil
+	return cmd
 
 	// NOTE Default Action
 	// No default action is provided triggering the default of displaying the help
@@ -342,11 +333,6 @@ func setPathFlag(cmd *cobra.Command) {
 	cmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)")
 }
 
-// setNamespaceFlag ensures common text/wording when the --namespace flag is used
-func setNamespaceFlag(cmd *cobra.Command) {
-	cmd.Flags().StringP("namespace", "n", "", "The namespace on the cluster. By default, the namespace in func.yaml is used or the currently active namespace if not set in the configuration. (Env: $FUNC_NAMESPACE)")
-}
-
 type Version struct {
 	// Date of compilation
 	Date string

cmd/root_test.go

@@ -11,8 +11,58 @@ import (
 	"knative.dev/client/pkg/util"
 
 	fn "knative.dev/kn-plugin-func"
+	. "knative.dev/kn-plugin-func/testing"
 )
 
+func TestRoot_PersistentFlags(t *testing.T) {
+	tests := []struct {
+		name          string
+		args          []string
+		skipNamespace bool
+	}{
+		{
+			name: "provided as root flags",
+			args: []string{"--verbose", "--namespace=namespace", "list"},
+		},
+		{
+			name: "provided as sub-command flags",
+			args: []string{"list", "--verbose", "--namespace=namespace"},
+		},
+		{
+			name:          "provided as sub-sub-command flags",
+			args:          []string{"repositories", "list", "--verbose"},
+			skipNamespace: true, // NOTE: no sub-sub commands yet use namespace, so it is not checked.
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			defer Fromtemp(t)()                       // from a temp dir, deferred cleanup
+			cmd := NewCreateCmd(NewClient)            // Create a new Function
+			cmd.SetArgs([]string{"--language", "go"}) // providing language (the only required param)
+			if err := cmd.Execute(); err != nil {     // fail on any errors
+				t.Fatal(err)
+			}
+
+			// Assert the persistent variables were propagated to the Client constructor
+			// when the command is actually invoked.
+			cmd = NewRootCmd(RootCommandConfig{NewClient: func(cfg ClientConfig, _ ...fn.Option) (*fn.Client, func()) {
+				if cfg.Namespace != "namespace" && !tt.skipNamespace {
+					t.Fatal("namespace not propagated")
+				}
+				if cfg.Verbose != true {
+					t.Fatal("verbose not propagated")
+				}
+				return fn.New(), func() {}
+			}})
+			cmd.SetArgs(tt.args)
+			if err := cmd.Execute(); err != nil {
+				t.Fatal(err)
+			}
+		})
+	}
+}
+
 func TestRoot_mergeEnvMaps(t *testing.T) {
 
 	a := "A"
@@ -121,31 +171,35 @@ func TestRoot_mergeEnvMaps(t *testing.T) {
 	}
 }
 
-func TestRoot_CMDParameterized(t *testing.T) {
+// TestRoot_CommandNameParameterized confirmst that the command name, as
+// printed in help text, is parameterized based on the constructor parameters
+// of the root command.  This allows, for example, to have help text correct
+// when both embedded as a plugin or standalone.
+func TestRoot_CommandNameParameterized(t *testing.T) {
 	expectedSynopsis := "%v [-v|--verbose] <command> [args]"
 
 	tests := []string{
-		"func",
+		"func",    // standalone
-		"kn func",
+		"kn func", // kn plugin
 	}
 
-	for _, test := range tests {
+	for _, testName := range tests {
 		var (
-			cfg    = RootCommandConfig{Name: test}
+			cmd = NewRootCmd(RootCommandConfig{Name: testName})
-			cmd, _ = NewRootCmd(cfg)
 			out = strings.Builder{}
 		)
+		cmd.SetArgs([]string{}) // Do not use test command args
 		cmd.SetOut(&out)
 		if err := cmd.Help(); err != nil {
 			t.Fatal(err)
 		}
-		if cmd.Use != cfg.Name {
+		if cmd.Use != testName {
-			t.Fatalf("expected command Use '%v', got '%v'", cfg.Name, cmd.Use)
+			t.Fatalf("expected command Use '%v', got '%v'", testName, cmd.Use)
 		}
-		if !strings.Contains(out.String(), fmt.Sprintf(expectedSynopsis, cfg.Name)) {
+		if !strings.Contains(out.String(), fmt.Sprintf(expectedSynopsis, testName)) {
-			t.Logf("Testing '%v'\n", test)
+			t.Logf("Testing '%v'\n", testName)
 			t.Log(out.String())
-			t.Fatalf("Help text does not include substituted name '%v'", cfg.Name)
+			t.Fatalf("Help text does not include substituted name '%v'", testName)
 		}
 	}
 }
@@ -171,11 +225,6 @@ func TestVerbose(t *testing.T) {
 			args: []string{"--verbose", "version"},
 			want: "v0.42.0-cafe-1970-01-01\n",
 		},
-		{
-			name: "version not as sub-command",
-			args: []string{"--version"},
-			want: "v0.42.0\n",
-		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -183,20 +232,17 @@ func TestVerbose(t *testing.T) {
 
 			var out bytes.Buffer
 
-			cmd, err := NewRootCmd(RootCommandConfig{
+			cmd := NewRootCmd(RootCommandConfig{
 				Name: "func",
+				Version: Version{
 					Date: "1970-01-01",
-				Version: "v0.42.0",
+					Vers: "v0.42.0",
 					Hash: "cafe",
-			})
+				}})
-			if err != nil {
-				t.Fatal(err)
-			}
 
 			cmd.SetArgs(tt.args)
 			cmd.SetOut(&out)
-			err = cmd.Execute()
+			if err := cmd.Execute(); err != nil {
-			if err != nil {
 				t.Fatal(err)
 			}
 
@@ -205,5 +251,4 @@ func TestVerbose(t *testing.T) {
 			}
 		})
 	}
-
 }

cmd/run.go

@@ -10,30 +10,9 @@ import (
 	"knative.dev/client/pkg/util"
 
 	fn "knative.dev/kn-plugin-func"
-	"knative.dev/kn-plugin-func/buildpacks"
-	"knative.dev/kn-plugin-func/docker"
-	"knative.dev/kn-plugin-func/progress"
 )
 
-func newRunClient(cfg runConfig) *fn.Client {
+func NewRunCmd(newClient ClientFactory) *cobra.Command {
-	bc := newBuildConfig()
-	runner := docker.NewRunner(cfg.Verbose)
-
-	// builder fields
-	builder := buildpacks.NewBuilder(cfg.Verbose)
-	listener := progress.New(cfg.Verbose)
-	return fn.New(
-		fn.WithBuilder(builder),
-		fn.WithProgressListener(listener),
-		fn.WithRegistry(bc.Registry),
-		fn.WithRunner(runner),
-		fn.WithVerbose(cfg.Verbose))
-}
-
-type runClientFn func(runConfig) *fn.Client
-
-func NewRunCmd(clientFn runClientFn) *cobra.Command {
-
 	cmd := &cobra.Command{
 		Use:   "run",
 		Short: "Run the function locally",
@@ -50,26 +29,27 @@ specified by --path flag. The function must already have been built with the 'bu
 {{.Name}} run
 `,
 		SuggestFor: []string{"rnu"},
-		PreRunE:    bindEnv("build", "path"),
+		PreRunE:    bindEnv("build", "path", "registry"),
 	}
 
 	cmd.Flags().StringArrayP("env", "e", []string{},
 		"Environment variable to set in the form NAME=VALUE. "+
 			"You may provide this flag multiple times for setting multiple environment variables. "+
 			"To unset, specify the environment variable name followed by a \"-\" (e.g., NAME-).")
-	setPathFlag(cmd)
 	cmd.Flags().BoolP("build", "b", false, "Build the function only if the function has not been built before")
+	cmd.Flags().StringP("registry", "r", GetDefaultRegistry(), "Registry + namespace part of the image if building, ex 'quay.io/myuser' (Env: $FUNC_REGISTRY)")
+	setPathFlag(cmd)
 
 	cmd.SetHelpFunc(defaultTemplatedHelp)
 
 	cmd.RunE = func(cmd *cobra.Command, args []string) error {
-		return runRun(cmd, args, clientFn)
+		return runRun(cmd, args, newClient)
 	}
 
 	return cmd
 }
 
-func runRun(cmd *cobra.Command, args []string, clientFn runClientFn) (err error) {
+func runRun(cmd *cobra.Command, args []string, newClient ClientFactory) (err error) {
 	config, err := newRunConfig(cmd)
 	if err != nil {
 		return
@@ -95,8 +75,11 @@ func runRun(cmd *cobra.Command, args []string, clientFn runClientFn) (err error)
 		return fmt.Errorf("the given path '%v' does not contain an initialized function", config.Path)
 	}
 
-	// Client for use running (and potentially building)
+	// Client for use running (and potentially building), using the config
-	client := clientFn(config)
+	// gathered plus any additional option overrieds (such as for providing
+	// mocks when testing for builder and runner)
+	client, done := newClient(ClientConfig{Verbose: config.Verbose}, fn.WithRegistry(config.Registry))
+	defer done()
 
 	// Build if not built and --build
 	if config.Build && !function.Built() {
@@ -141,6 +124,9 @@ type runConfig struct {
 
 	// Perform build if function hasn't been built yet
 	Build bool
+
+	// Registry for the build tag if building
+	Registry string
 }
 
 func newRunConfig(cmd *cobra.Command) (c runConfig, err error) {
@@ -153,6 +139,7 @@ func newRunConfig(cmd *cobra.Command) (c runConfig, err error) {
 		Build:       viper.GetBool("build"),
 		Path:        viper.GetString("path"),
 		Verbose:     viper.GetBool("verbose"), // defined on root
+		Registry:    viper.GetString("registry"),
 		EnvToUpdate: envToUpdate,
 		EnvToRemove: envToRemove,
 	}, nil

cmd/run_test.go

@@ -9,10 +9,10 @@ import (
 	"github.com/ory/viper"
 	fn "knative.dev/kn-plugin-func"
 	"knative.dev/kn-plugin-func/mock"
+	. "knative.dev/kn-plugin-func/testing"
 )
 
 func TestRun_Run(t *testing.T) {
-
 	tests := []struct {
 		name         string // name of the test
 		desc         string // description of the test
@@ -73,7 +73,7 @@ created: 2009-11-10 23:00:00`,
 	for _, tt := range tests {
 		// run as a sub-test
 		t.Run(tt.name, func(t *testing.T) {
-			defer fromTempDir(t)()
+			defer Fromtemp(t)()
 
 			runner := mock.NewRunner()
 			if tt.runError != nil {
@@ -88,13 +88,14 @@ created: 2009-11-10 23:00:00`,
 			// using a command whose client will be populated with mock
 			// builder and mock runner, each of which may be set to error if the
 			// test has an error defined.
-			cmd := NewRunCmd(func(rc runConfig) *fn.Client {
+			cmd := NewRunCmd(NewClientFactory(func() *fn.Client {
 				return fn.New(
 					fn.WithRunner(runner),
 					fn.WithBuilder(builder),
 					fn.WithRegistry("ghcr.com/reg"),
 				)
-			})
+			}))
+			cmd.SetArgs([]string{}) // Do not use test command args
 
 			// set test case's build
 			viper.SetDefault("build", tt.buildFlag)

docker/pusher.go

@@ -78,12 +78,18 @@ func WithPusherDockerClientFactory(dockerClientFactory PusherDockerClientFactory
 	}
 }
 
+func WithVerbose(verbose bool) Opt {
+	return func(pusher *Pusher) {
+		pusher.verbose = verbose
+	}
+}
+
 func EmptyCredentialsProvider(ctx context.Context, registry string) (Credentials, error) {
 	return Credentials{}, nil
 }
 
 // NewPusher creates an instance of a docker-based image pusher.
-func NewPusher(verbose bool, opts ...Opt) *Pusher {
+func NewPusher(opts ...Opt) *Pusher {
 	result := &Pusher{
 		credentialsProvider: EmptyCredentialsProvider,
 		progressListener:    &fn.NoopProgressListener{},
@@ -92,7 +98,6 @@ func NewPusher(verbose bool, opts ...Opt) *Pusher {
 			c, _, err := NewClient(client.DefaultDockerHost)
 			return c, err
 		},
-		verbose: verbose,
 	}
 	for _, opt := range opts {
 		opt(result)

docker/pusher_test.go

@@ -105,9 +105,10 @@ func TestDaemonPush(t *testing.T) {
 	dockerClientFactory := func() (docker.PusherDockerClient, error) {
 		return dockerClient, nil
 	}
-	pusher := docker.NewPusher(false,
+	pusher := docker.NewPusher(
 		docker.WithCredentialsProvider(testCredProvider),
-		docker.WithPusherDockerClientFactory(dockerClientFactory))
+		docker.WithPusherDockerClientFactory(dockerClientFactory),
+	)
 
 	f := fn.Function{
 		Image: functionImageLocal,
@@ -182,10 +183,11 @@ func TestNonDaemonPush(t *testing.T) {
 		return dockerClient, nil
 	}
 
-	pusher := docker.NewPusher(false,
+	pusher := docker.NewPusher(
 		docker.WithTransport(transport),
 		docker.WithCredentialsProvider(testCredProvider),
-		docker.WithPusherDockerClientFactory(dockerClientFactory))
+		docker.WithPusherDockerClientFactory(dockerClientFactory),
+	)
 
 	f := fn.Function{
 		Image: functionImageRemote,

function_labels_unit_test.go

@@ -4,8 +4,9 @@
 package function
 
 import (
-	"os"
 	"testing"
+
+	. "knative.dev/kn-plugin-func/testing"
 )
 
 func Test_validateLabels(t *testing.T) {
@@ -28,10 +29,10 @@ func Test_validateLabels(t *testing.T) {
 	valueLocalEnvIncorrect2 := "{{ MY_ENV }}"
 	valueLocalEnvIncorrect3 := "{{env:MY_ENV}}foo"
 
-	os.Setenv("BAD_EXAMPLE", ":invalid")
+	defer WithEnvVar(t, "BAD_EXAMPLE", ":invalid")()
 	valueLocalEnvIncorrect4 := "{{env:BAD_EXAMPLE}}"
 
-	os.Setenv("GOOD_EXAMPLE", "valid")
+	defer WithEnvVar(t, "GOOD_EXAMPLE", "valid")()
 	valueLocalEnv4 := "{{env:GOOD_EXAMPLE}}"
 
 	tests := []struct {

pipelines/tekton/pipeplines_provider.go

@@ -53,8 +53,14 @@ func WithCredentialsProvider(credentialsProvider docker.CredentialsProvider) Opt
 	}
 }
 
-func NewPipelinesProvider(verbose bool, opts ...Opt) *PipelinesProvider {
+func WithVerbose(verbose bool) Opt {
-	pp := &PipelinesProvider{verbose: verbose}
+	return func(pp *PipelinesProvider) {
+		pp.verbose = verbose
+	}
+}
+
+func NewPipelinesProvider(opts ...Opt) *PipelinesProvider {
+	pp := &PipelinesProvider{}
 
 	for _, opt := range opts {
 		opt(pp)

plugin/plugin.go

@@ -35,18 +35,17 @@ func (f *funcPlugin) Execute(args []string) error {
 		cancel()
 	}()
 
-	rootConfig := cmd.RootCommandConfig{
+	version := cmd.Version{}
-		Name: "kn func",
-	}
 
 	info, _ := debug.ReadBuildInfo()
 	for _, dep := range info.Deps {
 		if strings.Contains(dep.Path, "knative.dev/kn-plugin-func") {
-			rootConfig.Version, rootConfig.Hash = dep.Version, dep.Sum
+			version.Vers = dep.Version
+			version.Hash = dep.Sum
 		}
 	}
 
-	rootCmd, _ := cmd.NewRootCmd(rootConfig)
+	rootCmd := cmd.NewRootCmd(cmd.RootCommandConfig{Name: "kn func", Version: version})
 
 	oldArgs := os.Args
 	defer (func() {

testing/testing.go

@@ -91,9 +91,18 @@ func Mktemp(t *testing.T) (string, func()) {
 	}
 }
 
+// Fromtemp is like Mktemp, but does not bother returing the temp path.
+func Fromtemp(t *testing.T) func() {
+	_, done := Mktemp(t)
+	return done
+}
+
 // tempdir creates a new temporary directory and returns its path.
 // errors fail the current test.
 func tempdir(t *testing.T) string {
+	// NOTE: Not using t.TempDir() because it is sometimes helpful during
+	// debugging to skip running the returned deferred cleanup function
+	// and manually inspect the contents of the test's temp directory.
 	d, err := ioutil.TempDir("", "dir")
 	if err != nil {
 		t.Fatal(err)
@@ -139,6 +148,7 @@ func TestRepoURI(name string, t *testing.T) string {
 
 // WithEnvVar sets an environment variable
 // and returns deferrable function that restores previous value of the environment variable.
+// TODO: replace with t.Setenv when we upgrade to go.1.17
 func WithEnvVar(t *testing.T, name, value string) func() {
 	t.Helper()
 	oldDh, hadDh := os.LookupEnv(name)