Copilot commented on code in PR #3130:
URL: https://github.com/apache/dubbo-go/pull/3130#discussion_r2638198445
##########
common/rpc_service_test.go:
##########
@@ -251,3 +251,387 @@ func TestGetReference(t *testing.T) {
ref5 := GetReference(s5)
assert.Equal(t, expectedReference, ref5)
}
+
+// Additional tests for better coverage
+
+func TestServiceMethods(t *testing.T) {
+ s := &TestService{}
+ _, err := ServiceMap.Register("TestServiceMethods", testProtocol,
"group1", "v1", s)
+ assert.NoError(t, err)
+
+ service := ServiceMap.GetService(testProtocol, "TestServiceMethods",
"group1", "v1")
+ assert.NotNil(t, service)
+
+ // Test Service.Method()
+ methods := service.Method()
+ assert.NotNil(t, methods)
+ assert.True(t, len(methods) > 0)
+
+ // Test Service.Name()
+ assert.Equal(t, "group1/TestServiceMethods:v1", service.Name())
+
+ // Test Service.ServiceType()
+ svcType := service.ServiceType()
+ assert.NotNil(t, svcType)
+ assert.Equal(t, "*common.TestService", svcType.String())
+
+ // Test Service.Service()
+ svcValue := service.Service()
+ assert.True(t, svcValue.IsValid())
+
+ // Cleanup
+ ServiceMap.UnRegister("TestServiceMethods", testProtocol,
ServiceKey("TestServiceMethods", "group1", "v1"))
+}
+
+func TestGetServiceByServiceKey(t *testing.T) {
+ s := &TestService{}
+ _, err := ServiceMap.Register("TestGetServiceByKey", testProtocol, "",
"v1", s)
+ assert.NoError(t, err)
+
+ // Test GetServiceByServiceKey - found
+ serviceKey := ServiceKey("TestGetServiceByKey", "", "v1")
+ service := ServiceMap.GetServiceByServiceKey(testProtocol, serviceKey)
+ assert.NotNil(t, service)
+
+ // Test GetServiceByServiceKey - protocol not found
+ service = ServiceMap.GetServiceByServiceKey("nonexistent", serviceKey)
+ assert.Nil(t, service)
+
+ // Test GetServiceByServiceKey - service key not found
+ service = ServiceMap.GetServiceByServiceKey(testProtocol,
"nonexistent:v1")
+ assert.Nil(t, service)
+
+ // Cleanup
+ ServiceMap.UnRegister("TestGetServiceByKey", testProtocol, serviceKey)
+}
+
+func TestGetInterface(t *testing.T) {
+ s := &TestService{}
+ _, err := ServiceMap.Register("TestGetInterface", testProtocol, "",
"v1", s)
+ assert.NoError(t, err)
+
+ // Test GetInterface - found
+ services := ServiceMap.GetInterface("TestGetInterface")
+ assert.NotNil(t, services)
+ assert.Equal(t, 1, len(services))
+
+ // Test GetInterface - not found
+ services = ServiceMap.GetInterface("nonexistent")
+ assert.Nil(t, services)
+
+ // Cleanup
+ ServiceMap.UnRegister("TestGetInterface", testProtocol,
ServiceKey("TestGetInterface", "", "v1"))
+}
+
+func TestMethodTypeSuiteContextInvalid(t *testing.T) {
+ mt := &MethodType{ctxType:
reflect.TypeOf((*context.Context)(nil)).Elem()}
+
+ // Test with nil context (invalid)
+ var nilCtx context.Context = nil
+ result := mt.SuiteContext(nilCtx)
+ assert.True(t, result.IsValid())
+ assert.True(t, result.IsZero())
+}
+
+func TestIsExported(t *testing.T) {
+ assert.True(t, isExported("Exported"))
+ assert.True(t, isExported("A"))
+ assert.False(t, isExported("unexported"))
+ assert.False(t, isExported("a"))
+ assert.False(t, isExported(""))
+}
+
+func TestIsExportedOrBuiltinType(t *testing.T) {
+ // Exported type
+ assert.True(t, isExportedOrBuiltinType(reflect.TypeOf(TestService{})))
+
+ // Pointer to exported type
+ assert.True(t, isExportedOrBuiltinType(reflect.TypeOf(&TestService{})))
+
+ // Builtin type (string)
+ assert.True(t, isExportedOrBuiltinType(reflect.TypeOf("")))
+
+ // Builtin type (int)
+ assert.True(t, isExportedOrBuiltinType(reflect.TypeOf(0)))
+
+ // Pointer to builtin
+ var i int
+ assert.True(t, isExportedOrBuiltinType(reflect.TypeOf(&i)))
+
+ // Unexported type
+ assert.False(t, isExportedOrBuiltinType(reflect.TypeOf(testService{})))
+}
+
+// Test service with XXX prefix methods (should be skipped)
+type TestServiceWithXXX struct{}
+
+func (s *TestServiceWithXXX) XXX_InterfaceName() string {
+ return "test"
+}
+
+func (s *TestServiceWithXXX) NormalMethod(ctx context.Context) error {
+ return nil
+}
+
+func (s *TestServiceWithXXX) Reference() string {
+ return "TestServiceWithXXX"
+}
+
+func TestSuiteMethodSkipsXXXPrefix(t *testing.T) {
+ s := &TestServiceWithXXX{}
+ sType := reflect.TypeOf(s)
+
+ // XXX_ prefixed method should be skipped
+ method, ok := sType.MethodByName("XXX_InterfaceName")
+ assert.True(t, ok)
+ mt := suiteMethod(method)
+ assert.Nil(t, mt)
+
+ // Reference method should be skipped
+ method, ok = sType.MethodByName("Reference")
+ assert.True(t, ok)
+ mt = suiteMethod(method)
+ assert.Nil(t, mt)
+
+ // Normal method should not be skipped
+ method, ok = sType.MethodByName("NormalMethod")
+ assert.True(t, ok)
+ mt = suiteMethod(method)
+ assert.NotNil(t, mt)
+}
+
+// Test service with SetGRPCServer method
+type TestServiceWithGRPC struct{}
+
+func (s *TestServiceWithGRPC) SetGRPCServer(server any) {}
+
+func (s *TestServiceWithGRPC) ValidMethod(ctx context.Context, arg any) error {
+ return nil
+}
+
+func TestSuiteMethodSkipsSetGRPCServer(t *testing.T) {
+ s := &TestServiceWithGRPC{}
+ sType := reflect.TypeOf(s)
+
+ // SetGRPCServer should be skipped
+ method, ok := sType.MethodByName("SetGRPCServer")
+ assert.True(t, ok)
+ mt := suiteMethod(method)
+ assert.Nil(t, mt)
+}
+
+func TestGetReferenceWithStruct(t *testing.T) {
+ // Test with struct (not pointer)
+ s := TestService{}
+ ref := GetReference(s)
+ assert.Equal(t, "TestService", ref)
+}
+
+func TestGetReferenceWithAnonymousStruct(t *testing.T) {
+ // Anonymous struct embedded in pointer
+ s := &struct {
+ ServiceWithoutRef
+ }{}
+ ref := GetReference(s)
+ assert.Equal(t, "ServiceWithoutRef", ref)
+}
+
+func TestRegisterWithEmptyServiceName(t *testing.T) {
+ // This tests the edge case where service name cannot be determined
+ // Using a non-struct type
+ var fn func()
+ _, err := ServiceMap.Register("test", "proto", "", "v1", fn)
+ assert.Error(t, err)
+ assert.Contains(t, err.Error(), "no service name")
+}
+
+func TestUnRegisterInterfaceNotFound(t *testing.T) {
+ s := &TestService{}
+ _, err := ServiceMap.Register("TestUnRegisterInterface", testProtocol,
"", "v1", s)
+ assert.NoError(t, err)
+
+ // Manually remove from interfaceMap to simulate inconsistent state
+ ServiceMap.mutex.Lock()
+ delete(ServiceMap.interfaceMap, "TestUnRegisterInterface")
+ ServiceMap.mutex.Unlock()
+
+ err = ServiceMap.UnRegister("TestUnRegisterInterface", testProtocol,
ServiceKey("TestUnRegisterInterface", "", "v1"))
+ assert.Error(t, err)
+ assert.Contains(t, err.Error(), "no service for
TestUnRegisterInterface")
+
+ // Cleanup
+ ServiceMap.mutex.Lock()
+ delete(ServiceMap.serviceMap[testProtocol],
ServiceKey("TestUnRegisterInterface", "", "v1"))
+ ServiceMap.mutex.Unlock()
+}
Review Comment:
This test manually manipulates the ServiceMap's internal state
(interfaceMap) and then relies on manual cleanup of serviceMap. This creates
test interdependencies and potential test pollution. If the cleanup fails or
the test panics before cleanup, subsequent tests may be affected. Consider
using a dedicated test instance or a more robust cleanup mechanism like defer
or t.Cleanup().
##########
common/config/environment_test.go:
##########
@@ -75,3 +76,66 @@ func TestInmemoryConfigurationGetSubProperty(t *testing.T) {
assert.Equal(t, struct{}{}, m["123"])
}
+
+func TestNewEnvInstance(t *testing.T) {
+ // Reset instance
+ oldInstance := instance
+ defer func() { instance = oldInstance }()
+
+ NewEnvInstance()
+ assert.NotNil(t, instance)
+ assert.True(t, instance.configCenterFirst)
+}
+
+func TestSetAndGetDynamicConfiguration(t *testing.T) {
+ env := GetEnvInstance()
+
+ // Initially nil
+ assert.Nil(t, env.GetDynamicConfiguration())
+
+ // Set and get
+ // Using nil as mock since we just test the setter/getter
+ env.SetDynamicConfiguration(nil)
+ assert.Nil(t, env.GetDynamicConfiguration())
+}
Review Comment:
The test comment states "Initially nil" but doesn't verify the initial state
before the SetDynamicConfiguration call. While the assertion on line 94 does
check for nil, this might be testing the state after a previous test rather
than the truly initial state. Consider adding cleanup in the test setup or
verifying this is testing the intended scenario.
##########
common/url_test.go:
##########
@@ -648,3 +651,695 @@ func TestURLSetParamsMultiValue(t *testing.T) {
got3 := u2.GetParams()["only_empty"]
assert.ElementsMatch(t, []string{""}, got3)
}
+
+func TestRoleType(t *testing.T) {
+ assert.Equal(t, "consumers", RoleType(CONSUMER).String())
+ assert.Equal(t, "configurators", RoleType(CONFIGURATOR).String())
+ assert.Equal(t, "routers", RoleType(ROUTER).String())
+ assert.Equal(t, "providers", RoleType(PROVIDER).String())
+
+ assert.Equal(t, "consumer", RoleType(CONSUMER).Role())
+ assert.Equal(t, "", RoleType(CONFIGURATOR).Role())
+ assert.Equal(t, "routers", RoleType(ROUTER).Role())
+ assert.Equal(t, "provider", RoleType(PROVIDER).Role())
+}
+
+func TestJavaClassName(t *testing.T) {
+ u := &URL{}
+ assert.Equal(t, "org.apache.dubbo.common.URL", u.JavaClassName())
+}
+
+func TestWithInterface(t *testing.T) {
+ u := NewURLWithOptions(WithInterface("com.test.Service"))
+ assert.Equal(t, "com.test.Service", u.GetParam(constant.InterfaceKey,
""))
+}
+
+func TestWithLocation(t *testing.T) {
+ // WithLocation sets Location, but NewURLWithOptions overwrites it with
Ip:Port
+ // So we need to set Ip and Port as well, or test the option directly
+ u := &URL{}
+ WithLocation("192.168.1.1:8080")(u)
+ assert.Equal(t, "192.168.1.1:8080", u.Location)
+
+ // When using NewURLWithOptions with Ip and Port
+ u2 := NewURLWithOptions(WithIp("192.168.1.1"), WithPort("8080"))
+ assert.Equal(t, "192.168.1.1:8080", u2.Location)
+}
+
+func TestWithToken(t *testing.T) {
+ // empty token
+ u1 := NewURLWithOptions(WithToken(""))
+ assert.Equal(t, "", u1.GetParam(constant.TokenKey, ""))
+
+ // custom token
+ u2 := NewURLWithOptions(WithToken("my-token"))
+ assert.Equal(t, "my-token", u2.GetParam(constant.TokenKey, ""))
+
+ // "true" generates UUID
+ u3 := NewURLWithOptions(WithToken("true"))
+ token := u3.GetParam(constant.TokenKey, "")
+ assert.NotEmpty(t, token)
+ assert.NotEqual(t, "true", token)
+
+ // "default" generates UUID
+ u4 := NewURLWithOptions(WithToken("default"))
+ token2 := u4.GetParam(constant.TokenKey, "")
+ assert.NotEmpty(t, token2)
+ assert.NotEqual(t, "default", token2)
+
+ // "TRUE" (uppercase) generates UUID
+ u5 := NewURLWithOptions(WithToken("TRUE"))
+ token3 := u5.GetParam(constant.TokenKey, "")
+ assert.NotEmpty(t, token3)
+ assert.NotEqual(t, "TRUE", token3)
+}
+
+func TestWithWeight(t *testing.T) {
+ // positive weight
+ u1 := NewURLWithOptions(WithWeight(100))
+ assert.Equal(t, "100", u1.GetParam(constant.WeightKey, ""))
+
+ // zero weight (should not be set)
+ u2 := NewURLWithOptions(WithWeight(0))
+ assert.Equal(t, "", u2.GetParam(constant.WeightKey, ""))
+
+ // negative weight (should not be set)
+ u3 := NewURLWithOptions(WithWeight(-1))
+ assert.Equal(t, "", u3.GetParam(constant.WeightKey, ""))
+}
+
+func TestMatchKey(t *testing.T) {
+ assert.Equal(t, "com.test.Service:dubbo", MatchKey("com.test.Service",
"dubbo"))
+ assert.Equal(t, ":http", MatchKey("", "http"))
+}
+
+func TestURLGroupInterfaceVersion(t *testing.T) {
+ u, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?interface=com.test.Service&group=test-group&version=1.0.0")
+ assert.Equal(t, "test-group", u.Group())
+ assert.Equal(t, "com.test.Service", u.Interface())
+ assert.Equal(t, "1.0.0", u.Version())
+}
+
+func TestURLAddress(t *testing.T) {
+ u1 := &URL{Ip: "192.168.1.1", Port: "8080"}
+ assert.Equal(t, "192.168.1.1:8080", u1.Address())
+
+ u2 := &URL{Ip: "192.168.1.1", Port: ""}
+ assert.Equal(t, "192.168.1.1", u2.Address())
+}
+
+func TestURLKey(t *testing.T) {
+ u, _ :=
NewURL("dubbo://user:[email protected]:20000/com.test.Service?interface=com.test.Service&group=g1&version=1.0")
+ key := u.Key()
+ assert.Contains(t, key, "dubbo://")
+ assert.Contains(t, key, "user:pass@")
+ assert.Contains(t, key, "interface=com.test.Service")
+ assert.Contains(t, key, "group=g1")
+ assert.Contains(t, key, "version=1.0")
+}
+
+func TestGetCacheInvokerMapKey(t *testing.T) {
+ u, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?interface=com.test.Service&group=g1&version=1.0×tamp=12345")
+ key := u.GetCacheInvokerMapKey()
+ assert.Contains(t, key, "interface=com.test.Service")
+ assert.Contains(t, key, "group=g1")
+ assert.Contains(t, key, "version=1.0")
+ assert.Contains(t, key, "timestamp=12345")
+}
+
+func TestServiceKey(t *testing.T) {
+ // with group and version
+ assert.Equal(t, "group/interface:version", ServiceKey("interface",
"group", "version"))
+
+ // without group
+ assert.Equal(t, "interface:version", ServiceKey("interface", "",
"version"))
+
+ // without version
+ assert.Equal(t, "group/interface", ServiceKey("interface", "group", ""))
+
+ // version 0.0.0 should be ignored
+ assert.Equal(t, "group/interface", ServiceKey("interface", "group",
"0.0.0"))
+
+ // empty interface
+ assert.Equal(t, "", ServiceKey("", "group", "version"))
+
+ // URL method
+ u, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?interface=com.test.Service&group=g1&version=1.0")
+ assert.Equal(t, "g1/com.test.Service:1.0", u.ServiceKey())
+}
+
+func TestEncodedServiceKey(t *testing.T) {
+ u, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?interface=com.test.Service&group=g1&version=1.0")
+ encoded := u.EncodedServiceKey()
+ assert.Equal(t, "g1*com.test.Service:1.0", encoded)
+}
+
+func TestServiceWithSubURL(t *testing.T) {
+ subURL, _ := NewURL("dubbo://127.0.0.1:20000?interface=com.sub.Service")
+ u := &URL{SubURL: subURL}
+ assert.Equal(t, "com.sub.Service", u.Service())
+
+ // empty SubURL interface
+ subURL2 := &URL{}
+ u2 := &URL{SubURL: subURL2, Path: "/com.path.Service"}
+ assert.Equal(t, "com.path.Service", u2.Service())
+
+ // no SubURL, no interface param
+ u3 := &URL{Path: "/com.path.Service"}
+ assert.Equal(t, "com.path.Service", u3.Service())
+}
+
+func TestAddParam(t *testing.T) {
+ u := &URL{}
+ u.AddParam("key1", "value1")
+ assert.Equal(t, "value1", u.GetParam("key1", ""))
+
+ // add another value to same key
+ u.AddParam("key1", "value2")
+ params := u.GetParams()
+ assert.Equal(t, 2, len(params["key1"]))
+}
+
+func TestAddParamAvoidNil(t *testing.T) {
+ u := &URL{}
+ u.AddParamAvoidNil("key1", "value1")
+ assert.Equal(t, "value1", u.GetParam("key1", ""))
+}
+
+func TestDelParam(t *testing.T) {
+ u, _ := NewURL("dubbo://127.0.0.1:20000?key1=value1&key2=value2")
+ u.DelParam("key1")
+ assert.Equal(t, "", u.GetParam("key1", ""))
+ assert.Equal(t, "value2", u.GetParam("key2", ""))
+
+ // delete from nil params
+ u2 := &URL{}
+ u2.DelParam("key") // should not panic
+}
+
+func TestGetNonDefaultParam(t *testing.T) {
+ u, _ := NewURL("dubbo://127.0.0.1:20000?key1=value1")
+
+ v, ok := u.GetNonDefaultParam("key1")
+ assert.True(t, ok)
+ assert.Equal(t, "value1", v)
+
+ v, ok = u.GetNonDefaultParam("nonexistent")
+ assert.False(t, ok)
+ assert.Equal(t, "", v)
+}
+
+func TestRangeParams(t *testing.T) {
+ u, _ := NewURL("dubbo://127.0.0.1:20000?key1=value1&key2=value2")
+ count := 0
+ u.RangeParams(func(key, value string) bool {
+ count++
+ return true
+ })
+ assert.Equal(t, 2, count)
+
+ // test early break
+ count = 0
+ u.RangeParams(func(key, value string) bool {
+ count++
+ return false // break after first
+ })
+ assert.Equal(t, 1, count)
+}
+
+func TestGetParamInt32(t *testing.T) {
+ params := url.Values{}
+ params.Set("key", "123")
+
+ u := &URL{}
+ u.SetParams(params)
+
+ v := u.GetParamInt32("key", 0)
+ assert.Equal(t, int32(123), v)
+
+ // invalid value
+ u2 := &URL{}
+ v2 := u2.GetParamInt32("key", 99)
+ assert.Equal(t, int32(99), v2)
+}
+
+func TestGetParamByIntValue(t *testing.T) {
+ params := url.Values{}
+ params.Set("key", "456")
+
+ u := &URL{}
+ u.SetParams(params)
+
+ v := u.GetParamByIntValue("key", 0)
+ assert.Equal(t, 456, v)
+
+ // invalid value
+ u2 := &URL{}
+ v2 := u2.GetParamByIntValue("key", 99)
+ assert.Equal(t, 99, v2)
+}
+
+func TestGetMethodParamIntValue(t *testing.T) {
+ params := url.Values{}
+ params.Set("methods.GetValue.timeout", "100")
+
+ u := &URL{}
+ u.SetParams(params)
+
+ v := u.GetMethodParamIntValue("GetValue", "timeout", 0)
+ assert.Equal(t, 100, v)
+
+ // missing method param
+ v2 := u.GetMethodParamIntValue("OtherMethod", "timeout", 50)
+ assert.Equal(t, 50, v2)
+}
+
+func TestGetMethodParamInt64(t *testing.T) {
+ params := url.Values{}
+ params.Set("methods.GetValue.timeout", "200")
+ params.Set("timeout", "100")
+
+ u := &URL{}
+ u.SetParams(params)
+
+ // method param exists
+ v := u.GetMethodParamInt64("GetValue", "timeout", 0)
+ assert.Equal(t, int64(200), v)
+
+ // method param not exists, fallback to global
+ v2 := u.GetMethodParamInt64("OtherMethod", "timeout", 0)
+ assert.Equal(t, int64(100), v2)
+
+ // neither exists
+ v3 := u.GetMethodParamInt64("OtherMethod", "retries", 5)
+ assert.Equal(t, int64(5), v3)
+}
+
+func TestGetParamDuration(t *testing.T) {
+ params := url.Values{}
+ params.Set("timeout", "5s")
+
+ u := &URL{}
+ u.SetParams(params)
+
+ d := u.GetParamDuration("timeout", "1s")
+ assert.Equal(t, 5*time.Second, d)
+
+ // invalid duration, returns 3s default
+ params2 := url.Values{}
+ params2.Set("timeout", "invalid")
+ u2 := &URL{}
+ u2.SetParams(params2)
+ d2 := u2.GetParamDuration("timeout", "invalid")
+ assert.Equal(t, 3*time.Second, d2)
+
+ // missing param with valid default
+ u3 := &URL{}
+ d3 := u3.GetParamDuration("timeout", "2s")
+ assert.Equal(t, 2*time.Second, d3)
+}
+
+func TestCloneExceptParams(t *testing.T) {
+ u, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?key1=value1&key2=value2&key3=value3")
+
+ excludeSet := gxset.NewSet("key1", "key3")
+ cloned := u.CloneExceptParams(excludeSet)
+
+ assert.Equal(t, "", cloned.GetParam("key1", ""))
+ assert.Equal(t, "value2", cloned.GetParam("key2", ""))
+ assert.Equal(t, "", cloned.GetParam("key3", ""))
+ assert.Equal(t, "dubbo", cloned.Protocol)
+}
+
+func TestCloneWithParams(t *testing.T) {
+ u, _ :=
NewURL("dubbo://user:[email protected]:20000/com.test.Service?key1=value1&key2=value2&key3=value3")
+ u.Methods = []string{"method1", "method2"}
+
+ cloned := u.CloneWithParams([]string{"key1", "key3"})
+
+ assert.Equal(t, "value1", cloned.GetParam("key1", ""))
+ assert.Equal(t, "", cloned.GetParam("key2", ""))
+ assert.Equal(t, "value3", cloned.GetParam("key3", ""))
+ assert.Equal(t, "dubbo", cloned.Protocol)
+ assert.Equal(t, "user", cloned.Username)
+ assert.Equal(t, "pass", cloned.Password)
+ assert.Equal(t, []string{"method1", "method2"}, cloned.Methods)
+}
+
+func TestURLCompare(t *testing.T) {
+ u1, _ := NewURL("dubbo://127.0.0.1:20000/a.Service")
+ u2, _ := NewURL("dubbo://127.0.0.1:20000/b.Service")
+ u3, _ := NewURL("dubbo://127.0.0.1:20000/a.Service")
+
+ assert.Equal(t, -1, u1.Compare(u2))
+ assert.Equal(t, 1, u2.Compare(u1))
+ assert.Equal(t, 0, u1.Compare(u3))
+}
+
+func TestRangeAttributes(t *testing.T) {
+ u := &URL{}
+ u.SetAttribute("attr1", "value1")
+ u.SetAttribute("attr2", 123)
+
+ count := 0
+ u.RangeAttributes(func(key string, value any) bool {
+ count++
+ return true
+ })
+ assert.Equal(t, 2, count)
+
+ // test early break
+ count = 0
+ u.RangeAttributes(func(key string, value any) bool {
+ count++
+ return false
+ })
+ assert.Equal(t, 1, count)
+}
+
+func TestURLSlice(t *testing.T) {
+ u1, _ := NewURL("dubbo://127.0.0.1:20000/c.Service")
+ u2, _ := NewURL("dubbo://127.0.0.1:20000/a.Service")
+ u3, _ := NewURL("dubbo://127.0.0.1:20000/b.Service")
+
+ slice := URLSlice{u1, u2, u3}
+
+ assert.Equal(t, 3, slice.Len())
+ assert.True(t, slice.Less(1, 2)) // a < b
+ assert.False(t, slice.Less(0, 2)) // c > b
+
+ slice.Swap(0, 1)
+ assert.Equal(t, u2, slice[0])
+ assert.Equal(t, u1, slice[1])
+}
+
+func TestIsEquals(t *testing.T) {
+ u1, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?key1=value1&key2=value2")
+ u2, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?key1=value1&key2=value2")
+ u3, _ :=
NewURL("dubbo://127.0.0.1:20001/com.test.Service?key1=value1&key2=value2")
+ u4, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?key1=value1&key2=different")
+
+ // equal URLs
+ assert.True(t, IsEquals(u1, u2))
+
+ // different port
+ assert.False(t, IsEquals(u1, u3))
+
+ // different param value
+ assert.False(t, IsEquals(u1, u4))
+
+ // with excludes
+ assert.True(t, IsEquals(u1, u4, "key2"))
+
+ // nil checks
+ assert.False(t, IsEquals(nil, u1))
+ assert.False(t, IsEquals(u1, nil))
+}
+
+func TestGetSubscribeName(t *testing.T) {
+ u, _ :=
NewURL("dubbo://127.0.0.1:20000?interface=com.test.Service&version=1.0&group=test")
+ name := GetSubscribeName(u)
+ assert.Contains(t, name, "providers")
+ assert.Contains(t, name, "com.test.Service")
+ assert.Contains(t, name, "1.0")
+ assert.Contains(t, name, "test")
+}
+
+func TestNewURLEmptyString(t *testing.T) {
+ u, err := NewURL("")
+ assert.NoError(t, err)
+ assert.NotNil(t, u)
+ assert.Equal(t, "", u.Protocol)
+}
+
+func TestNewURLWithUsernamePassword(t *testing.T) {
+ u, err :=
NewURL("dubbo://admin:[email protected]:20000/com.test.Service")
+ assert.NoError(t, err)
+ assert.Equal(t, "admin", u.Username)
+ assert.Equal(t, "secret", u.Password)
+}
+
+func TestURLStringWithCredentials(t *testing.T) {
+ u, _ :=
NewURL("dubbo://admin:[email protected]:20000/com.test.Service?key=value")
+ str := u.String()
+ assert.Contains(t, str, "admin:secret@")
+}
+
+func TestToMapWithoutPort(t *testing.T) {
+ u := &URL{
+ Protocol: "dubbo",
+ Location: "127.0.0.1",
+ Path: "/com.test.Service",
+ }
+ m := u.ToMap()
+ assert.Equal(t, "0", m["port"])
+}
+
+func TestToMapEmpty(t *testing.T) {
+ u := &URL{}
+ m := u.ToMap()
+ assert.Nil(t, m)
+}
+
+func TestMergeURLWithAttributes(t *testing.T) {
+ u1, _ := NewURL("dubbo://127.0.0.1:20000?key1=value1")
+ u1.SetAttribute("attr1", "attrValue1")
+
+ u2, _ := NewURL("dubbo://127.0.0.1:20001?key2=value2")
+ u2.SetAttribute("attr2", "attrValue2")
+ u2.Methods = []string{"method1"}
+
+ merged := u1.MergeURL(u2)
+
+ // attributes should be merged
+ v1, ok1 := merged.GetAttribute("attr1")
+ assert.True(t, ok1)
+ assert.Equal(t, "attrValue1", v1)
+
+ v2, ok2 := merged.GetAttribute("attr2")
+ assert.True(t, ok2)
+ assert.Equal(t, "attrValue2", v2)
+}
+
+func TestURLEqualWithCategory(t *testing.T) {
+ // test category matching with RemoveValuePrefix
+ u1, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?interface=com.test.Service&category=providers")
+ u2, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?interface=com.test.Service&category=-providers")
+ assert.False(t, u1.URLEqual(u2))
+
+ // category contains target - u3 has category that contains u1's
category
+ u3, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?interface=com.test.Service&category=providers,consumers")
+ // u1 checks if u3's category matches - u3 has "providers,consumers"
which contains "providers"
+ assert.True(t, u3.URLEqual(u1))
+}
+
+func TestNewURLWithRegistryGroup(t *testing.T) {
+ u, err :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?registry.group=mygroup")
+ assert.NoError(t, err)
+ assert.Contains(t, u.PrimitiveURL, "mygroup")
+}
+
+func TestColonSeparatedKeyEmpty(t *testing.T) {
+ u := &URL{}
+ assert.Equal(t, "", u.ColonSeparatedKey())
+}
+
+func TestColonSeparatedKeyWithVersion000(t *testing.T) {
+ u, _ := NewURL("dubbo://127.0.0.1:20000")
+ u.AddParam(constant.InterfaceKey, "com.test.Service")
+ u.AddParam(constant.VersionKey, "0.0.0")
+ u.AddParam(constant.GroupKey, "group1")
+
+ // version 0.0.0 should be treated as empty
+ assert.Equal(t, "com.test.Service::group1", u.ColonSeparatedKey())
+}
+
+// Additional edge case tests for better coverage
+
+func TestNewURLParseError(t *testing.T) {
+ // Test invalid URL that causes parse error
+ _, err := NewURL("://invalid")
+ assert.Error(t, err)
+}
+
+func TestNewURLWithInvalidQuery(t *testing.T) {
+ // URL with special characters that need escaping
+ u, err :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?key=value%20with%20space")
+ assert.NoError(t, err)
+ assert.Equal(t, "value with space", u.GetParam("key", ""))
+}
+
+func TestURLStringWithoutCredentials(t *testing.T) {
+ u, _ := NewURL("dubbo://127.0.0.1:20000/com.test.Service?key=value")
+ str := u.String()
+ assert.Contains(t, str, "dubbo://127.0.0.1:20000")
+ assert.NotContains(t, str, "@")
+}
+
+func TestGetParamAndDecodedError(t *testing.T) {
+ u := &URL{}
+ params := url.Values{}
+ params.Set("rule", "invalid-base64!!!")
+ u.SetParams(params)
+
+ _, err := u.GetParamAndDecoded("rule")
+ assert.Error(t, err)
+}
+
+func TestIsEqualsWithDifferentMapLength(t *testing.T) {
+ u1, _ := NewURL("dubbo://127.0.0.1:20000/com.test.Service?key1=value1")
+ u2, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?key1=value1&key2=value2")
+ assert.False(t, IsEquals(u1, u2))
+}
+
+func TestIsEqualsWithMissingKey(t *testing.T) {
+ u1, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?key1=value1&key2=value2")
+ u2, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?key1=value1&key3=value3")
+ assert.False(t, IsEquals(u1, u2))
+}
+
+func TestIsEqualsBothNil(t *testing.T) {
+ // Both nil should not panic - but current implementation would panic
+ // This tests the nil check logic
+ var u1 *URL = nil
+ var u2 *URL = nil
+ // Note: IsEquals(nil, nil) would panic due to nil pointer dereference
+ // This is expected behavior based on current implementation
Review Comment:
The comment on lines 1202-1207 mentions that "IsEquals(nil, nil) would
panic" and describes it as "expected behavior," but the test doesn't actually
test this scenario. The test only checks one nil and one non-nil URL. Either
the comment should be updated to reflect what the test actually does, or the
test should be removed if the panic behavior with both nil arguments is indeed
expected and doesn't need testing.
```suggestion
// Verify that IsEquals handles cases where exactly one URL is nil
without panicking.
// Note: the behavior when both arguments are nil (IsEquals(nil, nil))
is not covered here.
var u1 *URL = nil
var u2 *URL = nil
```
##########
common/url_test.go:
##########
@@ -648,3 +651,695 @@ func TestURLSetParamsMultiValue(t *testing.T) {
got3 := u2.GetParams()["only_empty"]
assert.ElementsMatch(t, []string{""}, got3)
}
+
+func TestRoleType(t *testing.T) {
+ assert.Equal(t, "consumers", RoleType(CONSUMER).String())
+ assert.Equal(t, "configurators", RoleType(CONFIGURATOR).String())
+ assert.Equal(t, "routers", RoleType(ROUTER).String())
+ assert.Equal(t, "providers", RoleType(PROVIDER).String())
+
+ assert.Equal(t, "consumer", RoleType(CONSUMER).Role())
+ assert.Equal(t, "", RoleType(CONFIGURATOR).Role())
+ assert.Equal(t, "routers", RoleType(ROUTER).Role())
+ assert.Equal(t, "provider", RoleType(PROVIDER).Role())
+}
+
+func TestJavaClassName(t *testing.T) {
+ u := &URL{}
+ assert.Equal(t, "org.apache.dubbo.common.URL", u.JavaClassName())
+}
+
+func TestWithInterface(t *testing.T) {
+ u := NewURLWithOptions(WithInterface("com.test.Service"))
+ assert.Equal(t, "com.test.Service", u.GetParam(constant.InterfaceKey,
""))
+}
+
+func TestWithLocation(t *testing.T) {
+ // WithLocation sets Location, but NewURLWithOptions overwrites it with
Ip:Port
+ // So we need to set Ip and Port as well, or test the option directly
+ u := &URL{}
+ WithLocation("192.168.1.1:8080")(u)
+ assert.Equal(t, "192.168.1.1:8080", u.Location)
+
+ // When using NewURLWithOptions with Ip and Port
+ u2 := NewURLWithOptions(WithIp("192.168.1.1"), WithPort("8080"))
+ assert.Equal(t, "192.168.1.1:8080", u2.Location)
+}
+
+func TestWithToken(t *testing.T) {
+ // empty token
+ u1 := NewURLWithOptions(WithToken(""))
+ assert.Equal(t, "", u1.GetParam(constant.TokenKey, ""))
+
+ // custom token
+ u2 := NewURLWithOptions(WithToken("my-token"))
+ assert.Equal(t, "my-token", u2.GetParam(constant.TokenKey, ""))
+
+ // "true" generates UUID
+ u3 := NewURLWithOptions(WithToken("true"))
+ token := u3.GetParam(constant.TokenKey, "")
+ assert.NotEmpty(t, token)
+ assert.NotEqual(t, "true", token)
+
+ // "default" generates UUID
+ u4 := NewURLWithOptions(WithToken("default"))
+ token2 := u4.GetParam(constant.TokenKey, "")
+ assert.NotEmpty(t, token2)
+ assert.NotEqual(t, "default", token2)
+
+ // "TRUE" (uppercase) generates UUID
+ u5 := NewURLWithOptions(WithToken("TRUE"))
+ token3 := u5.GetParam(constant.TokenKey, "")
+ assert.NotEmpty(t, token3)
+ assert.NotEqual(t, "TRUE", token3)
+}
+
+func TestWithWeight(t *testing.T) {
+ // positive weight
+ u1 := NewURLWithOptions(WithWeight(100))
+ assert.Equal(t, "100", u1.GetParam(constant.WeightKey, ""))
+
+ // zero weight (should not be set)
+ u2 := NewURLWithOptions(WithWeight(0))
+ assert.Equal(t, "", u2.GetParam(constant.WeightKey, ""))
+
+ // negative weight (should not be set)
+ u3 := NewURLWithOptions(WithWeight(-1))
+ assert.Equal(t, "", u3.GetParam(constant.WeightKey, ""))
+}
+
+func TestMatchKey(t *testing.T) {
+ assert.Equal(t, "com.test.Service:dubbo", MatchKey("com.test.Service",
"dubbo"))
+ assert.Equal(t, ":http", MatchKey("", "http"))
+}
+
+func TestURLGroupInterfaceVersion(t *testing.T) {
+ u, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?interface=com.test.Service&group=test-group&version=1.0.0")
+ assert.Equal(t, "test-group", u.Group())
+ assert.Equal(t, "com.test.Service", u.Interface())
+ assert.Equal(t, "1.0.0", u.Version())
+}
+
+func TestURLAddress(t *testing.T) {
+ u1 := &URL{Ip: "192.168.1.1", Port: "8080"}
+ assert.Equal(t, "192.168.1.1:8080", u1.Address())
+
+ u2 := &URL{Ip: "192.168.1.1", Port: ""}
+ assert.Equal(t, "192.168.1.1", u2.Address())
+}
+
+func TestURLKey(t *testing.T) {
+ u, _ :=
NewURL("dubbo://user:[email protected]:20000/com.test.Service?interface=com.test.Service&group=g1&version=1.0")
+ key := u.Key()
+ assert.Contains(t, key, "dubbo://")
+ assert.Contains(t, key, "user:pass@")
+ assert.Contains(t, key, "interface=com.test.Service")
+ assert.Contains(t, key, "group=g1")
+ assert.Contains(t, key, "version=1.0")
+}
+
+func TestGetCacheInvokerMapKey(t *testing.T) {
+ u, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?interface=com.test.Service&group=g1&version=1.0×tamp=12345")
+ key := u.GetCacheInvokerMapKey()
+ assert.Contains(t, key, "interface=com.test.Service")
+ assert.Contains(t, key, "group=g1")
+ assert.Contains(t, key, "version=1.0")
+ assert.Contains(t, key, "timestamp=12345")
+}
+
+func TestServiceKey(t *testing.T) {
+ // with group and version
+ assert.Equal(t, "group/interface:version", ServiceKey("interface",
"group", "version"))
+
+ // without group
+ assert.Equal(t, "interface:version", ServiceKey("interface", "",
"version"))
+
+ // without version
+ assert.Equal(t, "group/interface", ServiceKey("interface", "group", ""))
+
+ // version 0.0.0 should be ignored
+ assert.Equal(t, "group/interface", ServiceKey("interface", "group",
"0.0.0"))
+
+ // empty interface
+ assert.Equal(t, "", ServiceKey("", "group", "version"))
+
+ // URL method
+ u, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?interface=com.test.Service&group=g1&version=1.0")
+ assert.Equal(t, "g1/com.test.Service:1.0", u.ServiceKey())
+}
+
+func TestEncodedServiceKey(t *testing.T) {
+ u, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?interface=com.test.Service&group=g1&version=1.0")
+ encoded := u.EncodedServiceKey()
+ assert.Equal(t, "g1*com.test.Service:1.0", encoded)
+}
+
+func TestServiceWithSubURL(t *testing.T) {
+ subURL, _ := NewURL("dubbo://127.0.0.1:20000?interface=com.sub.Service")
+ u := &URL{SubURL: subURL}
+ assert.Equal(t, "com.sub.Service", u.Service())
+
+ // empty SubURL interface
+ subURL2 := &URL{}
+ u2 := &URL{SubURL: subURL2, Path: "/com.path.Service"}
+ assert.Equal(t, "com.path.Service", u2.Service())
+
+ // no SubURL, no interface param
+ u3 := &URL{Path: "/com.path.Service"}
+ assert.Equal(t, "com.path.Service", u3.Service())
+}
+
+func TestAddParam(t *testing.T) {
+ u := &URL{}
+ u.AddParam("key1", "value1")
+ assert.Equal(t, "value1", u.GetParam("key1", ""))
+
+ // add another value to same key
+ u.AddParam("key1", "value2")
+ params := u.GetParams()
+ assert.Equal(t, 2, len(params["key1"]))
+}
+
+func TestAddParamAvoidNil(t *testing.T) {
+ u := &URL{}
+ u.AddParamAvoidNil("key1", "value1")
+ assert.Equal(t, "value1", u.GetParam("key1", ""))
+}
+
+func TestDelParam(t *testing.T) {
+ u, _ := NewURL("dubbo://127.0.0.1:20000?key1=value1&key2=value2")
+ u.DelParam("key1")
+ assert.Equal(t, "", u.GetParam("key1", ""))
+ assert.Equal(t, "value2", u.GetParam("key2", ""))
+
+ // delete from nil params
+ u2 := &URL{}
+ u2.DelParam("key") // should not panic
+}
+
+func TestGetNonDefaultParam(t *testing.T) {
+ u, _ := NewURL("dubbo://127.0.0.1:20000?key1=value1")
+
+ v, ok := u.GetNonDefaultParam("key1")
+ assert.True(t, ok)
+ assert.Equal(t, "value1", v)
+
+ v, ok = u.GetNonDefaultParam("nonexistent")
+ assert.False(t, ok)
+ assert.Equal(t, "", v)
+}
+
+func TestRangeParams(t *testing.T) {
+ u, _ := NewURL("dubbo://127.0.0.1:20000?key1=value1&key2=value2")
+ count := 0
+ u.RangeParams(func(key, value string) bool {
+ count++
+ return true
+ })
+ assert.Equal(t, 2, count)
+
+ // test early break
+ count = 0
+ u.RangeParams(func(key, value string) bool {
+ count++
+ return false // break after first
+ })
+ assert.Equal(t, 1, count)
+}
+
+func TestGetParamInt32(t *testing.T) {
+ params := url.Values{}
+ params.Set("key", "123")
+
+ u := &URL{}
+ u.SetParams(params)
+
+ v := u.GetParamInt32("key", 0)
+ assert.Equal(t, int32(123), v)
+
+ // invalid value
+ u2 := &URL{}
+ v2 := u2.GetParamInt32("key", 99)
+ assert.Equal(t, int32(99), v2)
+}
+
+func TestGetParamByIntValue(t *testing.T) {
+ params := url.Values{}
+ params.Set("key", "456")
+
+ u := &URL{}
+ u.SetParams(params)
+
+ v := u.GetParamByIntValue("key", 0)
+ assert.Equal(t, 456, v)
+
+ // invalid value
+ u2 := &URL{}
+ v2 := u2.GetParamByIntValue("key", 99)
+ assert.Equal(t, 99, v2)
+}
+
+func TestGetMethodParamIntValue(t *testing.T) {
+ params := url.Values{}
+ params.Set("methods.GetValue.timeout", "100")
+
+ u := &URL{}
+ u.SetParams(params)
+
+ v := u.GetMethodParamIntValue("GetValue", "timeout", 0)
+ assert.Equal(t, 100, v)
+
+ // missing method param
+ v2 := u.GetMethodParamIntValue("OtherMethod", "timeout", 50)
+ assert.Equal(t, 50, v2)
+}
+
+func TestGetMethodParamInt64(t *testing.T) {
+ params := url.Values{}
+ params.Set("methods.GetValue.timeout", "200")
+ params.Set("timeout", "100")
+
+ u := &URL{}
+ u.SetParams(params)
+
+ // method param exists
+ v := u.GetMethodParamInt64("GetValue", "timeout", 0)
+ assert.Equal(t, int64(200), v)
+
+ // method param not exists, fallback to global
+ v2 := u.GetMethodParamInt64("OtherMethod", "timeout", 0)
+ assert.Equal(t, int64(100), v2)
+
+ // neither exists
+ v3 := u.GetMethodParamInt64("OtherMethod", "retries", 5)
+ assert.Equal(t, int64(5), v3)
+}
+
+func TestGetParamDuration(t *testing.T) {
+ params := url.Values{}
+ params.Set("timeout", "5s")
+
+ u := &URL{}
+ u.SetParams(params)
+
+ d := u.GetParamDuration("timeout", "1s")
+ assert.Equal(t, 5*time.Second, d)
+
+ // invalid duration, returns 3s default
+ params2 := url.Values{}
+ params2.Set("timeout", "invalid")
+ u2 := &URL{}
+ u2.SetParams(params2)
+ d2 := u2.GetParamDuration("timeout", "invalid")
+ assert.Equal(t, 3*time.Second, d2)
+
+ // missing param with valid default
+ u3 := &URL{}
+ d3 := u3.GetParamDuration("timeout", "2s")
+ assert.Equal(t, 2*time.Second, d3)
+}
+
+func TestCloneExceptParams(t *testing.T) {
+ u, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?key1=value1&key2=value2&key3=value3")
+
+ excludeSet := gxset.NewSet("key1", "key3")
+ cloned := u.CloneExceptParams(excludeSet)
+
+ assert.Equal(t, "", cloned.GetParam("key1", ""))
+ assert.Equal(t, "value2", cloned.GetParam("key2", ""))
+ assert.Equal(t, "", cloned.GetParam("key3", ""))
+ assert.Equal(t, "dubbo", cloned.Protocol)
+}
+
+func TestCloneWithParams(t *testing.T) {
+ u, _ :=
NewURL("dubbo://user:[email protected]:20000/com.test.Service?key1=value1&key2=value2&key3=value3")
+ u.Methods = []string{"method1", "method2"}
+
+ cloned := u.CloneWithParams([]string{"key1", "key3"})
+
+ assert.Equal(t, "value1", cloned.GetParam("key1", ""))
+ assert.Equal(t, "", cloned.GetParam("key2", ""))
+ assert.Equal(t, "value3", cloned.GetParam("key3", ""))
+ assert.Equal(t, "dubbo", cloned.Protocol)
+ assert.Equal(t, "user", cloned.Username)
+ assert.Equal(t, "pass", cloned.Password)
+ assert.Equal(t, []string{"method1", "method2"}, cloned.Methods)
+}
+
+func TestURLCompare(t *testing.T) {
+ u1, _ := NewURL("dubbo://127.0.0.1:20000/a.Service")
+ u2, _ := NewURL("dubbo://127.0.0.1:20000/b.Service")
+ u3, _ := NewURL("dubbo://127.0.0.1:20000/a.Service")
+
+ assert.Equal(t, -1, u1.Compare(u2))
+ assert.Equal(t, 1, u2.Compare(u1))
+ assert.Equal(t, 0, u1.Compare(u3))
+}
+
+func TestRangeAttributes(t *testing.T) {
+ u := &URL{}
+ u.SetAttribute("attr1", "value1")
+ u.SetAttribute("attr2", 123)
+
+ count := 0
+ u.RangeAttributes(func(key string, value any) bool {
+ count++
+ return true
+ })
+ assert.Equal(t, 2, count)
+
+ // test early break
+ count = 0
+ u.RangeAttributes(func(key string, value any) bool {
+ count++
+ return false
+ })
+ assert.Equal(t, 1, count)
+}
+
+func TestURLSlice(t *testing.T) {
+ u1, _ := NewURL("dubbo://127.0.0.1:20000/c.Service")
+ u2, _ := NewURL("dubbo://127.0.0.1:20000/a.Service")
+ u3, _ := NewURL("dubbo://127.0.0.1:20000/b.Service")
+
+ slice := URLSlice{u1, u2, u3}
+
+ assert.Equal(t, 3, slice.Len())
+ assert.True(t, slice.Less(1, 2)) // a < b
+ assert.False(t, slice.Less(0, 2)) // c > b
+
+ slice.Swap(0, 1)
+ assert.Equal(t, u2, slice[0])
+ assert.Equal(t, u1, slice[1])
+}
+
+func TestIsEquals(t *testing.T) {
+ u1, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?key1=value1&key2=value2")
+ u2, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?key1=value1&key2=value2")
+ u3, _ :=
NewURL("dubbo://127.0.0.1:20001/com.test.Service?key1=value1&key2=value2")
+ u4, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?key1=value1&key2=different")
+
+ // equal URLs
+ assert.True(t, IsEquals(u1, u2))
+
+ // different port
+ assert.False(t, IsEquals(u1, u3))
+
+ // different param value
+ assert.False(t, IsEquals(u1, u4))
+
+ // with excludes
+ assert.True(t, IsEquals(u1, u4, "key2"))
+
+ // nil checks
+ assert.False(t, IsEquals(nil, u1))
+ assert.False(t, IsEquals(u1, nil))
+}
+
+func TestGetSubscribeName(t *testing.T) {
+ u, _ :=
NewURL("dubbo://127.0.0.1:20000?interface=com.test.Service&version=1.0&group=test")
+ name := GetSubscribeName(u)
+ assert.Contains(t, name, "providers")
+ assert.Contains(t, name, "com.test.Service")
+ assert.Contains(t, name, "1.0")
+ assert.Contains(t, name, "test")
+}
+
+func TestNewURLEmptyString(t *testing.T) {
+ u, err := NewURL("")
+ assert.NoError(t, err)
+ assert.NotNil(t, u)
+ assert.Equal(t, "", u.Protocol)
+}
+
+func TestNewURLWithUsernamePassword(t *testing.T) {
+ u, err :=
NewURL("dubbo://admin:[email protected]:20000/com.test.Service")
+ assert.NoError(t, err)
+ assert.Equal(t, "admin", u.Username)
+ assert.Equal(t, "secret", u.Password)
+}
+
+func TestURLStringWithCredentials(t *testing.T) {
+ u, _ :=
NewURL("dubbo://admin:[email protected]:20000/com.test.Service?key=value")
+ str := u.String()
+ assert.Contains(t, str, "admin:secret@")
+}
+
+func TestToMapWithoutPort(t *testing.T) {
+ u := &URL{
+ Protocol: "dubbo",
+ Location: "127.0.0.1",
+ Path: "/com.test.Service",
+ }
+ m := u.ToMap()
+ assert.Equal(t, "0", m["port"])
+}
+
+func TestToMapEmpty(t *testing.T) {
+ u := &URL{}
+ m := u.ToMap()
+ assert.Nil(t, m)
+}
+
+func TestMergeURLWithAttributes(t *testing.T) {
+ u1, _ := NewURL("dubbo://127.0.0.1:20000?key1=value1")
+ u1.SetAttribute("attr1", "attrValue1")
+
+ u2, _ := NewURL("dubbo://127.0.0.1:20001?key2=value2")
+ u2.SetAttribute("attr2", "attrValue2")
+ u2.Methods = []string{"method1"}
+
+ merged := u1.MergeURL(u2)
+
+ // attributes should be merged
+ v1, ok1 := merged.GetAttribute("attr1")
+ assert.True(t, ok1)
+ assert.Equal(t, "attrValue1", v1)
+
+ v2, ok2 := merged.GetAttribute("attr2")
+ assert.True(t, ok2)
+ assert.Equal(t, "attrValue2", v2)
+}
+
+func TestURLEqualWithCategory(t *testing.T) {
+ // test category matching with RemoveValuePrefix
+ u1, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?interface=com.test.Service&category=providers")
+ u2, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?interface=com.test.Service&category=-providers")
+ assert.False(t, u1.URLEqual(u2))
+
+ // category contains target - u3 has category that contains u1's
category
+ u3, _ :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?interface=com.test.Service&category=providers,consumers")
+ // u1 checks if u3's category matches - u3 has "providers,consumers"
which contains "providers"
+ assert.True(t, u3.URLEqual(u1))
+}
+
+func TestNewURLWithRegistryGroup(t *testing.T) {
+ u, err :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?registry.group=mygroup")
+ assert.NoError(t, err)
+ assert.Contains(t, u.PrimitiveURL, "mygroup")
+}
+
+func TestColonSeparatedKeyEmpty(t *testing.T) {
+ u := &URL{}
+ assert.Equal(t, "", u.ColonSeparatedKey())
+}
+
+func TestColonSeparatedKeyWithVersion000(t *testing.T) {
+ u, _ := NewURL("dubbo://127.0.0.1:20000")
+ u.AddParam(constant.InterfaceKey, "com.test.Service")
+ u.AddParam(constant.VersionKey, "0.0.0")
+ u.AddParam(constant.GroupKey, "group1")
+
+ // version 0.0.0 should be treated as empty
+ assert.Equal(t, "com.test.Service::group1", u.ColonSeparatedKey())
+}
+
+// Additional edge case tests for better coverage
+
+func TestNewURLParseError(t *testing.T) {
+ // Test invalid URL that causes parse error
+ _, err := NewURL("://invalid")
+ assert.Error(t, err)
+}
+
+func TestNewURLWithInvalidQuery(t *testing.T) {
+ // URL with special characters that need escaping
+ u, err :=
NewURL("dubbo://127.0.0.1:20000/com.test.Service?key=value%20with%20space")
+ assert.NoError(t, err)
+ assert.Equal(t, "value with space", u.GetParam("key", ""))
+}
Review Comment:
The test name and comment indicate this tests "invalid query" handling, but
the URL used contains valid percent-encoded characters that should decode
successfully. This is not actually testing an invalid query scenario. Consider
renaming to "TestNewURLWithEncodedQuery" or similar to accurately reflect what
is being tested.
--
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]
