From d52c598546918827774d64203e863f6213f833eb Mon Sep 17 00:00:00 2001 From: ThinkChaos Date: Sun, 19 Nov 2023 17:51:49 -0500 Subject: [PATCH] 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 --- .golangci.yml | 1 + api/api_suite_test.go | 5 +++- .../expiration_cache_suite_test.go | 5 +++- cache/stringcache/string_cache_suite_test.go | 5 +++- .../string_caches_benchmark_test.go | 2 +- cmd/cmd_suite_test.go | 5 +++- config/config_suite_test.go | 5 +++- e2e/e2e_suite_test.go | 5 +++- lists/list_suite_test.go | 5 +++- lists/parsers/parsers_suite_test.go | 5 +++- log/logger.go | 25 +++++++++++++++++-- log/mock_entry.go | 6 ++--- querylog/querylog_suite_test.go | 5 +++- redis/redis_suite_test.go | 5 +++- resolver/resolver_suite_test.go | 5 +++- resolver/upstream_tree_resolver.go | 2 +- resolver/upstream_tree_resolver_test.go | 15 +++-------- server/server.go | 2 -- server/server_suite_test.go | 5 +++- trie/trie_suite_test.go | 5 +++- util/util_suite_test.go | 5 +++- 21 files changed, 88 insertions(+), 35 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 9f41c7e1..1013d4d5 100644 --- 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 index 48db54eb..168730a3 100644 --- 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 index 0f413594..c19955e1 100644 --- 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 index 661ff43e..c32c9619 100644 --- 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 index 4016316b..97d8fa63 100644 --- 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 index 4c75bbc5..81c5714b 100644 --- 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 index 4b200bab..87ff8868 100644 --- 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 index a0f761ea..a4a6feb7 100644 --- 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 index 83d88910..549abd35 100644 --- 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 index c34db019..b955c954 100644 --- 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 index b685eb52..924b22bf 100644 --- 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 index b79ba4e7..b02207a9 100644 --- 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 index a8faf107..2c041278 100644 --- 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 index 34ee9133..7eba3d7a 100644 --- 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 index 4093fc56..71ce26c2 100644 --- 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 index 72a13795..36eed942 100644 --- 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 index 11faab42..3855761a 100644 --- 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 index bdbb4b17..9105aec7 100644 --- 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 index 2653307e..2c2eea0d 100644 --- 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 index e7c95c2f..0b091865 100644 --- 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 index 2a89a679..184521ca 100644 --- 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") }