Merge pull request #93 from github/parametrized-output

Don't write directly to `os.Stdout` and `os.Stderr`

Author: mhagger

git-sizer.go

@@ -8,12 +8,14 @@ import (
 	"os"
 	"runtime/pprof"
 	"strconv"
+	"time"
 
 	"github.com/spf13/pflag"
 
 	"github.com/github/git-sizer/git"
 	"github.com/github/git-sizer/internal/refopts"
 	"github.com/github/git-sizer/isatty"
+	"github.com/github/git-sizer/meter"
 	"github.com/github/git-sizer/sizes"
 )
 
@@ -91,29 +93,30 @@ var ReleaseVersion string
 var BuildVersion string
 
 func main() {
-	err := mainImplementation(os.Args[1:])
+	err := mainImplementation(os.Stdout, os.Stderr, os.Args[1:])
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "error: %s\n", err)
 		os.Exit(1)
 	}
 }
 
-func mainImplementation(args []string) error {
+func mainImplementation(stdout, stderr io.Writer, args []string) error {
 	var nameStyle sizes.NameStyle = sizes.NameStyleFull
 	var cpuprofile string
 	var jsonOutput bool
 	var jsonVersion int
 	var threshold sizes.Threshold = 1
 	var progress bool
 	var version bool
+	var showRefs bool
 
 	// Try to open the repository, but it's not an error yet if this
 	// fails, because the user might only be asking for `--help`.
 	repo, repoErr := git.NewRepository(".")
 
 	flags := pflag.NewFlagSet("git-sizer", pflag.ContinueOnError)
 	flags.Usage = func() {
-		fmt.Print(usage)
+		fmt.Fprint(stdout, usage)
 	}
 
 	flags.VarP(
@@ -151,11 +154,15 @@ func mainImplementation(args []string) error {
 	flags.BoolVarP(&jsonOutput, "json", "j", false, "output results in JSON format")
 	flags.IntVar(&jsonVersion, "json-version", 1, "JSON format version to output (1 or 2)")
 
-	atty, err := isatty.Isatty(os.Stderr.Fd())
-	if err != nil {
-		atty = false
+	defaultProgress := false
+	if f, ok := stderr.(*os.File); ok {
+		atty, err := isatty.Isatty(f.Fd())
+		if err == nil && atty {
+			defaultProgress = true
+		}
 	}
-	flags.BoolVar(&progress, "progress", atty, "report progress to stderr")
+
+	flags.BoolVar(&progress, "progress", defaultProgress, "report progress to stderr")
 	flags.BoolVar(&version, "version", false, "report the git-sizer version number")
 	flags.Var(&NegatedBoolValue{&progress}, "no-progress", "suppress progress output")
 	flags.Lookup("no-progress").NoOptDefVal = "true"
@@ -177,6 +184,8 @@ func mainImplementation(args []string) error {
 
 	rgb.AddRefopts(flags)
 
+	flags.BoolVar(&showRefs, "show-refs", false, "list the references being processed")
+
 	flags.SortFlags = false
 
 	err = flags.Parse(args)
@@ -200,9 +209,9 @@ func mainImplementation(args []string) error {
 
 	if version {
 		if ReleaseVersion != "" {
-			fmt.Printf("git-sizer release %s\n", ReleaseVersion)
+			fmt.Fprintf(stdout, "git-sizer release %s\n", ReleaseVersion)
 		} else {
-			fmt.Printf("git-sizer build %s\n", BuildVersion)
+			fmt.Fprintf(stdout, "git-sizer build %s\n", BuildVersion)
 		}
 		return nil
 	}
@@ -269,7 +278,17 @@ func mainImplementation(args []string) error {
 		return err
 	}
 
-	historySize, err := sizes.ScanRepositoryUsingGraph(repo, rg, nameStyle, progress)
+	if showRefs {
+		fmt.Fprintf(stderr, "References (included references marked with '+'):\n")
+		rg = refopts.NewShowRefGrouper(rg, stderr)
+	}
+
+	var progressMeter meter.Progress = meter.NoProgressMeter
+	if progress {
+		progressMeter = meter.NewProgressMeter(stderr, 100*time.Millisecond)
+	}
+
+	historySize, err := sizes.ScanRepositoryUsingGraph(repo, rg, nameStyle, progressMeter)
 	if err != nil {
 		return fmt.Errorf("error scanning repository: %w", err)
 	}
@@ -288,11 +307,10 @@ func mainImplementation(args []string) error {
 		if err != nil {
 			return fmt.Errorf("could not convert %v to json: %w", historySize, err)
 		}
-		fmt.Printf("%s\n", j)
+		fmt.Fprintf(stdout, "%s\n", j)
 	} else {
 		if _, err := io.WriteString(
-			os.Stdout,
-			historySize.TableString(rg.Groups(), threshold, nameStyle),
+			stdout, historySize.TableString(rg.Groups(), threshold, nameStyle),
 		); err != nil {
 			return fmt.Errorf("writing output: %w", err)
 		}

git_sizer_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/github/git-sizer/counts"
 	"github.com/github/git-sizer/git"
 	"github.com/github/git-sizer/internal/testutils"
+	"github.com/github/git-sizer/meter"
 	"github.com/github/git-sizer/sizes"
 )
 
@@ -561,7 +562,7 @@ func TestBomb(t *testing.T) {
 
 	h, err := sizes.ScanRepositoryUsingGraph(
 		repo.Repository(t),
-		refGrouper{}, sizes.NameStyleFull, false,
+		refGrouper{}, sizes.NameStyleFull, meter.NoProgressMeter,
 	)
 	require.NoError(t, err)
 
@@ -634,7 +635,7 @@ func TestTaggedTags(t *testing.T) {
 
 	h, err := sizes.ScanRepositoryUsingGraph(
 		repo.Repository(t),
-		refGrouper{}, sizes.NameStyleNone, false,
+		refGrouper{}, sizes.NameStyleNone, meter.NoProgressMeter,
 	)
 	require.NoError(t, err, "scanning repository")
 	assert.Equal(t, counts.Count32(3), h.MaxTagDepth, "tag depth")
@@ -656,7 +657,7 @@ func TestFromSubdir(t *testing.T) {
 
 	h, err := sizes.ScanRepositoryUsingGraph(
 		repo.Repository(t),
-		refGrouper{}, sizes.NameStyleNone, false,
+		refGrouper{}, sizes.NameStyleNone, meter.NoProgressMeter,
 	)
 	require.NoError(t, err, "scanning repository")
 	assert.Equal(t, counts.Count32(2), h.MaxPathDepth, "max path depth")
@@ -709,7 +710,7 @@ func TestSubmodule(t *testing.T) {
 	// Analyze the main repo:
 	h, err := sizes.ScanRepositoryUsingGraph(
 		mainRepo.Repository(t),
-		refGrouper{}, sizes.NameStyleNone, false,
+		refGrouper{}, sizes.NameStyleNone, meter.NoProgressMeter,
 	)
 	require.NoError(t, err, "scanning repository")
 	assert.Equal(t, counts.Count32(2), h.UniqueBlobCount, "unique blob count")
@@ -722,7 +723,7 @@ func TestSubmodule(t *testing.T) {
 	}
 	h, err = sizes.ScanRepositoryUsingGraph(
 		submRepo2.Repository(t),
-		refGrouper{}, sizes.NameStyleNone, false,
+		refGrouper{}, sizes.NameStyleNone, meter.NoProgressMeter,
 	)
 	require.NoError(t, err, "scanning repository")
 	assert.Equal(t, counts.Count32(2), h.UniqueBlobCount, "unique blob count")

internal/refopts/ref_group_builder.go

@@ -2,7 +2,6 @@ package refopts
 
 import (
 	"fmt"
-	"os"
 	"strings"
 
 	"github.com/spf13/pflag"
@@ -21,8 +20,6 @@ type Configger interface {
 type RefGroupBuilder struct {
 	topLevelGroup *refGroup
 	groups        map[sizes.RefGroupSymbol]*refGroup
-
-	ShowRefs bool
 }
 
 // NewRefGroupBuilder creates and returns a `RefGroupBuilder`
@@ -253,8 +250,6 @@ func (rgb *RefGroupBuilder) AddRefopts(flags *pflag.FlagSet) {
 	)
 	flag.Hidden = true
 	flag.Deprecated = "use --include=@REFGROUP"
-
-	flags.BoolVar(&rgb.ShowRefs, "show-refs", false, "list the references being processed")
 }
 
 // Finish collects the information gained from processing the options
@@ -280,11 +275,6 @@ func (rgb *RefGroupBuilder) Finish() (sizes.RefGrouper, error) {
 		refGrouper.refGroups = append(refGrouper.refGroups, *refGrouper.ignoredRefGroup)
 	}
 
-	if rgb.ShowRefs {
-		fmt.Fprintf(os.Stderr, "References (included references marked with '+'):\n")
-		return showRefGrouper{&refGrouper, os.Stderr}, nil
-	}
-
 	return &refGrouper, nil
 }
 

internal/refopts/show_ref_grouper.go

@@ -7,18 +7,28 @@ import (
 	"github.com/github/git-sizer/sizes"
 )
 
-// showRefFilter is a `git.ReferenceFilter` that logs its choices to Stderr.
+// showRefFilter is a `git.ReferenceFilter` that logs its choices to
+// an `io.Writer`.
 type showRefGrouper struct {
-	*refGrouper
+	sizes.RefGrouper
 	w io.Writer
 }
 
-func (refGrouper showRefGrouper) Categorize(refname string) (bool, []sizes.RefGroupSymbol) {
-	walk, symbols := refGrouper.refGrouper.Categorize(refname)
+// Return a `sizes.RefGrouper` that wraps its argument and behaves
+// like it except that it also logs its decisions to an `io.Writer`.
+func NewShowRefGrouper(rg sizes.RefGrouper, w io.Writer) sizes.RefGrouper {
+	return showRefGrouper{
+		RefGrouper: rg,
+		w:          w,
+	}
+}
+
+func (rg showRefGrouper) Categorize(refname string) (bool, []sizes.RefGroupSymbol) {
+	walk, symbols := rg.RefGrouper.Categorize(refname)
 	if walk {
-		fmt.Fprintf(refGrouper.w, "+ %s\n", refname)
+		fmt.Fprintf(rg.w, "+ %s\n", refname)
 	} else {
-		fmt.Fprintf(refGrouper.w, "  %s\n", refname)
+		fmt.Fprintf(rg.w, "  %s\n", refname)
 	}
 	return walk, symbols
 }

meter/meter.go

@@ -2,7 +2,7 @@ package meter
 
 import (
 	"fmt"
-	"os"
+	"io"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -30,9 +30,10 @@ type Progress interface {
 var Spinners = []string{"|", "(", "<", "-", "<", "(", "|", ")", ">", "-", ">", ")"}
 
 // progressMeter is a `Progress` that reports the current state every
-// `period`.
+// `period` to an `io.Writer`.
 type progressMeter struct {
 	lock           sync.Mutex
+	w              io.Writer
 	format         string
 	period         time.Duration
 	lastShownCount int64
@@ -48,8 +49,9 @@ type progressMeter struct {
 // NewProgressMeter returns a progress meter that can be used to show
 // progress to a TTY periodically, including an increasing int64
 // value.
-func NewProgressMeter(period time.Duration) Progress {
+func NewProgressMeter(w io.Writer, period time.Duration) Progress {
 	return &progressMeter{
+		w:      w,
 		period: period,
 	}
 }
@@ -81,7 +83,7 @@ func (p *progressMeter) Start(format string) {
 			} else {
 				s = ""
 			}
-			fmt.Fprintf(os.Stderr, p.format, c, s, "\r")
+			fmt.Fprintf(p.w, p.format, c, s, "\r")
 			p.lock.Unlock()
 		}
 	}()
@@ -100,14 +102,16 @@ func (p *progressMeter) Done() {
 	defer p.lock.Unlock()
 	p.ticker = nil
 	c := atomic.LoadInt64(&p.count)
-	fmt.Fprintf(os.Stderr, p.format, c, " ", "\n")
+	fmt.Fprintf(p.w, p.format, c, " ", "\n")
 }
 
 // NoProgressMeter is a `Progress` that doesn't actually report
 // anything.
-type NoProgressMeter struct{}
+var NoProgressMeter noProgressMeter
 
-func (p *NoProgressMeter) Start(string) {}
-func (p *NoProgressMeter) Inc()         {}
-func (p *NoProgressMeter) Add(int64)    {}
-func (p *NoProgressMeter) Done()        {}
+type noProgressMeter struct{}
+
+func (p noProgressMeter) Start(string) {}
+func (p noProgressMeter) Inc()         {}
+func (p noProgressMeter) Add(int64)    {}
+func (p noProgressMeter) Done()        {}

sizes/graph.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"io"
 	"sync"
-	"time"
 
 	"github.com/github/git-sizer/counts"
 	"github.com/github/git-sizer/git"
@@ -65,15 +64,10 @@ type refSeen struct {
 //
 // It returns the size data for the repository.
 func ScanRepositoryUsingGraph(
-	repo *git.Repository, rg RefGrouper, nameStyle NameStyle, progress bool,
+	repo *git.Repository, rg RefGrouper, nameStyle NameStyle,
+	progressMeter meter.Progress,
 ) (HistorySize, error) {
 	graph := NewGraph(rg, nameStyle)
-	var progressMeter meter.Progress
-	if progress {
-		progressMeter = meter.NewProgressMeter(100 * time.Millisecond)
-	} else {
-		progressMeter = &meter.NoProgressMeter{}
-	}
 
 	refIter, err := repo.NewReferenceIter()
 	if err != nil {