Copilot commented on code in PR #7044:
URL: https://github.com/apache/shenyu/pull/7044#discussion_r4023388102
##########
shenyu-registry/shenyu-registry-consul/src/test/java/org/apache/shenyu/registry/consul/ConsulInstanceRegisterRepositoryTest.java:
##########
@@ -115,4 +120,44 @@ public void testSelectInstancesAndWatcher() {
repository.close();
}
}
+
+ @Test
+ public void testCloseShutsDownExecutors() throws NoSuchFieldException,
IllegalAccessException {
+ final TtlScheduler ttlScheduler = new TtlScheduler(60,
mock(ConsulClient.class));
+ final Field ttlExecutorField =
TtlScheduler.class.getDeclaredField("scheduler");
+ ttlExecutorField.setAccessible(true);
+ final ScheduledExecutorService ttlExecutor =
(ScheduledExecutorService) ttlExecutorField.get(ttlScheduler);
+
+ final Field executorField =
ConsulInstanceRegisterRepository.class.getDeclaredField("executor");
+ executorField.setAccessible(true);
+ final ScheduledThreadPoolExecutor executor =
(ScheduledThreadPoolExecutor) executorField.get(repository);
+
+ final NewService service = new NewService();
+ service.setId("test-service");
+ final Field serviceField =
ConsulInstanceRegisterRepository.class.getDeclaredField("newService");
+ serviceField.setAccessible(true);
+ serviceField.set(repository, service);
+ final Field ttlSchedulerField =
ConsulInstanceRegisterRepository.class.getDeclaredField("ttlScheduler");
+ ttlSchedulerField.setAccessible(true);
+ ttlSchedulerField.set(repository, ttlScheduler);
+ final Field watchDelayField =
ConsulInstanceRegisterRepository.class.getDeclaredField("watchDelay");
+ watchDelayField.setAccessible(true);
+ watchDelayField.set(repository, "60");
+
+ ttlScheduler.add(service.getId());
+ repository.watcherStart("test-service");
+ try {
+ assertFalse(executor.isShutdown());
+ assertFalse(ttlExecutor.isShutdown());
+
+ repository.close();
+
+ assertAll(
+ () -> assertTrue(executor.isShutdown()),
+ () -> assertTrue(ttlExecutor.isShutdown()));
Review Comment:
This test only exercises a successful `agentServiceDeregister` (the mock has
no throwing behavior), so it does not protect the new `finally` guarantee
described by the PR. If cleanup were moved back into the success path, this
test would still pass; add a case that makes deregistration throw and asserts
that both executors are shut down.
--
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]
