Copilot commented on code in PR #3653:
URL: https://github.com/apache/dubbo-go/pull/3653#discussion_r3774264693
##########
registry/options_test.go:
##########
@@ -38,39 +33,197 @@ func TestNewOptionsRequireProtocol(t *testing.T) {
func TestNewOptionsWithHelpers(t *testing.T) {
tests := []struct {
- name string
- opts []Option
- wantProtocol string
- wantID string
- wantTimeout string
- wantAddress string
+ name string
+ opts []Option
+ wantProtocol string
+ wantID string
+ wantTimeout string
+ wantAddress string
+ wantTTL string
+ wantGroup string
+ wantNamespace string
+ wantUsername string
+ wantPassword string
+ wantSimplified bool
+ wantPreferred bool
+ wantZone string
+ wantWeight int64
+ wantParams map[string]string
+ wantRegistryType string
+ wantUseAsMetaReport string
+ wantUseAsConfigCenter string
Review Comment:
The test table struct is getting very wide, which makes it easy to miss
assertions or accidentally introduce gaps when new option fields are added.
Consider replacing the many `want*` fields with a single `wantRegistry
global.RegistryConfig` (or a smaller “expected” struct mirroring only the
relevant fields) and comparing it in one place (optionally after applying
defaults/normalization), to keep the test scalable and easier to extend.
##########
registry/options_test.go:
##########
@@ -84,6 +237,41 @@ func TestNewOptionsWithHelpers(t *testing.T) {
if tt.wantAddress != "" {
assert.Equal(t, tt.wantAddress,
options.Registry.Address)
}
+ if tt.wantTTL != "" {
+ assert.Equal(t, tt.wantTTL,
options.Registry.TTL)
+ }
+ if tt.wantGroup != "" {
+ assert.Equal(t, tt.wantGroup,
options.Registry.Group)
+ }
+ if tt.wantNamespace != "" {
+ assert.Equal(t, tt.wantNamespace,
options.Registry.Namespace)
+ }
+ if tt.wantUsername != "" {
+ assert.Equal(t, tt.wantUsername,
options.Registry.Username)
+ }
+ if tt.wantPassword != "" {
+ assert.Equal(t, tt.wantPassword,
options.Registry.Password)
+ }
+ assert.Equal(t, tt.wantSimplified,
options.Registry.Simplified)
+ assert.Equal(t, tt.wantPreferred,
options.Registry.Preferred)
+ if tt.wantZone != "" {
+ assert.Equal(t, tt.wantZone,
options.Registry.Zone)
+ }
+ if tt.wantWeight != 0 {
+ assert.Equal(t, tt.wantWeight,
options.Registry.Weight)
+ }
+ if tt.wantParams != nil {
+ assert.Equal(t, tt.wantParams,
options.Registry.Params)
+ }
+ if tt.wantRegistryType != "" {
+ assert.Equal(t, tt.wantRegistryType,
options.Registry.RegistryType)
+ }
+ if tt.wantUseAsMetaReport != "" {
+ assert.Equal(t, tt.wantUseAsMetaReport,
options.Registry.UseAsMetaReport)
+ }
+ if tt.wantUseAsConfigCenter != "" {
+ assert.Equal(t, tt.wantUseAsConfigCenter,
options.Registry.UseAsConfigCenter)
+ }
Review Comment:
Using zero values as “don’t assert” sentinels prevents testing legitimate
boundary cases (e.g., `Weight == 0`, `Params == nil` vs empty map, or fields
intentionally set to empty string). To cover the claimed “边界路径”, use explicit
“assert flags” (e.g., `assertWeight bool`) or pointer-based expectations (e.g.,
`*int64`, `*string`, `*map[string]string`) so tests can assert zero/empty
values when desired.
##########
registry/options_test.go:
##########
@@ -20,14 +20,9 @@ package registry
import (
"testing"
"time"
-)
-
-import (
- "github.com/stretchr/testify/assert"
-)
-import (
"dubbo.apache.org/dubbo-go/v3/common/constant"
+ "github.com/stretchr/testify/assert"
)
Review Comment:
This import block mixes standard library and third-party imports in the same
group. While `gofmt` accepts this, typical Go style (and `goimports`) groups
stdlib separate from non-stdlib imports; consider running `goimports` (or
adding a blank line) to keep imports consistent with common Go conventions.
--
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]
