Copilot commented on code in PR #7746:
URL: https://github.com/apache/incubator-seata/pull/7746#discussion_r2536701918
##########
discovery/seata-discovery-nacos/src/test/java/org/apache/seata/discovery/registry/nacos/NacosRegistryServiceImplTest.java:
##########
@@ -16,32 +16,233 @@
*/
package org.apache.seata.discovery.registry.nacos;
-import org.apache.seata.common.util.ReflectionUtil;
-import org.assertj.core.api.Assertions;
+import com.alibaba.nacos.api.naming.listener.Event;
+import com.alibaba.nacos.api.naming.listener.EventListener;
+import com.alibaba.nacos.api.naming.listener.NamingEvent;
+import com.alibaba.nacos.api.naming.pojo.Instance;
+import org.apache.seata.common.metadata.ServiceInstance;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.api.condition.EnabledIfSystemProperty;
import java.lang.reflect.Field;
-import java.lang.reflect.Method;
-import java.util.Properties;
+import java.net.InetSocketAddress;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
-/**
- * The type Nacos registry serivce impl test
- */
+@EnabledIfSystemProperty(named = "nacosCaseEnabled", matches = "true")
+@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
public class NacosRegistryServiceImplTest {
+ private static final String SERVICE_NAME = "default_tx_group";
+ private static final String CLUSTER_NAME = "default";
+
+ private static final NacosRegistryServiceImpl service =
NacosRegistryServiceImpl.getInstance();
+
+ @Test
+ public void testGetInstance() {
+ NacosRegistryServiceImpl instance =
NacosRegistryServiceImpl.getInstance();
+ assertInstanceOf(NacosRegistryServiceImpl.class, instance);
+ }
+
+ @Test
+ @Order(1)
+ public void testRegister() throws Exception {
+ InetSocketAddress address = new InetSocketAddress("127.0.0.1", 8091);
+ Map<String, Object> metadata = new HashMap<>();
+ metadata.put("version", "1.0.0");
+ metadata.put("weight", 1.0);
+ metadata.put("healthy", true);
+
+ ServiceInstance serviceInstance = new ServiceInstance(address,
metadata);
+
+ // Verify ServiceInstance metadata
+ assertNotNull(serviceInstance.getMetadata());
+ assertEquals("1.0.0", serviceInstance.getMetadata().get("version"));
+ assertEquals(1.0, serviceInstance.getMetadata().get("weight"));
+ assertEquals(true, serviceInstance.getMetadata().get("healthy"));
+
+ service.register(serviceInstance);
+
+ // Verify registration success
+ long startTime = System.currentTimeMillis();
+ while (service.lookup(SERVICE_NAME).isEmpty() &&
System.currentTimeMillis() - startTime < 10000) {
+ Thread.sleep(100);
+ }
+
+ List<ServiceInstance> instances = service.lookup(SERVICE_NAME);
+ assertTrue(instances.size() > 0);
Review Comment:
[nitpick] Use `assertFalse(instances.isEmpty())` or
`assertTrue(!instances.isEmpty())` instead of `assertTrue(instances.size() >
0)` for better readability and consistency with other assertions in the test
file.
```suggestion
assertFalse(instances.isEmpty());
```
##########
discovery/seata-discovery-nacos/src/test/java/org/apache/seata/discovery/registry/nacos/NacosRegistryServiceImplTest.java:
##########
@@ -16,32 +16,233 @@
*/
package org.apache.seata.discovery.registry.nacos;
-import org.apache.seata.common.util.ReflectionUtil;
-import org.assertj.core.api.Assertions;
+import com.alibaba.nacos.api.naming.listener.Event;
+import com.alibaba.nacos.api.naming.listener.EventListener;
+import com.alibaba.nacos.api.naming.listener.NamingEvent;
+import com.alibaba.nacos.api.naming.pojo.Instance;
+import org.apache.seata.common.metadata.ServiceInstance;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.api.condition.EnabledIfSystemProperty;
import java.lang.reflect.Field;
-import java.lang.reflect.Method;
-import java.util.Properties;
+import java.net.InetSocketAddress;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
-/**
- * The type Nacos registry serivce impl test
- */
+@EnabledIfSystemProperty(named = "nacosCaseEnabled", matches = "true")
+@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
public class NacosRegistryServiceImplTest {
+ private static final String SERVICE_NAME = "default_tx_group";
+ private static final String CLUSTER_NAME = "default";
+
+ private static final NacosRegistryServiceImpl service =
NacosRegistryServiceImpl.getInstance();
+
+ @Test
+ public void testGetInstance() {
+ NacosRegistryServiceImpl instance =
NacosRegistryServiceImpl.getInstance();
+ assertInstanceOf(NacosRegistryServiceImpl.class, instance);
+ }
+
+ @Test
+ @Order(1)
+ public void testRegister() throws Exception {
+ InetSocketAddress address = new InetSocketAddress("127.0.0.1", 8091);
+ Map<String, Object> metadata = new HashMap<>();
+ metadata.put("version", "1.0.0");
+ metadata.put("weight", 1.0);
+ metadata.put("healthy", true);
+
+ ServiceInstance serviceInstance = new ServiceInstance(address,
metadata);
+
+ // Verify ServiceInstance metadata
+ assertNotNull(serviceInstance.getMetadata());
+ assertEquals("1.0.0", serviceInstance.getMetadata().get("version"));
+ assertEquals(1.0, serviceInstance.getMetadata().get("weight"));
+ assertEquals(true, serviceInstance.getMetadata().get("healthy"));
+
+ service.register(serviceInstance);
+
+ // Verify registration success
+ long startTime = System.currentTimeMillis();
+ while (service.lookup(SERVICE_NAME).isEmpty() &&
System.currentTimeMillis() - startTime < 10000) {
+ Thread.sleep(100);
+ }
+
+ List<ServiceInstance> instances = service.lookup(SERVICE_NAME);
+ assertTrue(instances.size() > 0);
+
+ // Cleanup
+ service.unregister(serviceInstance);
+ }
+
+ @Test
+ public void testRegisterWithInvalidAddress() {
+ assertThrows(IllegalArgumentException.class, () -> {
+ InetSocketAddress invalidAddress = new
InetSocketAddress("127.0.0.1", 0);
+ ServiceInstance invalidInstance = new
ServiceInstance(invalidAddress, new HashMap<>());
+ service.register(invalidInstance);
+ });
+ }
+
+ @Test
+ @Order(2)
+ public void testUnregister() throws Exception {
+ InetSocketAddress address = new InetSocketAddress("127.0.0.1", 8092);
+ Map<String, Object> metadata = new HashMap<>();
+ metadata.put("version", "1.0.0");
+ metadata.put("weight", 1.0);
+
+ ServiceInstance serviceInstance = new ServiceInstance(address,
metadata);
+ InetSocketAddress address1 = new InetSocketAddress("127.0.0.1", 8092);
+
+ ServiceInstance serviceInstance1 = new ServiceInstance(address1,
metadata);
+
+ // Verify ServiceInstance metadata
+ assertNotNull(serviceInstance.getMetadata());
+ assertEquals("1.0.0", serviceInstance.getMetadata().get("version"));
+ assertEquals(1.0, serviceInstance.getMetadata().get("weight"));
+
+ service.register(serviceInstance);
+ service.register(serviceInstance1);
+
+ long startTime = System.currentTimeMillis();
+ while (service.lookup(SERVICE_NAME).isEmpty() &&
System.currentTimeMillis() - startTime < 10000) {
+ Thread.sleep(100);
+ }
+
+ List<ServiceInstance> instancesBefore = service.lookup(SERVICE_NAME);
+ assertTrue(instancesBefore.size() > 0);
+
+ service.unregister(serviceInstance);
+
+ startTime = System.currentTimeMillis();
+ while (!service.lookup(SERVICE_NAME).isEmpty() &&
System.currentTimeMillis() - startTime < 10000) {
+ Thread.sleep(100);
+ }
+
+ // Verify unregistration success
+ List<ServiceInstance> instancesAfter = service.lookup(SERVICE_NAME);
+ assertTrue(instancesAfter.size() == 1);
Review Comment:
Replace `assertTrue(instancesAfter.size() == 1)` with `assertEquals(1,
instancesAfter.size())` for clearer assertion failure messages that show the
expected vs actual values.
```suggestion
assertEquals(1, instancesAfter.size());
```
##########
discovery/seata-discovery-nacos/src/test/java/org/apache/seata/discovery/registry/nacos/NacosRegistryServiceImplTest.java:
##########
@@ -16,32 +16,233 @@
*/
package org.apache.seata.discovery.registry.nacos;
-import org.apache.seata.common.util.ReflectionUtil;
-import org.assertj.core.api.Assertions;
+import com.alibaba.nacos.api.naming.listener.Event;
+import com.alibaba.nacos.api.naming.listener.EventListener;
+import com.alibaba.nacos.api.naming.listener.NamingEvent;
+import com.alibaba.nacos.api.naming.pojo.Instance;
+import org.apache.seata.common.metadata.ServiceInstance;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.api.condition.EnabledIfSystemProperty;
import java.lang.reflect.Field;
-import java.lang.reflect.Method;
-import java.util.Properties;
+import java.net.InetSocketAddress;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
-/**
- * The type Nacos registry serivce impl test
- */
+@EnabledIfSystemProperty(named = "nacosCaseEnabled", matches = "true")
+@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
public class NacosRegistryServiceImplTest {
+ private static final String SERVICE_NAME = "default_tx_group";
+ private static final String CLUSTER_NAME = "default";
+
+ private static final NacosRegistryServiceImpl service =
NacosRegistryServiceImpl.getInstance();
+
+ @Test
+ public void testGetInstance() {
+ NacosRegistryServiceImpl instance =
NacosRegistryServiceImpl.getInstance();
+ assertInstanceOf(NacosRegistryServiceImpl.class, instance);
+ }
+
+ @Test
+ @Order(1)
+ public void testRegister() throws Exception {
+ InetSocketAddress address = new InetSocketAddress("127.0.0.1", 8091);
+ Map<String, Object> metadata = new HashMap<>();
+ metadata.put("version", "1.0.0");
+ metadata.put("weight", 1.0);
+ metadata.put("healthy", true);
+
+ ServiceInstance serviceInstance = new ServiceInstance(address,
metadata);
+
+ // Verify ServiceInstance metadata
+ assertNotNull(serviceInstance.getMetadata());
+ assertEquals("1.0.0", serviceInstance.getMetadata().get("version"));
+ assertEquals(1.0, serviceInstance.getMetadata().get("weight"));
+ assertEquals(true, serviceInstance.getMetadata().get("healthy"));
+
+ service.register(serviceInstance);
+
+ // Verify registration success
+ long startTime = System.currentTimeMillis();
+ while (service.lookup(SERVICE_NAME).isEmpty() &&
System.currentTimeMillis() - startTime < 10000) {
+ Thread.sleep(100);
+ }
+
+ List<ServiceInstance> instances = service.lookup(SERVICE_NAME);
+ assertTrue(instances.size() > 0);
+
+ // Cleanup
+ service.unregister(serviceInstance);
+ }
+
+ @Test
+ public void testRegisterWithInvalidAddress() {
+ assertThrows(IllegalArgumentException.class, () -> {
+ InetSocketAddress invalidAddress = new
InetSocketAddress("127.0.0.1", 0);
+ ServiceInstance invalidInstance = new
ServiceInstance(invalidAddress, new HashMap<>());
+ service.register(invalidInstance);
+ });
+ }
+
+ @Test
+ @Order(2)
+ public void testUnregister() throws Exception {
+ InetSocketAddress address = new InetSocketAddress("127.0.0.1", 8092);
+ Map<String, Object> metadata = new HashMap<>();
+ metadata.put("version", "1.0.0");
+ metadata.put("weight", 1.0);
+
+ ServiceInstance serviceInstance = new ServiceInstance(address,
metadata);
+ InetSocketAddress address1 = new InetSocketAddress("127.0.0.1", 8092);
Review Comment:
The test creates two ServiceInstance objects with the same address
(127.0.0.1:8092). Since `address` on line 106 and `address1` on line 112 both
use port 8092, `serviceInstance` and `serviceInstance1` are identical, which
means registering both doesn't test the unregister behavior properly. One
should use a different port (e.g., 8093) to properly test that unregistering
one instance leaves the other registered.
```suggestion
InetSocketAddress address1 = new InetSocketAddress("127.0.0.1",
8093);
```
##########
discovery/seata-discovery-nacos/src/test/java/org/apache/seata/discovery/registry/nacos/NacosRegistryServiceImplTest.java:
##########
@@ -16,32 +16,233 @@
*/
package org.apache.seata.discovery.registry.nacos;
-import org.apache.seata.common.util.ReflectionUtil;
-import org.assertj.core.api.Assertions;
+import com.alibaba.nacos.api.naming.listener.Event;
+import com.alibaba.nacos.api.naming.listener.EventListener;
+import com.alibaba.nacos.api.naming.listener.NamingEvent;
+import com.alibaba.nacos.api.naming.pojo.Instance;
+import org.apache.seata.common.metadata.ServiceInstance;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.api.condition.EnabledIfSystemProperty;
import java.lang.reflect.Field;
-import java.lang.reflect.Method;
-import java.util.Properties;
+import java.net.InetSocketAddress;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
-/**
- * The type Nacos registry serivce impl test
- */
+@EnabledIfSystemProperty(named = "nacosCaseEnabled", matches = "true")
+@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
public class NacosRegistryServiceImplTest {
+ private static final String SERVICE_NAME = "default_tx_group";
+ private static final String CLUSTER_NAME = "default";
+
+ private static final NacosRegistryServiceImpl service =
NacosRegistryServiceImpl.getInstance();
+
+ @Test
+ public void testGetInstance() {
+ NacosRegistryServiceImpl instance =
NacosRegistryServiceImpl.getInstance();
+ assertInstanceOf(NacosRegistryServiceImpl.class, instance);
+ }
+
+ @Test
+ @Order(1)
+ public void testRegister() throws Exception {
+ InetSocketAddress address = new InetSocketAddress("127.0.0.1", 8091);
+ Map<String, Object> metadata = new HashMap<>();
+ metadata.put("version", "1.0.0");
+ metadata.put("weight", 1.0);
+ metadata.put("healthy", true);
+
+ ServiceInstance serviceInstance = new ServiceInstance(address,
metadata);
+
+ // Verify ServiceInstance metadata
+ assertNotNull(serviceInstance.getMetadata());
+ assertEquals("1.0.0", serviceInstance.getMetadata().get("version"));
+ assertEquals(1.0, serviceInstance.getMetadata().get("weight"));
+ assertEquals(true, serviceInstance.getMetadata().get("healthy"));
+
+ service.register(serviceInstance);
+
+ // Verify registration success
+ long startTime = System.currentTimeMillis();
+ while (service.lookup(SERVICE_NAME).isEmpty() &&
System.currentTimeMillis() - startTime < 10000) {
+ Thread.sleep(100);
+ }
+
+ List<ServiceInstance> instances = service.lookup(SERVICE_NAME);
+ assertTrue(instances.size() > 0);
+
+ // Cleanup
+ service.unregister(serviceInstance);
+ }
+
+ @Test
+ public void testRegisterWithInvalidAddress() {
+ assertThrows(IllegalArgumentException.class, () -> {
+ InetSocketAddress invalidAddress = new
InetSocketAddress("127.0.0.1", 0);
+ ServiceInstance invalidInstance = new
ServiceInstance(invalidAddress, new HashMap<>());
+ service.register(invalidInstance);
+ });
+ }
+
+ @Test
+ @Order(2)
+ public void testUnregister() throws Exception {
+ InetSocketAddress address = new InetSocketAddress("127.0.0.1", 8092);
+ Map<String, Object> metadata = new HashMap<>();
+ metadata.put("version", "1.0.0");
+ metadata.put("weight", 1.0);
+
+ ServiceInstance serviceInstance = new ServiceInstance(address,
metadata);
+ InetSocketAddress address1 = new InetSocketAddress("127.0.0.1", 8092);
+
+ ServiceInstance serviceInstance1 = new ServiceInstance(address1,
metadata);
+
+ // Verify ServiceInstance metadata
+ assertNotNull(serviceInstance.getMetadata());
+ assertEquals("1.0.0", serviceInstance.getMetadata().get("version"));
+ assertEquals(1.0, serviceInstance.getMetadata().get("weight"));
+
+ service.register(serviceInstance);
+ service.register(serviceInstance1);
+
+ long startTime = System.currentTimeMillis();
+ while (service.lookup(SERVICE_NAME).isEmpty() &&
System.currentTimeMillis() - startTime < 10000) {
+ Thread.sleep(100);
+ }
+
+ List<ServiceInstance> instancesBefore = service.lookup(SERVICE_NAME);
+ assertTrue(instancesBefore.size() > 0);
Review Comment:
[nitpick] Use `assertFalse(instancesBefore.isEmpty())` instead of
`assertTrue(instancesBefore.size() > 0)` for better readability and consistency
with other assertions.
```suggestion
assertFalse(instancesBefore.isEmpty());
```
--
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]
