fix(tests): properly silence log output (#1259)

* fix(tests): properly silence log output Using `init` allows it to also work for benchmarks. And `log.Silence` was sometimes getting overridden by `log.init`. * squash: fix(server): don't setup the logger again
0xERR0R · Nov 19, 2023 · d52c598 · d52c598
1 parent 94663ee
commit d52c598
Show file tree

Hide file tree

Showing 21 changed files with 88 additions and 35 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -105,5 +105,6 @@ issues:
       linters:
         - dupl
         - funlen
+        - gochecknoinits
         - gochecknoglobals
         - gosec
diff --git a/api/api_suite_test.go b/api/api_suite_test.go
@@ -8,8 +8,11 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-func TestResolver(t *testing.T) {
+func init() {
 	log.Silence()
+}
+
+func TestResolver(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "API Suite")
 }
diff --git a/cache/expirationcache/expiration_cache_suite_test.go b/cache/expirationcache/expiration_cache_suite_test.go
@@ -9,8 +9,11 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-func TestCache(t *testing.T) {
+func init() {
 	log.Silence()
+}
+
+func TestCache(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "Expiration cache suite")
 }
diff --git a/cache/stringcache/string_cache_suite_test.go b/cache/stringcache/string_cache_suite_test.go
@@ -9,8 +9,11 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-func TestCache(t *testing.T) {
+func init() {
 	log.Silence()
+}
+
+func TestCache(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "String cache suite")
 }
diff --git a/cache/stringcache/string_caches_benchmark_test.go b/cache/stringcache/string_caches_benchmark_test.go
@@ -33,7 +33,7 @@ var (
 	baseMemStats runtime.MemStats
 )
 
-func init() { //nolint:gochecknoinits
+func init() {
 	// If you update either list, make sure both are the list version (see file header).
 	stringTestData = loadTestdata("../../helpertest/data/oisd-big-plain.txt")
 

diff --git a/cmd/cmd_suite_test.go b/cmd/cmd_suite_test.go
@@ -9,8 +9,11 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-func TestCmd(t *testing.T) {
+func init() {
 	log.Silence()
+}
+
+func TestCmd(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "Command Suite")
 }
diff --git a/config/config_suite_test.go b/config/config_suite_test.go
@@ -14,8 +14,11 @@ var (
 	hook   *log.MockLoggerHook
 )
 
-func TestConfig(t *testing.T) {
+func init() {
 	log.Silence()
+}
+
+func TestConfig(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "Config Suite")
 }

diff --git a/e2e/e2e_suite_test.go b/e2e/e2e_suite_test.go
@@ -14,8 +14,11 @@ import (
 	"github.com/testcontainers/testcontainers-go"
 )
 
-func TestLists(t *testing.T) {
+func init() {
 	log.Silence()
+}
+
+func TestLists(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "e2e Suite", Label("e2e"))
 }

diff --git a/lists/list_suite_test.go b/lists/list_suite_test.go
@@ -9,8 +9,11 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-func TestLists(t *testing.T) {
+func init() {
 	log.Silence()
+}
+
+func TestLists(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "Lists Suite")
 }
diff --git a/lists/parsers/parsers_suite_test.go b/lists/parsers/parsers_suite_test.go
@@ -9,8 +9,11 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-func TestLists(t *testing.T) {
+func init() {
 	log.Silence()
+}
+
+func TestLists(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "Parsers Suite")
 }
diff --git a/log/logger.go b/log/logger.go
@@ -7,6 +7,7 @@ import (
 	"io"
 	"strings"
 	"sync"
+	"sync/atomic"
 
 	"github.com/mattn/go-colorable"
 	"github.com/sirupsen/logrus"
@@ -19,7 +20,10 @@ const prefixField = "prefix"
 // Logger is the global logging instance
 //
 //nolint:gochecknoglobals
-var logger *logrus.Logger
+var (
+	logger   *logrus.Logger
+	initDone atomic.Bool
+)
 
 // FormatType format for logging ENUM(
 // text // logging as text
@@ -47,6 +51,10 @@ type Config struct {
 
 //nolint:gochecknoinits
 func init() {
+	if !initDone.CompareAndSwap(false, true) {
+		return
+	}
+
 	logger = logrus.New()
 
 	defaultConfig := &Config{
@@ -122,7 +130,20 @@ func ConfigureLogger(cfg *Config) {
 
 // Silence disables the logger output
 func Silence() {
-	logger.Out = io.Discard
+	initDone.Store(true)
+
+	logger = logrus.New()
+
+	logger.SetFormatter(nopFormatter{}) // skip expensive formatting
+
+	// not actually needed but doesn't hurt
+	logger.SetOutput(io.Discard)
+}
+
+type nopFormatter struct{}
+
+func (f nopFormatter) Format(*logrus.Entry) ([]byte, error) {
+	return nil, nil
 }
 
 func WithIndent(log *logrus.Entry, prefix string, callback func(*logrus.Entry)) {

diff --git a/log/mock_entry.go b/log/mock_entry.go
@@ -1,15 +1,13 @@
 package log
 
 import (
-	"io"
-
 	"github.com/sirupsen/logrus"
+	"github.com/sirupsen/logrus/hooks/test"
 	"github.com/stretchr/testify/mock"
 )
 
 func NewMockEntry() (*logrus.Entry, *MockLoggerHook) {
-	logger := logrus.New()
-	logger.Out = io.Discard
+	logger, _ := test.NewNullLogger()
 	logger.Level = logrus.TraceLevel
 
 	entry := logrus.Entry{Logger: logger}

diff --git a/querylog/querylog_suite_test.go b/querylog/querylog_suite_test.go
@@ -9,8 +9,11 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-func TestResolver(t *testing.T) {
+func init() {
 	log.Silence()
+}
+
+func TestResolver(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "Querylog Suite")
 }
diff --git a/redis/redis_suite_test.go b/redis/redis_suite_test.go
@@ -10,9 +10,12 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-func TestRedisClient(t *testing.T) {
+func init() {
 	log.Silence()
 	redis.SetLogger(NoLogs{})
+}
+
+func TestRedisClient(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "Redis Suite")
 }

diff --git a/resolver/resolver_suite_test.go b/resolver/resolver_suite_test.go
@@ -12,9 +12,12 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-func TestResolver(t *testing.T) {
+func init() {
 	log.Silence()
 	redis.SetLogger(NoLogs{})
+}
+
+func TestResolver(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "Resolver Suite")
 }

diff --git a/resolver/upstream_tree_resolver.go b/resolver/upstream_tree_resolver.go
@@ -104,7 +104,7 @@ func (r *UpstreamTreeResolver) upstreamGroupByClient(request *model.Request) str
 
 	if len(groups) > 0 {
 		if len(groups) > 1 {
-			r.log().WithFields(logrus.Fields{
+			request.Log.WithFields(logrus.Fields{
 				"clientNames": request.ClientNames,
 				"clientIP":    clientIP,
 				"groups":      groups,

diff --git a/resolver/upstream_tree_resolver_test.go b/resolver/upstream_tree_resolver_test.go
@@ -9,7 +9,6 @@ import (
 	"github.com/miekg/dns"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	"github.com/sirupsen/logrus/hooks/test"
 	"github.com/stretchr/testify/mock"
 )
 
@@ -21,8 +20,6 @@ var _ = Describe("UpstreamTreeResolver", Label("upstreamTreeResolver"), func() {
 		sutConfig config.UpstreamsConfig
 		branches  map[string]Resolver
 
-		loggerHook *test.Hook
-
 		err error
 	)
 
@@ -143,9 +140,6 @@ var _ = Describe("UpstreamTreeResolver", Label("upstreamTreeResolver"), func() {
 
 		When("client specific resolvers are defined", func() {
 			BeforeEach(func() {
-				loggerHook = test.NewGlobal()
-				log.Log().AddHook(loggerHook)
-
 				sutConfig = config.UpstreamsConfig{Groups: config.UpstreamGroups{
 					upstreamDefaultCfgName: {config.Upstream{}},
 					"laptop":               {config.Upstream{}},
@@ -194,10 +188,6 @@ var _ = Describe("UpstreamTreeResolver", Label("upstreamTreeResolver"), func() {
 				Expect(branches).To(HaveLen(8))
 			})
 
-			AfterEach(func() {
-				loggerHook.Reset()
-			})
-
 			It("Should use default if client name or IP don't match", func() {
 				request := newRequestWithClient("example.com.", A, "192.168.178.55", "test")
 
@@ -298,7 +288,10 @@ var _ = Describe("UpstreamTreeResolver", Label("upstreamTreeResolver"), func() {
 						))
 			})
 			It("Should use one of the matching resolvers & log warning", func() {
+				logger, hook := log.NewMockEntry()
+
 				request := newRequestWithClient("example.com.", A, "0.0.0.0", "name-matches1")
+				request.Log = logger
 
 				Expect(sut.Resolve(request)).
 					Should(
@@ -311,7 +304,7 @@ var _ = Describe("UpstreamTreeResolver", Label("upstreamTreeResolver"), func() {
 							HaveReturnCode(dns.RcodeSuccess),
 						))
 
-				Expect(loggerHook.LastEntry().Message).Should(ContainSubstring("client matches multiple groups"))
+				Expect(hook.Messages).Should(ContainElement(ContainSubstring("client matches multiple groups")))
 			})
 		})
 	})

diff --git a/server/server.go b/server/server.go
@@ -115,8 +115,6 @@ func retrieveCertificate(cfg *config.Config) (cert tls.Certificate, err error) {
 //
 //nolint:funlen
 func NewServer(ctx context.Context, cfg *config.Config) (server *Server, err error) {
-	log.ConfigureLogger(&cfg.Log)
-
 	var cert tls.Certificate
 
 	if len(cfg.Ports.HTTPS) > 0 || len(cfg.Ports.TLS) > 0 {

diff --git a/server/server_suite_test.go b/server/server_suite_test.go
@@ -8,8 +8,11 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-func TestDNSServer(t *testing.T) {
+func init() {
 	log.Silence()
+}
+
+func TestDNSServer(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "Server Suite")
 }
diff --git a/trie/trie_suite_test.go b/trie/trie_suite_test.go
@@ -9,8 +9,11 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-func TestTrie(t *testing.T) {
+func init() {
 	log.Silence()
+}
+
+func TestTrie(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "Trie Suite")
 }
diff --git a/util/util_suite_test.go b/util/util_suite_test.go
@@ -8,8 +8,11 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-func TestLists(t *testing.T) {
+func init() {
 	log.Silence()
+}
+
+func TestLists(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "Util Suite")
 }