revert unnecessary changes in config and util

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

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)
 }

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), "")

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)

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"

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())

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

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,22 +103,22 @@ 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)
 }
 
 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)
 }

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{