refactor: prepare for e2e tests (#870)

This PR adds refactors that are necessary for the e2e tests.
Changes:
- Subcommand initialization has been moved to `internal/cli/root.go`
- `output.Table`s now accept writers as output. This is needed to catch
command output. If we only catch stdout we can only run one command at
once or there will be interference.

cmd/hcloud/main.go

@@ -5,28 +5,6 @@ import (
 	"os"
 
 	"github.com/hetznercloud/cli/internal/cli"
-	"github.com/hetznercloud/cli/internal/cmd/all"
-	"github.com/hetznercloud/cli/internal/cmd/certificate"
-	"github.com/hetznercloud/cli/internal/cmd/completion"
-	configCmd "github.com/hetznercloud/cli/internal/cmd/config"
-	"github.com/hetznercloud/cli/internal/cmd/context"
-	"github.com/hetznercloud/cli/internal/cmd/datacenter"
-	"github.com/hetznercloud/cli/internal/cmd/firewall"
-	"github.com/hetznercloud/cli/internal/cmd/floatingip"
-	"github.com/hetznercloud/cli/internal/cmd/image"
-	"github.com/hetznercloud/cli/internal/cmd/iso"
-	"github.com/hetznercloud/cli/internal/cmd/loadbalancer"
-	"github.com/hetznercloud/cli/internal/cmd/loadbalancertype"
-	"github.com/hetznercloud/cli/internal/cmd/location"
-	"github.com/hetznercloud/cli/internal/cmd/network"
-	"github.com/hetznercloud/cli/internal/cmd/placementgroup"
-	"github.com/hetznercloud/cli/internal/cmd/primaryip"
-	"github.com/hetznercloud/cli/internal/cmd/server"
-	"github.com/hetznercloud/cli/internal/cmd/servertype"
-	"github.com/hetznercloud/cli/internal/cmd/sshkey"
-	"github.com/hetznercloud/cli/internal/cmd/util"
-	"github.com/hetznercloud/cli/internal/cmd/version"
-	"github.com/hetznercloud/cli/internal/cmd/volume"
 	"github.com/hetznercloud/cli/internal/state"
 	"github.com/hetznercloud/cli/internal/state/config"
 )
@@ -50,33 +28,6 @@ func main() {
 
 	rootCommand := cli.NewRootCommand(s)
 
-	util.AddGroup(rootCommand, "resource", "Resources",
-		all.NewCommand(s),
-		floatingip.NewCommand(s),
-		image.NewCommand(s),
-		server.NewCommand(s),
-		sshkey.NewCommand(s),
-		servertype.NewCommand(s),
-		datacenter.NewCommand(s),
-		location.NewCommand(s),
-		iso.NewCommand(s),
-		volume.NewCommand(s),
-		network.NewCommand(s),
-		loadbalancer.NewCommand(s),
-		loadbalancertype.NewCommand(s),
-		certificate.NewCommand(s),
-		firewall.NewCommand(s),
-		placementgroup.NewCommand(s),
-		primaryip.NewCommand(s),
-	)
-
-	rootCommand.AddCommand(
-		version.NewCommand(s),
-		completion.NewCommand(s),
-		context.NewCommand(s),
-		configCmd.NewCommand(s),
-	)
-
 	if err := rootCommand.Execute(); err != nil {
 		log.Fatalln(err)
 	}

internal/cli/root.go

@@ -1,10 +1,32 @@
 package cli
 
 import (
-	"os"
+	"io"
 
 	"github.com/spf13/cobra"
 
+	"github.com/hetznercloud/cli/internal/cmd/all"
+	"github.com/hetznercloud/cli/internal/cmd/certificate"
+	"github.com/hetznercloud/cli/internal/cmd/completion"
+	configCmd "github.com/hetznercloud/cli/internal/cmd/config"
+	"github.com/hetznercloud/cli/internal/cmd/context"
+	"github.com/hetznercloud/cli/internal/cmd/datacenter"
+	"github.com/hetznercloud/cli/internal/cmd/firewall"
+	"github.com/hetznercloud/cli/internal/cmd/floatingip"
+	"github.com/hetznercloud/cli/internal/cmd/image"
+	"github.com/hetznercloud/cli/internal/cmd/iso"
+	"github.com/hetznercloud/cli/internal/cmd/loadbalancer"
+	"github.com/hetznercloud/cli/internal/cmd/loadbalancertype"
+	"github.com/hetznercloud/cli/internal/cmd/location"
+	"github.com/hetznercloud/cli/internal/cmd/network"
+	"github.com/hetznercloud/cli/internal/cmd/placementgroup"
+	"github.com/hetznercloud/cli/internal/cmd/primaryip"
+	"github.com/hetznercloud/cli/internal/cmd/server"
+	"github.com/hetznercloud/cli/internal/cmd/servertype"
+	"github.com/hetznercloud/cli/internal/cmd/sshkey"
+	"github.com/hetznercloud/cli/internal/cmd/util"
+	"github.com/hetznercloud/cli/internal/cmd/version"
+	"github.com/hetznercloud/cli/internal/cmd/volume"
 	"github.com/hetznercloud/cli/internal/state"
 	"github.com/hetznercloud/cli/internal/state/config"
 )
@@ -20,6 +42,33 @@ func NewRootCommand(s state.State) *cobra.Command {
 		DisableFlagsInUseLine: true,
 	}
 
+	util.AddGroup(cmd, "resource", "Resources",
+		all.NewCommand(s),
+		floatingip.NewCommand(s),
+		image.NewCommand(s),
+		server.NewCommand(s),
+		sshkey.NewCommand(s),
+		servertype.NewCommand(s),
+		datacenter.NewCommand(s),
+		location.NewCommand(s),
+		iso.NewCommand(s),
+		volume.NewCommand(s),
+		network.NewCommand(s),
+		loadbalancer.NewCommand(s),
+		loadbalancertype.NewCommand(s),
+		certificate.NewCommand(s),
+		firewall.NewCommand(s),
+		placementgroup.NewCommand(s),
+		primaryip.NewCommand(s),
+	)
+
+	cmd.AddCommand(
+		version.NewCommand(s),
+		completion.NewCommand(s),
+		context.NewCommand(s),
+		configCmd.NewCommand(s),
+	)
+
 	cmd.PersistentFlags().AddFlagSet(s.Config().FlagSet())
 
 	for _, opt := range config.Options {
@@ -35,17 +84,15 @@ func NewRootCommand(s state.State) *cobra.Command {
 	}
 
 	cmd.PersistentPreRunE = func(cmd *cobra.Command, _ []string) error {
-		var err error
-		out := os.Stdout
+		out := cmd.OutOrStdout()
 		quiet, err := config.OptionQuiet.Get(s.Config())
 		if err != nil {
 			return err
 		}
 		if quiet {
-			out, err = os.Open(os.DevNull)
-			if err != nil {
-				return err
-			}
+			// We save the original output in cmd.errWriter so that we can still use it if we need it later.
+			cmd.SetErr(out)
+			out = io.Discard
 		}
 		cmd.SetOut(out)
 		return nil

internal/cmd/all/list.go

@@ -153,14 +153,15 @@ Listed resources are:
 				schema[lc.JSONKeyGetByName] = lc.Schema(resources[i])
 			}
 			if outOpts.IsSet("json") {
-				return util.DescribeJSON(schema)
+				return util.DescribeJSON(cmd.OutOrStdout(), schema)
 			}
-			return util.DescribeYAML(schema)
+			return util.DescribeYAML(cmd.OutOrStdout(), schema)
 		}
 
 		for i, lc := range cmds {
 			cols := lc.DefaultColumns
-			table := lc.OutputTable(s.Client())
+			table := output.NewTable(cmd.OutOrStdout())
+			lc.OutputTable(table, s.Client())
 			table.WriteHeader(cols)
 
 			if len(resources[i]) == 0 {

internal/cmd/base/create.go

@@ -48,9 +48,16 @@ func (cc *CreateCmd) CobraCommand(s state.State) *cobra.Command {
 			return err
 		}
 
+		schemaOut := cmd.OutOrStdout()
 		isSchema := outputFlags.IsSet("json") || outputFlags.IsSet("yaml")
-		if isSchema && !quiet {
-			cmd.SetOut(os.Stderr)
+		if isSchema {
+			if quiet {
+				// If we are in quiet mode, we saved the original output in cmd.errWriter. We can now restore it.
+				schemaOut = cmd.ErrOrStderr()
+			} else {
+				// We don't want anything other than the schema in stdout, so we set the default to stderr
+				cmd.SetOut(os.Stderr)
+			}
 		}
 
 		resource, schema, err := cc.Run(s, cmd, args)
@@ -60,9 +67,9 @@ func (cc *CreateCmd) CobraCommand(s state.State) *cobra.Command {
 
 		if isSchema {
 			if outputFlags.IsSet("json") {
-				return util.DescribeJSON(schema)
+				return util.DescribeJSON(schemaOut, schema)
 			}
-			return util.DescribeYAML(schema)
+			return util.DescribeYAML(schemaOut, schema)
 		} else if cc.PrintResource != nil && resource != nil {
 			cc.PrintResource(s, cmd, resource)
 		}

internal/cmd/base/describe.go

@@ -60,9 +60,16 @@ func (dc *DescribeCmd) Run(s state.State, cmd *cobra.Command, args []string) err
 		return err
 	}
 
+	schemaOut := cmd.OutOrStdout()
 	isSchema := outputFlags.IsSet("json") || outputFlags.IsSet("yaml")
-	if isSchema && !quiet {
-		cmd.SetOut(os.Stderr)
+	if isSchema {
+		if quiet {
+			// If we are in quiet mode, we saved the original output in cmd.errWriter. We can now restore it.
+			schemaOut = cmd.ErrOrStderr()
+		} else {
+			// We don't want anything other than the schema in stdout, so we set the default to stderr
+			cmd.SetOut(os.Stderr)
+		}
 	}
 
 	idOrName := args[0]
@@ -79,11 +86,11 @@ func (dc *DescribeCmd) Run(s state.State, cmd *cobra.Command, args []string) err
 
 	switch {
 	case outputFlags.IsSet("json"):
-		return util.DescribeJSON(schema)
+		return util.DescribeJSON(schemaOut, schema)
 	case outputFlags.IsSet("yaml"):
-		return util.DescribeYAML(schema)
+		return util.DescribeYAML(schemaOut, schema)
 	case outputFlags.IsSet("format"):
-		return util.DescribeFormat(resource, outputFlags["format"][0])
+		return util.DescribeFormat(schemaOut, resource, outputFlags["format"][0])
 	default:
 		return dc.PrintText(s, cmd, resource)
 	}

internal/cmd/base/list.go

@@ -2,6 +2,7 @@ package base
 
 import (
 	"fmt"
+	"io"
 	"os"
 
 	"github.com/spf13/cobra"
@@ -23,13 +24,15 @@ type ListCmd struct {
 	DefaultColumns     []string
 	Fetch              func(state.State, *pflag.FlagSet, hcloud.ListOpts, []string) ([]interface{}, error)
 	AdditionalFlags    func(*cobra.Command)
-	OutputTable        func(client hcapi2.Client) *output.Table
+	OutputTable        func(t *output.Table, client hcapi2.Client)
 	Schema             func([]interface{}) interface{}
 }
 
 // CobraCommand creates a command that can be registered with cobra.
 func (lc *ListCmd) CobraCommand(s state.State) *cobra.Command {
-	outputColumns := lc.OutputTable(s.Client()).Columns()
+	t := output.NewTable(io.Discard)
+	lc.OutputTable(t, s.Client())
+	outputColumns := t.Columns()
 
 	cmd := &cobra.Command{
 		Use:   "list [options]",
@@ -59,10 +62,9 @@ func (lc *ListCmd) CobraCommand(s state.State) *cobra.Command {
 func (lc *ListCmd) Run(s state.State, cmd *cobra.Command) error {
 	outOpts := output.FlagsForCommand(cmd)
 
-	labelSelector, _ := cmd.Flags().GetString("selector")
-	listOpts := hcloud.ListOpts{
-		LabelSelector: labelSelector,
-		PerPage:       50,
+	quiet, err := config.OptionQuiet.Get(s.Config())
+	if err != nil {
+		return err
 	}
 
 	var sorts []string
@@ -80,32 +82,44 @@ func (lc *ListCmd) Run(s state.State, cmd *cobra.Command) error {
 		}
 	}
 
+	labelSelector, _ := cmd.Flags().GetString("selector")
+	listOpts := hcloud.ListOpts{
+		LabelSelector: labelSelector,
+		PerPage:       50,
+	}
+
 	resources, err := lc.Fetch(s, cmd.Flags(), listOpts, sorts)
 	if err != nil {
 		return err
 	}
 
-	if outOpts.IsSet("json") || outOpts.IsSet("yaml") {
+	out := cmd.OutOrStdout()
+	if quiet {
+		// If we are in quiet mode, we saved the original output in cmd.errWriter. We can now restore it.
+		out = cmd.ErrOrStderr()
+	}
+
+	isSchema := outOpts.IsSet("json") || outOpts.IsSet("yaml")
+	if isSchema {
 		schema := lc.Schema(resources)
 		if outOpts.IsSet("json") {
-			return util.DescribeJSON(schema)
+			return util.DescribeJSON(out, schema)
 		}
-		return util.DescribeYAML(schema)
+		return util.DescribeYAML(out, schema)
 	}
 
 	cols := lc.DefaultColumns
 	if outOpts.IsSet("columns") {
 		cols = outOpts["columns"]
 	}
 
-	table := lc.OutputTable(s.Client())
+	t := output.NewTable(out)
+	lc.OutputTable(t, s.Client())
 	if !outOpts.IsSet("noheader") {
-		table.WriteHeader(cols)
+		t.WriteHeader(cols)
 	}
 	for _, resource := range resources {
-		table.Write(cols, resource)
+		t.Write(cols, resource)
 	}
-	table.Flush()
-
-	return nil
+	return t.Flush()
 }

internal/cmd/base/list_test.go

@@ -25,8 +25,8 @@ var fakeListCmd = &base.ListCmd{
 		return i
 	},
 
-	OutputTable: func(hcapi2.Client) *output.Table {
-		return output.NewTable().
+	OutputTable: func(t *output.Table, _ hcapi2.Client) {
+		t.
 			AddAllowedFields(hcloud.Firewall{}).
 			AddFieldFn("id", func(obj interface{}) string {
 				rsc := obj.(*fakeResource)

internal/cmd/certificate/list.go

@@ -36,8 +36,8 @@ var ListCmd = base.ListCmd{
 		return resources, err
 	},
 
-	OutputTable: func(_ hcapi2.Client) *output.Table {
-		return output.NewTable().
+	OutputTable: func(t *output.Table, _ hcapi2.Client) {
+		t.
 			AddAllowedFields(hcloud.Certificate{}).
 			RemoveAllowedField("certificate", "chain").
 			AddFieldFn("labels", output.FieldFn(func(obj interface{}) string {

internal/cmd/config/list.go

@@ -73,17 +73,17 @@ func runList(s state.State, cmd *cobra.Command, _ []string) error {
 	if outOpts.IsSet("json") || outOpts.IsSet("yaml") {
 		schema := util.Wrap("options", options)
 		if outOpts.IsSet("json") {
-			return util.DescribeJSON(schema)
+			return util.DescribeJSON(cmd.OutOrStdout(), schema)
 		}
-		return util.DescribeYAML(schema)
+		return util.DescribeYAML(cmd.OutOrStdout(), schema)
 	}
 
 	cols := outputColumns
 	if outOpts.IsSet("columns") {
 		cols = outOpts["columns"]
 	}
 
-	t := output.NewTable()
+	t := output.NewTable(cmd.OutOrStdout())
 	t.AddAllowedFields(option{})
 	if !outOpts.IsSet("noheader") {
 		t.WriteHeader(cols)

internal/cmd/context/active_test.go

@@ -12,6 +12,10 @@ import (
 )
 
 func TestActive(t *testing.T) {
+	// Make sure we don't have any environment variables set that could interfere with the test
+	t.Setenv("HCLOUD_TOKEN", "")
+	t.Setenv("HCLOUD_CONTEXT", "")
+
 	testConfig := `
 active_context = "my-context"
 

internal/cmd/context/create_test.go

@@ -13,6 +13,10 @@ import (
 )
 
 func TestCreate(t *testing.T) {
+	// Make sure we don't have any environment variables set that could interfere with the test
+	t.Setenv("HCLOUD_TOKEN", "")
+	t.Setenv("HCLOUD_CONTEXT", "")
+
 	testConfig := `
 active_context = "my-context"