diff --git a/config/config.go b/config/config.go index 4d4a807513d01..2319d953286c7 100644 --- a/config/config.go +++ b/config/config.go @@ -79,7 +79,7 @@ var ( // checkBeforeDropLDFlag is a go build flag. checkBeforeDropLDFlag = "None" // tempStorageDirName is the default temporary storage dir name by base64 encoding a string `port/statusPort` - tempStorageDirName = EncodeDefTempStorageDir(os.TempDir(), DefHost, DefStatusHost, DefPort, DefStatusPort) + tempStorageDirName = encodeDefTempStorageDir(os.TempDir(), DefHost, DefStatusHost, DefPort, DefStatusPort) ) // Config contains configuration options. @@ -197,9 +197,9 @@ type Config struct { // and the `tmp-storage-path` was not specified in the conf.toml or was specified the same as the default value. func (c *Config) UpdateTempStoragePath() { if c.TempStoragePath == tempStorageDirName { - c.TempStoragePath = EncodeDefTempStorageDir(os.TempDir(), c.Host, c.Status.StatusHost, c.Port, c.Status.StatusPort) + c.TempStoragePath = encodeDefTempStorageDir(os.TempDir(), c.Host, c.Status.StatusHost, c.Port, c.Status.StatusPort) } else { - c.TempStoragePath = EncodeDefTempStorageDir(c.TempStoragePath, c.Host, c.Status.StatusHost, c.Port, c.Status.StatusPort) + c.TempStoragePath = encodeDefTempStorageDir(c.TempStoragePath, c.Host, c.Status.StatusHost, c.Port, c.Status.StatusPort) } } @@ -220,8 +220,7 @@ func (c *Config) getTiKVConfig() *tikvcfg.Config { } } -// EncodeDefTempStorageDir encodes the storage temporary directory. -func EncodeDefTempStorageDir(tempDir string, host, statusHost string, port, statusPort uint) string { +func encodeDefTempStorageDir(tempDir string, host, statusHost string, port, statusPort uint) string { dirName := base64.URLEncoding.EncodeToString([]byte(fmt.Sprintf("%v:%v/%v:%v", host, port, statusHost, statusPort))) var osUID string currentUser, err := user.Current() @@ -246,9 +245,6 @@ var ( nbTrue = nullableBool{true, true} ) -// NullableBoolForTest is exported for tests. -type NullableBoolForTest = nullableBool - func (b *nullableBool) toBool() bool { return b.IsValid && b.IsTrue } @@ -812,8 +808,7 @@ var deprecatedConfig = map[string]struct{}{ "performance.mem-profile-interval": {}, } -// IsAllDeprecatedConfigItems checks whether the items are all deprecated. -func IsAllDeprecatedConfigItems(items []string) bool { +func isAllDeprecatedConfigItems(items []string) bool { for _, item := range items { if _, ok := deprecatedConfig[item]; !ok { return false @@ -845,7 +840,7 @@ func InitializeConfig(confPath string, configCheck, configStrict bool, enforceCm // is not the default behavior of TiDB. The warning message must be deferred until // logging has been set up. After strict config checking is the default behavior, // This should all be removed. - if (!configCheck && !configStrict) || IsAllDeprecatedConfigItems(tmp.UndecodedItems) { + if (!configCheck && !configStrict) || isAllDeprecatedConfigItems(tmp.UndecodedItems) { fmt.Fprintln(os.Stderr, err.Error()) err = nil } @@ -1071,11 +1066,6 @@ func init() { initByLDFlags(versioninfo.TiDBEdition, checkBeforeDropLDFlag) } -// InitByLDFlagsForTest is exported only for tests. -func InitByLDFlagsForTest(edition, checkBeforeDropLDFlag string) { - initByLDFlags(edition, checkBeforeDropLDFlag) -} - func initByLDFlags(edition, checkBeforeDropLDFlag string) { if edition != versioninfo.CommunityEdition { defaultConf.EnableTelemetry = false diff --git a/config/config_test.go b/config/config_test.go index f9f79eca6822a..25c79dc40ebe3 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package config_test +package config import ( "bytes" @@ -27,20 +27,11 @@ import ( "github.com/BurntSushi/toml" zaplog "github.com/pingcap/log" - . "github.com/pingcap/tidb/config" "github.com/pingcap/tidb/util/logutil" "github.com/stretchr/testify/require" tracing "github.com/uber/jaeger-client-go/config" ) -var ( - nbUnset = NullableBoolForTest{false, false} - nbFalse = NullableBoolForTest{true, false} - nbTrue = NullableBoolForTest{true, true} - - tempStorageDirName = EncodeDefTempStorageDir(os.TempDir(), DefHost, DefStatusHost, DefPort, DefStatusPort) -) - func TestAtomicBoolUnmarshal(t *testing.T) { type data struct { Ab AtomicBool `toml:"ab"` @@ -67,21 +58,21 @@ func TestAtomicBoolUnmarshal(t *testing.T) { } func TestNullableBoolUnmarshal(t *testing.T) { - var nb = NullableBoolForTest{false, false} + var nb = nullableBool{false, false} data, err := json.Marshal(nb) require.NoError(t, err) err = json.Unmarshal(data, &nb) require.NoError(t, err) require.Equal(t, nbUnset, nb) - nb = NullableBoolForTest{true, false} + nb = nullableBool{true, false} data, err = json.Marshal(nb) require.NoError(t, err) err = json.Unmarshal(data, &nb) require.NoError(t, err) require.Equal(t, nbFalse, nb) - nb = NullableBoolForTest{true, true} + nb = nullableBool{true, true} data, err = json.Marshal(nb) require.NoError(t, err) err = json.Unmarshal(data, &nb) @@ -125,8 +116,8 @@ func TestLogConfig(t *testing.T) { require.NoError(t, os.Remove(configFile)) }() - var testLoad = func(confStr string, expectedEnableErrorStack, expectedDisableErrorStack, expectedEnableTimestamp, expectedDisableTimestamp NullableBoolForTest, resultedDisableTimestamp, resultedDisableErrorVerbose bool) { - conf = *NewConfig() + var testLoad = func(confStr string, expectedEnableErrorStack, expectedDisableErrorStack, expectedEnableTimestamp, expectedDisableTimestamp nullableBool, resultedDisableTimestamp, resultedDisableErrorVerbose bool) { + conf = defaultConf _, err = f.WriteString(confStr) require.NoError(t, err) require.NoError(t, conf.Load(configFile)) @@ -352,7 +343,7 @@ mem-profile-interval="1m"`) require.NoError(t, err) err = conf.Load(configFile) tmp := err.(*ErrConfigValidationFailed) - require.True(t, IsAllDeprecatedConfigItems(tmp.UndecodedItems)) + require.True(t, isAllDeprecatedConfigItems(tmp.UndecodedItems)) // Test telemetry config default value and whether it will be overwritten. conf = NewConfig() @@ -400,10 +391,7 @@ spilled-file-encryption-method = "aes128-ctr" require.Equal(t, GetGlobalConfig(), conf) // Test for log config. - require.Equal(t, - logutil.NewLogConfig("info", "text", "tidb-slow.log", conf.Log.File, false, - func(config *zaplog.Config) { config.DisableErrorVerbose = true }), - conf.Log.ToLogConfig()) + require.Equal(t, logutil.NewLogConfig("info", "text", "tidb-slow.log", conf.Log.File, false, func(config *zaplog.Config) { config.DisableErrorVerbose = conf.Log.getDisableErrorStack() }), conf.Log.ToLogConfig()) // Test for tracing config. tracingConf := &tracing.Configuration{ @@ -610,13 +598,12 @@ func TestEncodeDefTempStorageDir(t *testing.T) { dirPrefix := filepath.Join(os.TempDir(), osUID+"_tidb") for _, test := range tests { - tempStorageDir := EncodeDefTempStorageDir(os.TempDir(), test.host, test.statusHost, test.port, test.statusPort) + tempStorageDir := encodeDefTempStorageDir(os.TempDir(), test.host, test.statusHost, test.port, test.statusPort) require.Equal(t, filepath.Join(dirPrefix, test.expect, "tmp-storage"), tempStorageDir) } } func TestModifyThroughLDFlags(t *testing.T) { - originDefaultCfg := NewConfig() tests := []struct { Edition string CheckBeforeDropLDFlag string @@ -629,21 +616,23 @@ func TestModifyThroughLDFlags(t *testing.T) { {"Enterprise", "1", false, true}, } + originalEnableTelemetry := defaultConf.EnableTelemetry originalCheckTableBeforeDrop := CheckTableBeforeDrop originalGlobalConfig := GetGlobalConfig() for _, test := range tests { - originDefaultCfg.EnableTelemetry = true + defaultConf.EnableTelemetry = true CheckTableBeforeDrop = false - InitByLDFlagsForTest(test.Edition, test.CheckBeforeDropLDFlag) + initByLDFlags(test.Edition, test.CheckBeforeDropLDFlag) conf := GetGlobalConfig() require.Equal(t, test.EnableTelemetry, conf.EnableTelemetry) - require.Equal(t, test.EnableTelemetry, NewConfig().EnableTelemetry) + require.Equal(t, test.EnableTelemetry, defaultConf.EnableTelemetry) require.Equal(t, test.CheckTableBeforeDrop, CheckTableBeforeDrop) } + defaultConf.EnableTelemetry = originalEnableTelemetry CheckTableBeforeDrop = originalCheckTableBeforeDrop StoreGlobalConfig(originalGlobalConfig) } diff --git a/config/config_util.go b/config/config_util.go index 5f25e64bac1e5..f8ae5221c2617 100644 --- a/config/config_util.go +++ b/config/config_util.go @@ -62,12 +62,6 @@ var ( } ) -// IsDynamicConfigItems checks whether an item is a dynamic config item. -func IsDynamicConfigItems(item string) bool { - _, ok := dynamicConfigItems[item] - return ok -} - // MergeConfigItems overwrites the dynamic config items and leaves the other items unchanged. func MergeConfigItems(dstConf, newConf *Config) (acceptedItems, rejectedItems []string) { return mergeConfigItems(reflect.ValueOf(dstConf), reflect.ValueOf(newConf), "") diff --git a/config/config_util_test.go b/config/config_util_test.go index 97eae9d0cb0e6..f23c47f8dd796 100644 --- a/config/config_util_test.go +++ b/config/config_util_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package config_test +package config import ( "encoding/json" @@ -21,12 +21,11 @@ import ( "testing" "github.com/BurntSushi/toml" - . "github.com/pingcap/tidb/config" "github.com/stretchr/testify/require" ) func TestCloneConf(t *testing.T) { - c1, err := CloneConf(NewConfig()) + c1, err := CloneConf(&defaultConf) require.NoError(t, err) c2, err := CloneConf(c1) require.NoError(t, err) @@ -43,7 +42,7 @@ func TestCloneConf(t *testing.T) { } func TestMergeConfigItems(t *testing.T) { - oriConf, _ := CloneConf(NewConfig()) + oriConf, _ := CloneConf(&defaultConf) oldConf, _ := CloneConf(oriConf) newConf, _ := CloneConf(oldConf) @@ -68,10 +67,12 @@ func TestMergeConfigItems(t *testing.T) { require.Equal(t, 10, len(as)) require.Equal(t, 3, len(rs)) for _, a := range as { - require.True(t, IsDynamicConfigItems(a)) + _, ok := dynamicConfigItems[a] + require.True(t, ok) } for _, a := range rs { - require.False(t, IsDynamicConfigItems(a)) + _, ok := dynamicConfigItems[a] + require.False(t, ok) } require.Equal(t, newConf.Performance.MaxProcs, oldConf.Performance.MaxProcs) diff --git a/config/main_test.go b/config/main_test.go index f425725402d31..847d12a0a12ce 100644 --- a/config/main_test.go +++ b/config/main_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package config_test +package config import ( "testing" diff --git a/util/logutil/hex.go b/util/logutil/hex.go index 9e8c3d4467e24..2794fa87a206c 100644 --- a/util/logutil/hex.go +++ b/util/logutil/hex.go @@ -38,12 +38,11 @@ type hexStringer struct { func (h hexStringer) String() string { val := reflect.ValueOf(h.Message) var w bytes.Buffer - PrettyPrint(&w, val) + prettyPrint(&w, val) return w.String() } -// PrettyPrint prints a hex value according to its reflect type. -func PrettyPrint(w io.Writer, val reflect.Value) { +func prettyPrint(w io.Writer, val reflect.Value) { tp := val.Type() switch val.Kind() { case reflect.Slice: @@ -65,14 +64,14 @@ func PrettyPrint(w io.Writer, val reflect.Value) { fmt.Fprintf(w, " ") } fmt.Fprintf(w, "%s:", ft.Name) - PrettyPrint(w, fv) + prettyPrint(w, fv) } fmt.Fprintf(w, "}") case reflect.Ptr: if val.IsNil() { fmt.Fprintf(w, "%v", val.Interface()) } else { - PrettyPrint(w, reflect.Indirect(val)) + prettyPrint(w, reflect.Indirect(val)) } default: fmt.Fprintf(w, "%v", val.Interface()) diff --git a/util/logutil/log.go b/util/logutil/log.go index 21eda699e4ac2..a9e7efa6a0b78 100644 --- a/util/logutil/log.go +++ b/util/logutil/log.go @@ -119,11 +119,6 @@ func InitLogger(cfg *LogConfig) error { return nil } -// InitGRPCLoggerForTest is exported for test. -func InitGRPCLoggerForTest(cfg *LogConfig) (*zap.Logger, *log.ZapProperties, error) { - return initGRPCLogger(cfg) -} - func initGRPCLogger(cfg *LogConfig) (*zap.Logger, *log.ZapProperties, error) { // Copy Config struct by assignment. config := cfg.Config diff --git a/util/logutil/log_test.go b/util/logutil/log_test.go index c065f5a55d937..69eb731dee8c6 100644 --- a/util/logutil/log_test.go +++ b/util/logutil/log_test.go @@ -12,10 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//go:build !codes -// +build !codes - -package logutil_test +package logutil import ( "bufio" @@ -26,7 +23,6 @@ import ( "testing" "github.com/pingcap/log" - . "github.com/pingcap/tidb/util/logutil" "github.com/stretchr/testify/require" "go.uber.org/zap" "go.uber.org/zap/zapcore" @@ -107,14 +103,14 @@ func TestSetLevel(t *testing.T) { func TestGrpcLoggerCreation(t *testing.T) { level := "info" conf := NewLogConfig(level, DefaultLogFormat, "", EmptyFileLogConfig, false) - _, p, err := InitGRPCLoggerForTest(conf) + _, p, err := initGRPCLogger(conf) // assert after init grpc logger, the original conf is not changed require.Equal(t, conf.Level, level) require.Nil(t, err) require.Equal(t, p.Level.Level(), zap.ErrorLevel) os.Setenv("GRPC_DEBUG", "1") defer os.Unsetenv("GRPC_DEBUG") - _, newP, err := InitGRPCLoggerForTest(conf) + _, newP, err := initGRPCLogger(conf) require.Nil(t, err) require.Equal(t, newP.Level.Level(), zap.DebugLevel) } @@ -122,7 +118,7 @@ func TestGrpcLoggerCreation(t *testing.T) { func TestSlowQueryLoggerCreation(t *testing.T) { level := "Error" conf := NewLogConfig(level, DefaultLogFormat, "", EmptyFileLogConfig, false) - _, prop, err := NewSlowQueryLoggerForTest(conf) + _, prop, err := newSlowQueryLogger(conf) // assert after init slow query logger, the original conf is not changed require.Equal(t, conf.Level, level) require.Nil(t, err) @@ -142,7 +138,7 @@ func TestSlowQueryLoggerCreation(t *testing.T) { }, } conf = NewLogConfig(level, DefaultLogFormat, slowQueryFn, fileConf, false) - slowQueryConf := NewSlowQueryLogConfigForTest(conf) + slowQueryConf := newSlowQueryLogConfig(conf) // slowQueryConf.MaxDays/MaxSize/MaxBackups should be same with global config. require.Equal(t, fileConf.FileLogConfig, slowQueryConf.File) } diff --git a/util/logutil/main_test.go b/util/logutil/main_test.go index 5097a3e9b3fa2..0113d6f75ee6c 100644 --- a/util/logutil/main_test.go +++ b/util/logutil/main_test.go @@ -12,10 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//go:build !codes -// +build !codes - -package logutil_test +package logutil import ( "testing" @@ -32,6 +29,10 @@ const ( zapLogWithKeyValPattern = `\[\d\d\d\d/\d\d/\d\d \d\d:\d\d:\d\d.\d\d\d\ (\+|-)\d\d:\d\d\] \[(FATAL|ERROR|WARN|INFO|DEBUG)\] \[([\w_%!$@.,+~-]+|\\.)+:\d+\] \[.*\] \[ctxKey=.*\] (\[.*=.*\]).*\n` ) +var ( + PrettyPrint = prettyPrint +) + func TestMain(m *testing.M) { testbridge.SetupForCommonTest() opts := []goleak.Option{ diff --git a/util/logutil/slow_query_logger.go b/util/logutil/slow_query_logger.go index 0c41395412a0e..d5d59f98aface 100644 --- a/util/logutil/slow_query_logger.go +++ b/util/logutil/slow_query_logger.go @@ -27,11 +27,6 @@ import ( var _pool = buffer.NewPool() -// NewSlowQueryLoggerForTest is exported for tests. -func NewSlowQueryLoggerForTest(cfg *LogConfig) (*zap.Logger, *log.ZapProperties, error) { - return newSlowQueryLogger(cfg) -} - func newSlowQueryLogger(cfg *LogConfig) (*zap.Logger, *log.ZapProperties, error) { // create the slow query logger sqLogger, prop, err := log.InitLogger(newSlowQueryLogConfig(cfg)) @@ -49,11 +44,6 @@ func newSlowQueryLogger(cfg *LogConfig) (*zap.Logger, *log.ZapProperties, error) return sqLogger, prop, nil } -// NewSlowQueryLogConfigForTest is exported for tests. -func NewSlowQueryLogConfigForTest(cfg *LogConfig) *log.Config { - return newSlowQueryLogConfig(cfg) -} - func newSlowQueryLogConfig(cfg *LogConfig) *log.Config { // copy the global log config to slow log config // if the filename of slow log config is empty, slow log will behave the same as global log. diff --git a/util/testbridge/bridge.go b/util/testbridge/bridge.go index 377d3724ad241..a682a6574fa3c 100644 --- a/util/testbridge/bridge.go +++ b/util/testbridge/bridge.go @@ -23,7 +23,6 @@ import ( "os" "github.com/pingcap/log" - "github.com/pingcap/tidb/util/logutil" "go.uber.org/zap" "go.uber.org/zap/zapcore" ) @@ -54,7 +53,7 @@ func applyOSLogLevel() { cfg := log.Config{ Level: osLoglevel, Format: "text", - File: log.FileLogConfig{MaxSize: logutil.DefaultLogMaxSize}, + File: log.FileLogConfig{}, } gl, props, err := log.InitLogger(&cfg, zap.AddStacktrace(zapcore.FatalLevel)) if err != nil {