Skip to content

Commit

Permalink
main: rework the way the mock logger is passed
Browse files Browse the repository at this point in the history
Pass the mock logger directly to `run()` instead of mocking
`logrus.New`. Doing the later leads to a data race when multiple
parallel tests modify the (global) `var logrusNew logrus.New`.

Thanks to Tomas Hozza for reporting.
  • Loading branch information
mvo5 committed Jun 6, 2024
1 parent 95b4a9e commit dce99e4
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 24 deletions.
16 changes: 0 additions & 16 deletions cmd/osbuild-worker-executor/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@ import (
"os"
"path/filepath"
"testing"

"github.com/sirupsen/logrus"
logrusTest "github.com/sirupsen/logrus/hooks/test"
)

var (
Expand All @@ -15,19 +12,6 @@ var (
HandleIncludedSources = handleIncludedSources
)

func MockLogger() (hook *logrusTest.Hook, restore func()) {
saved := logrusNew
logger, hook := logrusTest.NewNullLogger()
logrusNew = func() *logrus.Logger {
return logger
}
logger.SetLevel(logrus.DebugLevel)

return hook, func() {
logrusNew = saved
}
}

func MockOsbuildBinary(t *testing.T, new string) (restore func()) {
t.Helper()

Expand Down
8 changes: 3 additions & 5 deletions cmd/osbuild-worker-executor/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ import (
// based on the excellent post
// https://grafana.com/blog/2024/02/09/how-i-write-http-services-in-go-after-13-years/

var logrusNew = logrus.New

func newServer(logger *logrus.Logger, config *Config) http.Handler {
mux := http.NewServeMux()
addRoutes(mux, logger, config)
Expand All @@ -27,11 +25,10 @@ func newServer(logger *logrus.Logger, config *Config) http.Handler {
return handler
}

func run(ctx context.Context, args []string, getenv func(string) string) error {
func run(ctx context.Context, args []string, getenv func(string) string, logger *logrus.Logger) error {
ctx, cancel := signal.NotifyContext(ctx, os.Interrupt)
defer cancel()

logger := logrusNew()
config, err := newConfigFromCmdline(args)
if err != nil {
return err
Expand Down Expand Up @@ -75,8 +72,9 @@ func run(ctx context.Context, args []string, getenv func(string) string) error {
}

func main() {
logger := logrus.New()
ctx := context.Background()
if err := run(ctx, os.Args, os.Getenv); err != nil {
if err := run(ctx, os.Args, os.Getenv, logger); err != nil {
fmt.Fprintf(os.Stderr, "%s\n", err)
os.Exit(1)
}
Expand Down
7 changes: 4 additions & 3 deletions cmd/osbuild-worker-executor/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"testing"
"time"

"github.com/sirupsen/logrus"
logrusTest "github.com/sirupsen/logrus/hooks/test"
"github.com/stretchr/testify/assert"

Expand Down Expand Up @@ -57,16 +58,16 @@ func runTestServer(t *testing.T) (baseURL, buildBaseDir string, loggerHook *logr
ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)

loggerHook, restore := main.MockLogger()
defer restore()
logger, loggerHook := logrusTest.NewNullLogger()
logger.SetLevel(logrus.DebugLevel)

args := []string{
"-host", host,
"-port", port,
"-build-path", buildBaseDir,
}
go func() {
_ = main.Run(ctx, args, os.Getenv)
_ = main.Run(ctx, args, os.Getenv, logger)
}()

err := waitReady(ctx, defaultTimeout, baseURL)
Expand Down

0 comments on commit dce99e4

Please sign in to comment.