Copilot commented on code in PR #2978:
URL: https://github.com/apache/dubbo-go/pull/2978#discussion_r2312491054
##########
config/logger_config_test.go:
##########
@@ -27,6 +27,11 @@ import (
"github.com/stretchr/testify/assert"
)
+import (
+ cfgcenter "dubbo.apache.org/dubbo-go/v3/config_center"
+ dlogger "dubbo.apache.org/dubbo-go/v3/logger"
+)
Review Comment:
The second import block should be merged with the existing import block
above (lines 25-28) to follow Go's standard import grouping conventions.
##########
config/logger_config_test.go:
##########
@@ -83,3 +88,62 @@ func TestNewLoggerConfigBuilder(t *testing.T) {
assert.Equal(t, *config.File.Compress, true)
assert.Equal(t, config.File.MaxBackups, 5)
}
+
+func TestLoggerDynamicUpdateLevel(t *testing.T) {
+ // load initial config from bytes
+ initialYAML := `
+ dubbo:
+ logger:
+ driver: zap
+ level: info
+ `
+ err := Load(WithBytes([]byte(initialYAML)))
+ assert.Nil(t, err)
+ assert.NotNil(t, rootConfig)
+ assert.NotNil(t, rootConfig.Logger)
+ assert.Equal(t, "info", rootConfig.Logger.Level)
+
+ // if runtime logger doesn't support dynamic level, skip this test
+ if !dlogger.SetLoggerLevel(rootConfig.Logger.Level) {
Review Comment:
Using `SetLoggerLevel` with the current level ('info') to test dynamic level
support is incorrect. This should test with a different level or use a
dedicated method to check if dynamic level changes are supported, as setting
the same level may always return true regardless of actual dynamic support.
```suggestion
// test dynamic level support by attempting to set a different level
if !dlogger.SetLoggerLevel("debug") {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
