Remove data races by avoiding global CLI logger (#43)

* Updated README.md

* Updated build.yml

* comment out nasty data race lines, to see how they fare on CI

* bump golangci-lint in CI

* restore color.NoColor line

* restore debug level

* revert that

* No more global logger

* fix lints

* Padding is no longer a global

* reverted formatting changes in shell_test.go

* don't customize console color during tests

* finally fixed the data race in tests

* Add GHA test annotation

* remove verbose flag on test run because it no longer does anything

* remove test annotation action

* Comment out color-mode customization

* Fix parallel testing issues (maybe?)

.github/workflows/build.yml

@@ -39,7 +39,7 @@ jobs:
             ${{ runner.os }}-go-
 
       - name: Run Tests
-        run: go test -v -race -coverprofile coverage.out -covermode atomic ./...
+        run: go test -race -coverprofile coverage.out -covermode atomic ./...
 
       - name: Upload Coverage to Codecov
         if: success()
@@ -60,7 +60,7 @@ jobs:
       - name: golangci-lint
         uses: golangci/golangci-lint-action@master
         with:
-          version: v1.37
+          version: v1.39
           skip-go-installation: true
 
   verify_doc_tools:
@@ -75,7 +75,7 @@ jobs:
 
       - name: Run gendoc tool
         run: go run ./tools/gendoc
-      
+
       - name: Run licensing tool
         run: go run ./tools/licensing
 

.gitignore

@@ -16,7 +16,7 @@
 # Test binary, built with `go test -c`
 *.test
 
-# Output of the go coverage tool, specifically when used with LiteIDE
+# Output of the go coverage tool
 *.out
 
 # Dependency directories (remove the comment below to include it)

README.md

@@ -17,8 +17,8 @@ Documentation is hosted at <https://cidertool.github.io/cider>. Check out our in
 
 ## Integrations
 
--   [GitHub Action](https://github.com/marketplace/actions/cider-action)
--   [Buildkite Plugin](https://github.com/cidertool/cider-buildkite-plugin)
+- [GitHub Action](https://github.com/marketplace/actions/cider-action)
+- [Buildkite Plugin](https://github.com/cidertool/cider-buildkite-plugin)
 
 ## Badges
 
@@ -37,7 +37,7 @@ This project's primary goal is to simplify the process to release on the App Sto
 
 Special thanks to:
 
--   [GoReleaser](https://goreleaser.com/) for inspiring the architecture and open sourcing several components used in Cider
+- [GoReleaser](https://goreleaser.com/) for inspiring the architecture and open sourcing several components used in Cider
 
 ## License
 

internal/clicommand/check.go

@@ -23,20 +23,20 @@ package clicommand
 import (
 	"fmt"
 
-	"github.com/apex/log"
 	"github.com/cidertool/cider/internal/pipe/defaults"
 	"github.com/cidertool/cider/pkg/context"
 	"github.com/fatih/color"
 	"github.com/spf13/cobra"
 )
 
 type checkCmd struct {
-	cmd    *cobra.Command
-	config string
+	cmd            *cobra.Command
+	debugFlagValue *bool
+	config         string
 }
 
-func newCheckCmd() *checkCmd {
-	var root = &checkCmd{}
+func newCheckCmd(debugFlagValue *bool) *checkCmd {
+	var root = &checkCmd{debugFlagValue: debugFlagValue}
 
 	var cmd = &cobra.Command{
 		Use:           "check",
@@ -45,32 +45,37 @@ func newCheckCmd() *checkCmd {
 		Example:       "cider check",
 		SilenceUsage:  true,
 		SilenceErrors: true,
-		RunE: func(cmd *cobra.Command, args []string) error {
-			cfg, err := loadConfig(root.config, "")
-			if err != nil {
-				return err
-			}
-			var ctx = context.New(cfg)
+		RunE:          root.Run,
+	}
 
-			if err := context.NewInterrupt().Run(ctx, func() error {
-				log.Info(color.New(color.Bold).Sprint("checking config:"))
+	cmd.Flags().StringVarP(&root.config, "config", "f", "", "Configuration file to check")
 
-				return defaults.Pipe{}.Run(ctx)
-			}); err != nil {
-				log.WithError(err).Error(color.New(color.Bold).Sprintf("config is invalid"))
+	root.cmd = cmd
 
-				return fmt.Errorf("invalid config: %w", err)
-			}
+	return root
+}
 
-			log.Info(color.New(color.Bold).Sprintf("config is valid"))
+func (cmd *checkCmd) Run(c *cobra.Command, args []string) error {
+	logger := newLogger(cmd.debugFlagValue)
 
-			return nil
-		},
+	cfg, err := loadConfig(cmd.config, "")
+	if err != nil {
+		return err
 	}
 
-	cmd.Flags().StringVarP(&root.config, "config", "f", "", "Configuration file to check")
+	var ctx = context.New(cfg)
 
-	root.cmd = cmd
+	if err := context.NewInterrupt().Run(ctx, func() error {
+		logger.Info(color.New(color.Bold).Sprint("checking config:"))
 
-	return root
+		return defaults.Pipe{}.Run(ctx)
+	}); err != nil {
+		logger.WithError(err).Error(color.New(color.Bold).Sprintf("config is invalid"))
+
+		return fmt.Errorf("invalid config: %w", err)
+	}
+
+	logger.Info(color.New(color.Bold).Sprintf("config is valid"))
+
+	return nil
 }

internal/clicommand/check_test.go

@@ -32,7 +32,9 @@ import (
 func TestCheckCmd(t *testing.T) {
 	t.Parallel()
 
-	var cmd = newCheckCmd()
+	var noDebug bool
+
+	var cmd = newCheckCmd(&noDebug)
 
 	var path = filepath.Join(t.TempDir(), "foo.yaml")
 

internal/clicommand/clicommand.go

@@ -0,0 +1,42 @@
+/**
+Copyright (C) 2020 Aaron Sky.
+
+This file is part of Cider, a tool for automating submission
+of apps to Apple's App Stores.
+
+Cider is free software: you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation, either version 3 of the License, or
+(at your option) any later version.
+
+Cider is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with Cider.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+// Package clicommand declares the command line interface for Cider.
+package clicommand
+
+import (
+	"github.com/cidertool/cider/internal/log"
+)
+
+func newLogger(debugFlagValue *bool) *log.Log {
+	logger := log.New()
+
+	// Comment this out as it's causing issues during parallel testing
+	// if os.Getenv("CI") != "" {
+	// 	logger.SetColorMode(false)
+	// }
+
+	if debugFlagValue != nil {
+		logger.SetDebug(*debugFlagValue)
+		logger.Debug("debug logs enabled")
+	}
+
+	return logger
+}

internal/clicommand/init.go

@@ -25,8 +25,8 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/apex/log"
 	"github.com/cidertool/cider/internal/closer"
+	"github.com/cidertool/cider/internal/log"
 	"github.com/cidertool/cider/pkg/config"
 	"github.com/fatih/color"
 	"github.com/manifoldco/promptui"
@@ -50,7 +50,7 @@ type initOpts struct {
 	skipPrompt bool
 }
 
-func newInitCmd() *initCmd {
+func newInitCmd(debugFlagValue *bool) *initCmd {
 	var root = &initCmd{}
 
 	var cmd = &cobra.Command{
@@ -62,7 +62,9 @@ func newInitCmd() *initCmd {
 		SilenceUsage:  true,
 		SilenceErrors: true,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return initProject(root.opts)
+			logger := newLogger(debugFlagValue)
+
+			return initProject(root.opts, logger)
 		},
 	}
 
@@ -74,17 +76,17 @@ func newInitCmd() *initCmd {
 	return root
 }
 
-func initProject(opts initOpts) (err error) {
-	file, err := createFileIfNeeded(opts.config, opts.skipPrompt)
+func initProject(opts initOpts, logger log.Interface) (err error) {
+	file, err := createFileIfNeeded(opts.config, opts.skipPrompt, logger)
 	if err != nil {
 		return err
 	}
 
 	defer closer.Close(file)
 
-	log.Info(color.New(color.Bold).Sprintf("Populating project file at %s", opts.config))
+	logger.Info(color.New(color.Bold).Sprintf("Populating project file at %s", opts.config))
 
-	project, err := newProject(opts.skipPrompt)
+	project, err := newProject(opts.skipPrompt, logger)
 	if err != nil {
 		return err
 	}
@@ -93,16 +95,16 @@ func initProject(opts initOpts) (err error) {
 		return err
 	}
 
-	log.
+	logger.
 		WithField("file", file.Name()).
 		Info("config created")
-	log.Info("Please edit accordingly to fit your needs.")
-	log.Info("For additional configuration options, see: https://cidertool.github.io/cider/configuration")
+	logger.Info("Please edit accordingly to fit your needs.")
+	logger.Info("For additional configuration options, see: https://cidertool.github.io/cider/configuration")
 
 	return nil
 }
 
-func createFileIfNeeded(path string, skipPrompt bool) (*os.File, error) {
+func createFileIfNeeded(path string, skipPrompt bool, logger log.Interface) (*os.File, error) {
 	f, err := os.OpenFile(filepath.Clean(path), os.O_WRONLY|os.O_CREATE|os.O_TRUNC|os.O_EXCL, 0600)
 	if err == nil {
 		return f, nil
@@ -113,7 +115,7 @@ func createFileIfNeeded(path string, skipPrompt bool) (*os.File, error) {
 	}
 
 	if skipPrompt {
-		log.Warn("file exists, overwriting")
+		logger.Warn("file exists, overwriting")
 	} else {
 		prompt := promptui.Prompt{
 			Label:     "Overwrite file?",
@@ -128,26 +130,26 @@ func createFileIfNeeded(path string, skipPrompt bool) (*os.File, error) {
 	return os.OpenFile(filepath.Clean(path), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600)
 }
 
-func newProject(skipPrompt bool) (*config.Project, error) {
+func newProject(skipPrompt bool, logger log.Interface) (*config.Project, error) {
 	var project *config.Project
 
 	var err error
 
 	if skipPrompt {
 		project = newProjectFromDefaults()
 	} else {
-		project, err = newProjectFromPrompts()
+		project, err = newProjectFromPrompts(logger)
 	}
 
 	return project, err
 }
 
-func newProjectFromPrompts() (*config.Project, error) {
+func newProjectFromPrompts(logger log.Interface) (*config.Project, error) {
 	values := projectInitValues{}
 
 	var continueAppsSetup = true
 	for continueAppsSetup {
-		name, app, err := promptAppValues()
+		name, app, err := promptAppValues(logger)
 		if err != nil {
 			return nil, err
 		}
@@ -167,7 +169,7 @@ func newProjectFromPrompts() (*config.Project, error) {
 	return &proj, nil
 }
 
-func promptAppValues() (name string, app *projectInitAppValues, err error) {
+func promptAppValues(logger log.Interface) (name string, app *projectInitAppValues, err error) {
 	var prompt promptui.Prompt
 
 	var selec promptui.Select
@@ -180,7 +182,7 @@ func promptAppValues() (name string, app *projectInitAppValues, err error) {
 		PhasedReleaseEnabled:         true,
 	}
 
-	log.Info("Let's set up an app in your project!")
+	logger.Info("Let's set up an app in your project!")
 
 	prompt = promptui.Prompt{Label: "App Name"}
 

internal/clicommand/init_test.go

@@ -32,7 +32,9 @@ func TestInitCmd(t *testing.T) {
 
 	var folder = t.TempDir()
 
-	var cmd = newInitCmd().cmd
+	var noDebug bool
+
+	var cmd = newInitCmd(&noDebug).cmd
 
 	var path = filepath.Join(folder, "foo.yaml")