Repository navigation
[ISSUE #7401] Start heartbeat task in failure isolation test - #7402
Conversation
|
CI follow-up: all triggered checks now pass, including the Kubernetes Gateway API integration job. The configured skips are unchanged. The focused |
Aias00
left a comment
There was a problem hiding this comment.
Correct fix for #7401. ShenyuClientURIExecutorSubscriber.start() — not the constructor — is what enqueues the periodic heartbeat task, and it is guarded by started.compareAndSet(false, true). Without this call executor.getQueue() is empty, so executor.getQueue().iterator().next() at line 57 throws NoSuchElementException. Calling start() after the URIs are registered is the natural placement.
The rest of the test is sound: the task is only ever executed manually (queue.remove(task); task.run();), assertFalse(task.isDone(), ...) is the real regression assertion for #7401, and executor.shutdownNow() in the finally keeps the heartbeat thread from leaking.
Nit: #7403 is the same one-line change, placed just after construction instead. Please coordinate and close one of the two.
Fixes #7401.
HeartbeatFailureIsolationTestreads a scheduled task from the executor queue, but the test never started the subscriber, so no periodic task was scheduled. Start the subscriber before accessing the queue; the test can then invoke the scheduled task manually and verify that a failing heartbeat does not suppress other URIs or subsequent ticks.Tests:
./mvnw -B -pl shenyu-client/shenyu-client-core -Djacoco.skip=true -Dmaven.javadoc.skip=true test(101 tests passed; Checkstyle clean).