javanna commented on PR #13609: URL: https://github.com/apache/lucene/pull/13609#issuecomment-2255481310
I believe this change is ok, but I am no longer very confident about `TestTaskExecutor#testCancelTasksOnException` testing a real-life scenario, because it was recently changed to no longer rely on executorService. If I restore that, I get some failures here and there (before as well as after this change). It seems to me that the test was simplified by removing concurrency, and that changes entirely what the test ends up testing. Additionally, could we have a test around memory being released? (does not have to be perfect, but just something that verifies that the result is nullified? -- 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: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
