Do not use OS globals in `check` command

The `check` command currently has a few anti-patterns:

1. It's exiting directly with os.Exit. This is undesired as the
   program will exit direcly without chance to catch error on
   higher levels or run deferred functions. The command is mostly
   using os.Exit to exit with a specific error code. Replace these
   uses by returning an error which carries the desired exit code.
   This ensures the control flow is closer to the typical.
2. The command is accessing OS globals stdout and stderr directly.
   The command should instead access them through the context which
   ensures they can be substituted as necessary for example in tests.
   Relying on globals is bad in general, so let's use the injected
   outputs from the context.

Some of the called code is still using globals through a global logger
and environment variables. These are left for later as their uses span
a wider aread in the code base. The goal here is to align the command
to better patterns so we have a better standard to follow in future
command implementations.

1 files changed, 14 insertions, 13 deletions

diff --git a/internal/cli/gitaly/check.go b/internal/cli/gitaly/check.go
index a6f4ef82b..010575575 100644
--- a/internal/cli/gitaly/check.go
+++ b/internal/cli/gitaly/check.go
@@ -3,7 +3,6 @@ package gitaly
 import (
 	"context"
 	"fmt"
-	"os"
 
 	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli/v2"
@@ -27,26 +26,28 @@ func checkAction(ctx *cli.Context) error {
 	logrus.SetLevel(logrus.ErrorLevel)
 
 	if ctx.NArg() != 1 || ctx.Args().First() == "" {
-		fmt.Fprintf(os.Stderr, "error: invalid argument(s)")
-		cli.ShowSubcommandHelpAndExit(ctx, 2)
+		if err := cli.ShowSubcommandHelp(ctx); err != nil {
+			return err
+		}
+
+		return cli.Exit("error: invalid argument(s)", 2)
 	}
 
 	configPath := ctx.Args().First()
 
-	fmt.Print("Checking GitLab API access: ")
+	fmt.Fprint(ctx.App.Writer, "Checking GitLab API access: ")
 	info, err := checkAPI(configPath)
 	if err != nil {
-		fmt.Println("FAILED")
-		fmt.Fprintln(os.Stderr, err)
-		os.Exit(1)
+		fmt.Fprintln(ctx.App.Writer, "FAILED")
+		return err
 	}
 
-	fmt.Println("OK")
-	fmt.Printf("GitLab version: %s\n", info.Version)
-	fmt.Printf("GitLab revision: %s\n", info.Revision)
-	fmt.Printf("GitLab Api version: %s\n", info.APIVersion)
-	fmt.Printf("Redis reachable for GitLab: %t\n", info.RedisReachable)
-	fmt.Println("OK")
+	fmt.Fprintln(ctx.App.Writer, "OK")
+	fmt.Fprintf(ctx.App.Writer, "GitLab version: %s\n", info.Version)
+	fmt.Fprintf(ctx.App.Writer, "GitLab revision: %s\n", info.Revision)
+	fmt.Fprintf(ctx.App.Writer, "GitLab Api version: %s\n", info.APIVersion)
+	fmt.Fprintf(ctx.App.Writer, "Redis reachable for GitLab: %t\n", info.RedisReachable)
+	fmt.Fprintln(ctx.App.Writer, "OK")
 
 	return nil
 }