test: fix race conditions (#390)

* stop setting the JLog for packages multiple times (somtimes caused a race)
* serial stdout-dependent tests
* panic wasn't restoring stdout in `testing/service_test.go`
* consistently pass logFrom as a pointer

cmd/argus/main.go

@@ -68,7 +68,7 @@ func main() {
 			}
 		}
 		msg := fmt.Sprintf("Found %d services to monitor:", serviceCount)
-		jLog.Info(msg, util.LogFrom{}, true)
+		jLog.Info(msg, &util.LogFrom{}, true)
 
 		for _, key := range config.Order {
 			if config.Service[key].Options.GetActive() {
@@ -77,8 +77,7 @@ func main() {
 		}
 	}
 
-	db.LogInit(&jLog, config.Settings.DataDatabaseFile())
-	go db.Run(&config)
+	go db.Run(&config, &jLog)
 
 	// Track all targets for changes in version and act on any found changes.
 	go (&config).Service.Track(&config.Order, &config.OrderMutex)

cmd/argus/main_test.go

@@ -18,12 +18,12 @@ package main
 
 import (
 	"fmt"
-	"io"
 	"os"
 	"strings"
 	"testing"
 	"time"
 
+	"github.com/release-argus/Argus/test"
 	"github.com/release-argus/Argus/util"
 )
 
@@ -76,35 +76,29 @@ func TestTheMain(t *testing.T) {
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
+			// t.Parallel() - Cannot run in parallel since we're using stdout
+			releaseStdout := test.CaptureStdout()
 
 			file := fmt.Sprintf("%s.yml", name)
 			os.Remove(tc.db)
 			tc.file(file, t)
 			defer os.Remove(tc.db)
 			resetFlags()
 			configFile = &file
-			stdout := os.Stdout
-			r, w, _ := os.Pipe()
-			os.Stdout = w
 			accessToken := os.Getenv("GITHUB_TOKEN")
 			os.Setenv("ARGUS_SERVICE_LATEST_VERSION_ACCESS_TOKEN", accessToken)
 
 			// WHEN Main is called
-			go func() {
-				main()
-			}()
+			go main()
 			time.Sleep(3 * time.Second)
 
 			// THEN the program will have printed everything expected
-			w.Close()
-			out, _ := io.ReadAll(r)
-			os.Stdout = stdout
-			output := string(out)
+			stdout := releaseStdout()
 			if tc.outputContains != nil {
 				for _, text := range *tc.outputContains {
-					if !strings.Contains(output, text) {
-						t.Errorf("%q couldn't be found in the output:\n%s",
-							text, output)
+					if !strings.Contains(stdout, text) {
+						t.Errorf("%q couldn't be found in stdout:\n%s",
+							text, stdout)
 					}
 				}
 			}

commands/commands.go

@@ -94,11 +94,11 @@ func (c *Controller) ExecIndex(logFrom *util.LogFrom, index int) (err error) {
 
 // Exec this Command and return any errors encountered.
 func (c *Command) Exec(logFrom *util.LogFrom) error {
-	jLog.Info(fmt.Sprintf("Executing '%s'", c), *logFrom, true)
+	jLog.Info(fmt.Sprintf("Executing '%s'", c), logFrom, true)
 	out, err := exec.Command((*c)[0], (*c)[1:]...).Output()
 
-	jLog.Error(util.ErrorToString(err), *logFrom, err != nil)
-	jLog.Info(string(out), *logFrom, err == nil && string(out) != "")
+	jLog.Error(util.ErrorToString(err), logFrom, err != nil)
+	jLog.Info(string(out), logFrom, err == nil && string(out) != "")
 
 	//nolint:wrapcheck
 	return err

commands/commands_test.go

@@ -18,13 +18,12 @@ package command
 
 import (
 	"fmt"
-	"io"
-	"os"
 	"reflect"
 	"regexp"
 	"testing"
 
 	svcstatus "github.com/release-argus/Argus/service/status"
+	"github.com/release-argus/Argus/test"
 	"github.com/release-argus/Argus/util"
 )
 
@@ -79,41 +78,36 @@ func TestCommand_Exec(t *testing.T) {
 	tests := map[string]struct {
 		cmd         Command
 		err         error
-		outputRegex string
+		stdoutRegex string
 	}{
 		"command that will pass": {
 			cmd:         Command{"date", "+%m-%d-%Y"},
-			outputRegex: `[0-9]{2}-[0-9]{2}-[0-9]{4}\s+$`},
+			stdoutRegex: `[0-9]{2}-[0-9]{2}-[0-9]{4}\s+$`},
 		"command that will fail": {
 			cmd:         Command{"false"},
 			err:         fmt.Errorf("exit status 1"),
-			outputRegex: `exit status 1\s+$`},
+			stdoutRegex: `exit status 1\s+$`},
 	}
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-
-			stdout := os.Stdout
-			r, w, _ := os.Pipe()
-			os.Stdout = w
+			// t.Parallel() - Cannot run in parallel since we're using stdout
+			releaseStdout := test.CaptureStdout()
 
 			// WHEN Exec is called on it
 			err := tc.cmd.Exec(&util.LogFrom{})
 
-			// THEN the output is expected
+			// THEN the stdout is expected
 			if util.ErrorToString(err) != util.ErrorToString(tc.err) {
 				t.Fatalf("err's differ\nwant: %s\ngot:  %s",
 					tc.err, err)
 			}
-			w.Close()
-			out, _ := io.ReadAll(r)
-			os.Stdout = stdout
-			output := string(out)
-			re := regexp.MustCompile(tc.outputRegex)
-			match := re.MatchString(output)
+			stdout := releaseStdout()
+			re := regexp.MustCompile(tc.stdoutRegex)
+			match := re.MatchString(stdout)
 			if !match {
 				t.Errorf("want match for %q\nnot: %q",
-					tc.outputRegex, output)
+					tc.stdoutRegex, stdout)
 			}
 		})
 	}
@@ -138,49 +132,44 @@ func TestController_ExecIndex(t *testing.T) {
 	tests := map[string]struct {
 		index       int
 		err         error
-		outputRegex string
+		stdoutRegex string
 		noAnnounce  bool
 	}{
 		"command index out of range": {
 			index:       2,
-			outputRegex: `^$`,
+			stdoutRegex: `^$`,
 			noAnnounce:  true},
 		"command index that will pass": {
 			index:       0,
-			outputRegex: `[0-9]{2}-[0-9]{2}-[0-9]{4}\s+$`},
+			stdoutRegex: `[0-9]{2}-[0-9]{2}-[0-9]{4}\s+$`},
 		"command index that will fail": {
 			index:       1,
 			err:         fmt.Errorf("exit status 1"),
-			outputRegex: `exit status 1\s+$`},
+			stdoutRegex: `exit status 1\s+$`},
 	}
 
 	runNumber := 0
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-
-			stdout := os.Stdout
-			r, w, _ := os.Pipe()
-			os.Stdout = w
+			// t.Parallel() - Cannot run in parallel since we're using stdout
+			releaseStdout := test.CaptureStdout()
 
 			// WHEN the Command @index is exectured
 			err := controller.ExecIndex(&util.LogFrom{}, tc.index)
 
-			// THEN the output is expected
+			// THEN the stdout is expected
 			// err
 			if util.ErrorToString(err) != util.ErrorToString(tc.err) {
 				t.Fatalf("err's differ\nwant: %s\ngot:  %s",
 					tc.err, err)
 			}
-			// output
-			w.Close()
-			out, _ := io.ReadAll(r)
-			os.Stdout = stdout
-			output := string(out)
-			re := regexp.MustCompile(tc.outputRegex)
-			match := re.MatchString(output)
+			// stdout
+			stdout := releaseStdout()
+			re := regexp.MustCompile(tc.stdoutRegex)
+			match := re.MatchString(stdout)
 			if !match {
 				t.Fatalf("want match for %q\nnot: %q",
-					tc.outputRegex, output)
+					tc.stdoutRegex, stdout)
 			}
 			// announced
 			if !tc.noAnnounce {
@@ -200,36 +189,35 @@ func TestController_Exec(t *testing.T) {
 		nilController bool
 		commands      *Slice
 		err           error
-		outputRegex   string
+		stdoutRegex   string
 		noAnnounce    bool
 	}{
 		"nil Controller": {
 			nilController: true,
-			outputRegex:   `^$`,
+			stdoutRegex:   `^$`,
 			noAnnounce:    true},
 		"nil Command": {
-			outputRegex: `^$`,
+			stdoutRegex: `^$`,
 			noAnnounce:  true},
 		"single Command": {
-			outputRegex: `[0-9]{2}-[0-9]{2}-[0-9]{4}\s+$`,
+			stdoutRegex: `[0-9]{2}-[0-9]{2}-[0-9]{4}\s+$`,
 			commands: &Slice{
 				{"date", "+%m-%d-%Y"}}},
 		"multiple Command's": {
 			err:         fmt.Errorf("\nexit status 1"),
-			outputRegex: `[0-9]{2}-[0-9]{2}-[0-9]{4}\s+.*'false'\s.*exit status 1\s+$`,
+			stdoutRegex: `[0-9]{2}-[0-9]{2}-[0-9]{4}\s+.*'false'\s.*exit status 1\s+$`,
 			commands: &Slice{
 				{"date", "+%m-%d-%Y"},
 				{"false"}}},
 	}
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
+			// t.Parallel() - Cannot run in parallel since we're using stdout
+			releaseStdout := test.CaptureStdout()
 
 			announce := make(chan []byte, 8)
 			controller := testController(&announce)
-			stdout := os.Stdout
-			r, w, _ := os.Pipe()
-			os.Stdout = w
 
 			// WHEN the Command @index is exectured
 			controller.Command = tc.commands
@@ -238,22 +226,19 @@ func TestController_Exec(t *testing.T) {
 			}
 			err := controller.Exec(&util.LogFrom{})
 
-			// THEN the output is expected
+			// THEN the stdout is expected
 			// err
 			if util.ErrorToString(err) != util.ErrorToString(tc.err) {
 				t.Fatalf("err's differ\nwant: %q\ngot:  %q",
 					util.ErrorToString(tc.err), util.ErrorToString(err))
 			}
-			// output
-			w.Close()
-			out, _ := io.ReadAll(r)
-			os.Stdout = stdout
-			output := string(out)
-			re := regexp.MustCompile(tc.outputRegex)
-			match := re.MatchString(output)
+			// stdout
+			stdout := releaseStdout()
+			re := regexp.MustCompile(tc.stdoutRegex)
+			match := re.MatchString(stdout)
 			if !match {
 				t.Fatalf("want match for %q\nnot: %q",
-					tc.outputRegex, output)
+					tc.stdoutRegex, stdout)
 			}
 			// announced
 			runNumber := 0

config/defaults.go

@@ -71,7 +71,7 @@ func (d *Defaults) MapEnvToStruct() {
 		jLog.Fatal(
 			"One or more 'ARGUS_' environment variables are incorrect:\n"+
 				strings.ReplaceAll(util.ErrorToString(err), "\\", "\n"),
-			util.LogFrom{}, true)
+			&util.LogFrom{}, true)
 	}
 }
 

config/defaults_test.go

@@ -17,7 +17,6 @@
 package config
 
 import (
-	"io"
 	"os"
 	"regexp"
 	"strings"
@@ -29,6 +28,7 @@ import (
 	latestver "github.com/release-argus/Argus/service/latest_version"
 	"github.com/release-argus/Argus/service/latest_version/filter"
 	opt "github.com/release-argus/Argus/service/options"
+	"github.com/release-argus/Argus/test"
 	"github.com/release-argus/Argus/util"
 	"github.com/release-argus/Argus/webhook"
 )
@@ -919,19 +919,22 @@ func TestDefaults_MapEnvToStruct(t *testing.T) {
 			// Catch fatal panics.
 			defer func() {
 				r := recover()
-				if r != nil {
-					if tc.errRegex == "" {
-						t.Fatalf("unexpected panic: %v", r)
-					}
-					switch r.(type) {
-					case string:
-						if !regexp.MustCompile(tc.errRegex).MatchString(r.(string)) {
-							t.Errorf("want error matching:\n%v\ngot:\n%v",
-								tc.errRegex, r.(string))
-						}
-					default:
-						t.Fatalf("unexpected panic: %v", r)
+				// Ignore nil panics.
+				if r == nil {
+					return
+				}
+
+				if tc.errRegex == "" {
+					t.Fatalf("unexpected panic: %v", r)
+				}
+				switch r.(type) {
+				case string:
+					if !regexp.MustCompile(tc.errRegex).MatchString(r.(string)) {
+						t.Errorf("want error matching:\n%v\ngot:\n%v",
+							tc.errRegex, r.(string))
 					}
+				default:
+					t.Fatalf("unexpected panic: %v", r)
 				}
 			}()
 
@@ -1072,22 +1075,18 @@ func TestDefaults_Print(t *testing.T) {
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-
-			stdout := os.Stdout
-			r, w, _ := os.Pipe()
-			os.Stdout = w
+			// t.Parallel() - Cannot run in parallel since we're using stdout
+			releaseStdout := test.CaptureStdout()
 
 			// WHEN Print is called
 			tc.input.Print("")
 
 			// THEN the expected number of lines are printed
-			w.Close()
-			out, _ := io.ReadAll(r)
-			os.Stdout = stdout
-			got := strings.Count(string(out), "\n")
+			stdout := releaseStdout()
+			got := strings.Count(stdout, "\n")
 			if got != tc.lines {
 				t.Errorf("Print should have given %d lines, but gave %d\n%s",
-					tc.lines, got, out)
+					tc.lines, got, stdout)
 			}
 		})
 	}