Refactor for testability

cmd/wait-for-github/ci.go

@@ -1,5 +1,5 @@
 // wait-for-github
-// Copyright (C) 2022, Grafana Labs
+// Copyright (C) 2022-2023, Grafana Labs
 
 // This program is free software: you can redistribute it and/or modify it under
 // the terms of the GNU Affero General Public License as published by the Free
@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"regexp"
 	"strings"
+	"time"
 
 	"github.com/grafana/wait-for-github/internal/github"
 	"github.com/grafana/wait-for-github/internal/utils"
@@ -41,10 +42,9 @@ type ciConfig struct {
 var (
 	// https://regex101.com/r/dqMmDP/1
 	commitRegexp = regexp.MustCompile(`.*github\.com/(?P<owner>[^/]+)/(?P<repo>[^/]+)/commit/(?P<commit>[abcdef\d]+)/?.*`)
-	ciConf       ciConfig
 )
 
-func parseCIArguments(c *cli.Context) error {
+func parseCIArguments(c *cli.Context) (ciConfig, error) {
 	var owner, repo, ref string
 
 	switch {
@@ -53,7 +53,7 @@ func parseCIArguments(c *cli.Context) error {
 		url := c.Args().Get(0)
 		match := commitRegexp.FindStringSubmatch(url)
 		if match == nil {
-			return fmt.Errorf("invalid commit URL: %s", url)
+			return ciConfig{}, fmt.Errorf("invalid commit URL: %s", url)
 		}
 
 		owner = match[1]
@@ -74,108 +74,129 @@ func parseCIArguments(c *cli.Context) error {
 		cli.ShowCommandHelpAndExit(parent, "ci", 1)
 
 		// shouldn't get here, the previous line should exit
-		return nil
+		return ciConfig{}, nil
 	}
 
-	ciConf.owner = owner
-	ciConf.repo = repo
-	ciConf.ref = ref
-	ciConf.checks = c.StringSlice("check")
-
-	log.Debugf("Will wait for CI on %s/%s@%s", owner, repo, ref)
-
-	return nil
+	return ciConfig{
+		owner:  owner,
+		repo:   repo,
+		ref:    ref,
+		checks: c.StringSlice("check"),
+	}, nil
 }
 
-func handleCIStatus(status github.CIStatus) cli.ExitCoder {
+func handleCIStatus(status github.CIStatus, recheckInterval time.Duration) cli.ExitCoder {
 	switch status {
 	case github.CIStatusUnknown:
-		log.Infof("CI status is unknown, rechecking in %s", cfg.recheckInterval)
+		log.Infof("CI status is unknown, rechecking in %s", recheckInterval)
 	case github.CIStatusPending:
 	case github.CIStatusPassed:
 		return cli.Exit("CI successful", 0)
 	case github.CIStatusFailed:
 		return cli.Exit("CI failed", 1)
 	}
 
-	log.Infof("CI is not finished yet, rechecking in %s", cfg.recheckInterval)
+	log.Infof("CI is not finished yet, rechecking in %s", recheckInterval)
 	return nil
 }
 
-func checkCIStatus(c *cli.Context) error {
-	ctx := context.Background()
-
-	timeoutCtx, cancel := context.WithTimeout(ctx, cfg.globalTimeout)
-	defer cancel()
+type checkAllCI struct {
+	githubClient github.CheckCIStatus
+	owner        string
+	repo         string
+	ref          string
+}
 
-	githubClient, err := github.NewGithubClient(ctx, cfg.AuthInfo)
+func (ci checkAllCI) Check(ctx context.Context, recheckInterval time.Duration) error {
+	status, err := ci.githubClient.GetCIStatus(ctx, ci.owner, ci.repo, ci.ref)
 	if err != nil {
 		return err
 	}
 
-	log.Infof("Checking CI status on %s/%s@%s", ciConf.owner, ciConf.repo, ciConf.ref)
+	return handleCIStatus(status, recheckInterval)
+}
 
-	checkSpecificCI := func() error {
-		var status github.CIStatus
+type checkSpecificCI struct {
+	// same field as checkAllCI, and also a list of checks to wait for
+	checkAllCI
+	checks []string
+}
 
-		status, interestingChecks, err := githubClient.GetCIStatusForChecks(ctx, ciConf.owner, ciConf.repo, ciConf.ref, ciConf.checks)
-		if err != nil {
-			return err
-		}
+func (ci checkSpecificCI) Check(ctx context.Context, recheckInterval time.Duration) error {
+	var status github.CIStatus
 
-		if status == github.CIStatusFailed {
-			log.Infof("CI check %s failed, not waiting for other checks", strings.Join(interestingChecks, ", "))
-		}
+	status, interestingChecks, err := ci.githubClient.GetCIStatusForChecks(ctx, ci.owner, ci.repo, ci.ref, ci.checks)
+	if err != nil {
+		return err
+	}
 
-		// we didn't find any failed checks, and not all checks are finished, so
-		// we need to recheck
-		if status == github.CIStatusPending {
-			log.Infof("CI checks are not finished yet (still waiting for %s), rechecking in %s", strings.Join(interestingChecks, ", "), cfg.recheckInterval)
-		}
+	if status == github.CIStatusFailed {
+		log.Infof("CI check %s failed, not waiting for other checks", strings.Join(interestingChecks, ", "))
+	}
 
-		return handleCIStatus(status)
+	// we didn't find any failed checks, and not all checks are finished, so
+	// we need to recheck
+	if status == github.CIStatusPending {
+		log.Infof("CI checks are not finished yet (still waiting for %s), rechecking in %s", strings.Join(interestingChecks, ", "), recheckInterval)
 	}
 
-	checkAllCI := func() error {
-		status, err := githubClient.GetCIStatus(ctx, ciConf.owner, ciConf.repo, ciConf.ref)
-		if err != nil {
-			return err
-		}
+	return handleCIStatus(status, recheckInterval)
+}
 
-		ret := handleCIStatus(status)
+func checkCIStatus(timeoutCtx context.Context, cfg *config, ciConf *ciConfig) error {
+	githubClient, err := github.NewGithubClient(timeoutCtx, cfg.AuthInfo)
+	if err != nil {
+		return err
+	}
 
-		if ret == nil {
-			log.Infof("CI checks are not finished yet, rechecking in %s", cfg.recheckInterval)
-		}
+	log.Infof("Checking CI status on %s/%s@%s", ciConf.owner, ciConf.repo, ciConf.ref)
 
-		return ret
+	all := checkAllCI{
+		githubClient: githubClient,
+		owner:        ciConf.owner,
+		repo:         ciConf.repo,
+		ref:          ciConf.ref,
+	}
+
+	specific := checkSpecificCI{
+		checkAllCI: all,
+		checks:     ciConf.checks,
 	}
 
 	if len(ciConf.checks) > 0 {
 		log.Infof("Checking CI status for checks: %s", strings.Join(ciConf.checks, ", "))
-		return utils.RunUntilCancelledOrTimeout(timeoutCtx, checkSpecificCI, cfg.recheckInterval)
+		return utils.RunUntilCancelledOrTimeout(timeoutCtx, specific, cfg.recheckInterval)
 	}
 
-	return utils.RunUntilCancelledOrTimeout(timeoutCtx, checkAllCI, cfg.recheckInterval)
+	return utils.RunUntilCancelledOrTimeout(timeoutCtx, all, cfg.recheckInterval)
 }
 
-var ciCommand = &cli.Command{
-	Name:      "ci",
-	Usage:     "Wait for CI to be finished",
-	ArgsUsage: "<https://github.com/OWNER/REPO/commit/HASH|owner> [<repo> <ref>]",
-	Before:    parseCIArguments,
-	Action:    checkCIStatus,
-	Flags: []cli.Flag{
-		&cli.StringSliceFlag{
-			Name: "check",
-			Aliases: []string{
-				"c",
-			},
-			Usage: "Check the status of a specific CI check. " +
-				"By default, the status of all checks is checked.",
-			EnvVars: []string{
-				"GITHUB_CI_CHECKS",
+func ciCommand(cfg *config) *cli.Command {
+	var ciConf ciConfig
+
+	return &cli.Command{
+		Name:      "ci",
+		Usage:     "Wait for CI to be finished",
+		ArgsUsage: "<https://github.com/OWNER/REPO/commit/HASH|owner> [<repo> <ref>]",
+		Before: func(c *cli.Context) error {
+			var err error
+			ciConf, err = parseCIArguments(c)
+
+			return err
+		},
+		Action: func(c *cli.Context) error { return checkCIStatus(c.Context, cfg, &ciConf) },
+		Flags: []cli.Flag{
+			&cli.StringSliceFlag{
+				Name: "check",
+				Aliases: []string{
+					"c",
+				},
+				Usage: "Check the status of a specific CI check. " +
+					"By default, the status of all checks is checked.",
+				EnvVars: []string{
+					"GITHUB_CI_CHECKS",
+				},
 			},
 		},
-	},
+	}
 }

cmd/wait-for-github/config.go

@@ -27,5 +27,3 @@ type config struct {
 	recheckInterval time.Duration
 	globalTimeout   time.Duration
 }
-
-var cfg config

cmd/wait-for-github/main.go

@@ -1,5 +1,5 @@
 // wait-for-github
-// Copyright (C) 2022, Grafana Labs
+// Copyright (C) 2022-2023, Grafana Labs
 
 // This program is free software: you can redistribute it and/or modify it under
 // the terms of the GNU Affero General Public License as published by the Free
@@ -23,7 +23,7 @@ import (
 )
 
 func main() {
-	if err := root.Run(os.Args); err != nil {
+	if err := root().Run(os.Args); err != nil {
 		log.Fatal(err)
 	}
 }