From 63b3a3f1bbba00bd042d4f8d3a63d1c7f10e367a Mon Sep 17 00:00:00 2001 From: Domenico Rizzo Date: Thu, 5 Dec 2024 15:40:05 +0100 Subject: [PATCH] Refactored logging tests for better readability The logging tests have been refactored to improve readability and maintainability. The changes include: - Grouping related assertions into subtests using t.Run - Using require instead of assert where appropriate to stop test execution after a failure - Simplifying variable declarations by using the new keyword --- server/logging_test.go | 213 ++++++++++++++++++++++++----------------- 1 file changed, 124 insertions(+), 89 deletions(-) diff --git a/server/logging_test.go b/server/logging_test.go index bdb6b9d..6e55d95 100644 --- a/server/logging_test.go +++ b/server/logging_test.go @@ -14,125 +14,160 @@ import ( type logOptionsTest map[string]string func TestEscapeDoubleQuotesOK(t *testing.T) { - assert.Equal(t, "test", escapeDoubleQuotes("test")) - assert.Equal(t, "\\\"test\\\"", escapeDoubleQuotes("\"test\"")) - assert.Equal(t, "\\\"\\\"test\\\"", escapeDoubleQuotes("\"\"test\"")) - assert.Equal(t, "\\\"\\\"\\\"\\\"", escapeDoubleQuotes("\"\"\"\"")) + t.Run("No quotes", func(t *testing.T) { + assert.Equal(t, "test", escapeDoubleQuotes("test")) + }) + t.Run("One quote", func(t *testing.T) { + assert.Equal(t, "\\\"test", escapeDoubleQuotes("\"test")) + }) + t.Run("Two quotes", func(t *testing.T) { + assert.Equal(t, "\\\"test\\\"", escapeDoubleQuotes("\"test\"")) + }) + t.Run("Three quotes", func(t *testing.T) { + assert.Equal(t, "\\\"\\\"test\\\"", escapeDoubleQuotes("\"\"test\"")) + }) + t.Run("Four quotes", func(t *testing.T) { + assert.Equal(t, "\\\"\\\"\\\"\\\"", escapeDoubleQuotes("\"\"\"\"")) + }) } func TestDefaultLoggingConsoleLogConfigOK(t *testing.T) { - var mapCfgEscaped mlog.LoggerConfiguration - err := json.Unmarshal([]byte(defaultLoggingConsoleLogConfig()), &mapCfgEscaped) - assert.NoError(t, err) - - var logOptionsConsole logOptionsTest - - targetCfg := mapCfgEscaped["def"] - - err = json.Unmarshal(targetCfg.Options, &logOptionsConsole) - - assert.NoError(t, err) - assert.Equal(t, "console", targetCfg.Type) - assert.Equal(t, "plain", targetCfg.Format) - assert.Equal(t, "stdout", logOptionsConsole["out"]) + var mapCfgEscaped = new(mlog.LoggerConfiguration) + var logOptionsConsole = new(logOptionsTest) + + var targetCfg mlog.TargetCfg + + t.Run("Unmarshall base correctly", func(t *testing.T) { + err := json.Unmarshal([]byte(defaultLoggingConsoleLogConfig()), mapCfgEscaped) + require.NoError(t, err) + require.NotNil(t, mapCfgEscaped) + targetCfg = (*mapCfgEscaped)["def"] + assert.Equal(t, "console", targetCfg.Type) + assert.Equal(t, "plain", targetCfg.Format) + }) + + t.Run("Unmarshall options correctly", func(t *testing.T) { + err := json.Unmarshal(targetCfg.Options, logOptionsConsole) + require.NoError(t, err) + require.NotNil(t, logOptionsConsole) + assert.Equal(t, "stdout", (*logOptionsConsole)["out"]) + }) } func TestDefaultLoggingFileLogConfigOK(t *testing.T) { filename := randomString(10) - var mapCfgEscaped mlog.LoggerConfiguration - err := json.Unmarshal([]byte(defaultLoggingFileLogConfig(filename)), &mapCfgEscaped) - assert.NoError(t, err) - - var logOptionsConsole logOptionsTest - - targetCfg := mapCfgEscaped["def"] - - err = json.Unmarshal(targetCfg.Options, &logOptionsConsole) - - assert.NoError(t, err) - assert.Equal(t, "file", targetCfg.Type) - assert.Equal(t, "plain", targetCfg.Format) - assert.Equal(t, filename, logOptionsConsole["filename"]) + var mapCfgEscaped = new(mlog.LoggerConfiguration) + var logOptionsConsole = new(logOptionsTest) + + var targetCfg mlog.TargetCfg + + t.Run("Unmarshall base correctly", func(t *testing.T) { + err := json.Unmarshal([]byte(defaultLoggingFileLogConfig(filename)), mapCfgEscaped) + require.NoError(t, err) + require.NotNil(t, mapCfgEscaped) + targetCfg = (*mapCfgEscaped)["def"] + assert.Equal(t, "file", targetCfg.Type) + assert.Equal(t, "plain", targetCfg.Format) + }) + + t.Run("Unmarshall options correctly", func(t *testing.T) { + err := json.Unmarshal(targetCfg.Options, logOptionsConsole) + require.NoError(t, err) + require.NotNil(t, logOptionsConsole) + assert.Equal(t, filename, (*logOptionsConsole)["filename"]) + }) } func TestDefaultLoggingConfigOK(t *testing.T) { - cfg := &ConfigPushProxy{ - EnableFileLog: false, - } - assert.Equal(t, defaultLoggingConsoleLogConfig(), defaultLoggingConfig(cfg)) - - cfg = &ConfigPushProxy{ - EnableFileLog: true, - LogFileLocation: randomString(10), - } - assert.Equal(t, defaultLoggingFileLogConfig(cfg.LogFileLocation), defaultLoggingConfig(cfg)) + t.Run("Console config is get correctly", func(t *testing.T) { + cfg := &ConfigPushProxy{ + EnableFileLog: false, + } + assert.Equal(t, defaultLoggingConsoleLogConfig(), defaultLoggingConfig(cfg)) + }) + + t.Run("File config is get correctly", func(t *testing.T) { + cfg := &ConfigPushProxy{ + EnableFileLog: true, + LogFileLocation: randomString(10), + } + assert.Equal(t, defaultLoggingFileLogConfig(cfg.LogFileLocation), defaultLoggingConfig(cfg)) + }) } func TestNewMlogLoggerConsoleLegacyOK(t *testing.T) { - cfg := &ConfigPushProxy{ - EnableFileLog: false, - } - logger, err := NewMlogLogger(cfg) - assert.NoError(t, err) - assert.NotNil(t, logger) + t.Run("Instancing logger with console for legacy conf", func(t *testing.T) { + cfg := &ConfigPushProxy{ + EnableFileLog: false, + } + logger, err := NewMlogLogger(cfg) + assert.NoError(t, err) + assert.NotNil(t, logger) + }) } func TestNewMlogLoggerFileLegacyOk(t *testing.T) { - log, err := os.CreateTemp("", "log") - require.NoError(t, err) - - err = log.Close() - require.NoError(t, err) - defer os.Remove(log.Name()) - - cfg := &ConfigPushProxy{ - EnableFileLog: true, - LogFileLocation: log.Name(), - } - - logger, err := NewMlogLogger(cfg) - assert.NoError(t, err) - assert.NotNil(t, logger) + t.Run("Instancing logger with file for legacy conf", func(t *testing.T) { + log, err := os.CreateTemp("", "log") + require.NoError(t, err) + + err = log.Close() + require.NoError(t, err) + defer os.Remove(log.Name()) + + cfg := &ConfigPushProxy{ + EnableFileLog: true, + LogFileLocation: log.Name(), + } + + logger, err := NewMlogLogger(cfg) + assert.NoError(t, err) + assert.NotNil(t, logger) + }) } func TestNewMlogLoggerLoggingCfgFileOk(t *testing.T) { - conf, err := os.CreateTemp("", "logget-cfg-conf.json") - require.NoError(t, err) + t.Run("Instancing logger with file", func(t *testing.T) { + conf, err := os.CreateTemp("", "logget-cfg-conf.json") + require.NoError(t, err) - _, err = conf.WriteString(defaultLoggingConsoleLogConfig()) - require.NoError(t, err) + _, err = conf.WriteString(defaultLoggingConsoleLogConfig()) + require.NoError(t, err) - err = conf.Close() - require.NoError(t, err) - defer os.Remove(conf.Name()) + err = conf.Close() + require.NoError(t, err) + defer os.Remove(conf.Name()) - cfg := &ConfigPushProxy{ - LoggingCfgFile: conf.Name(), - } + cfg := &ConfigPushProxy{ + LoggingCfgFile: conf.Name(), + } - logger, err := NewMlogLogger(cfg) - assert.NoError(t, err) - assert.NotNil(t, logger) + logger, err := NewMlogLogger(cfg) + assert.NoError(t, err) + assert.NotNil(t, logger) + }) } func TestNewMlogLoggerLoggingCfgJSONOk(t *testing.T) { - conf, err := os.CreateTemp("", "logget-cfg-conf.json") - require.NoError(t, err) + t.Run("Instancing logger with json", func(t *testing.T) { + conf, err := os.CreateTemp("", "logget-cfg-conf.json") + require.NoError(t, err) - _, err = conf.WriteString(defaultLoggingConsoleLogConfig()) - require.NoError(t, err) + _, err = conf.WriteString(defaultLoggingConsoleLogConfig()) + require.NoError(t, err) - err = conf.Close() - require.NoError(t, err) - defer os.Remove(conf.Name()) + err = conf.Close() + require.NoError(t, err) + defer os.Remove(conf.Name()) - cfg := &ConfigPushProxy{ - LoggingCfgJSON: defaultLoggingConsoleLogConfig(), - } + cfg := &ConfigPushProxy{ + LoggingCfgJSON: defaultLoggingConsoleLogConfig(), + } - logger, err := NewMlogLogger(cfg) - assert.NoError(t, err) - assert.NotNil(t, logger) + logger, err := NewMlogLogger(cfg) + assert.NoError(t, err) + assert.NotNil(t, logger) + }) } func randomString(length int) string {