mjsax commented on PR #23010:
URL: https://github.com/apache/kafka/pull/23010#issuecomment-5272212264
Claude has a few test suggestions:
```
Test hygiene
shouldFailTaskAddedWhileStateUpdaterThreadIsStopping leaves
releaseStoppingThread un-counted-down if any assertion between the two latches
fails. The state updater thread then stays parked in the mocked clear(), so
shutdownThread burns its 1-minute join and @AfterEach's
stateUpdater.shutdown(Duration.ofMinutes(1)) burns another and throws
StreamsException — masking the real assertion failure and costing ~2 minutes. A
try { … } finally { releaseStoppingThread.countDown(); } fixes it.
Leftover nits from last round, still open
- StateUpdater#tasks()'s javadoc lists the methods that remove tasks from
the updater; drainQueuedTasks() isn't in the list.
- The new javadoc still doesn't say this is shutdown-only. That matters a
bit more now: #23093 added fatalException() and
maybeThrowFatalExceptionFromStateUpdater(), so a stray call while the updater
is running would swallow failed tasks that handleExceptionsFromStateUpdater
needs to surface.
-
result.addAll(drainExceptionsAndFailedTasks().stream().map(...).collect(toList()))
→ drainExceptionsAndFailedTasks().forEach(e -> result.add(e.task())).
- TaskManager.java:1542-1543 — // And if there is anything left unhandled
due to timeouts, handling now too reads as a fragment continuing the line
above; and "due to timeouts" is now stale wording since #23093 removed the
timeout.
```
--
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]
