test(incarnation-sapere): harden LsaNode concurrency test#5325
test(incarnation-sapere): harden LsaNode concurrency test#5325
Conversation
There was a problem hiding this comment.
Pull request overview
This PR strengthens the SAPERE incarnation’s LsaNode concurrency test by explicitly tracking submitted tasks and waiting on their completion via Future#get, aiming to make concurrency failures more reliably detectable.
Changes:
- Track submitted executor tasks via
List<Future<?>>and wait for completion with a timeout. - Convert previously in-task exception handling into main-thread handling via
Future#get. - Adjust writer-thread molecule naming used during concurrent modifications.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5325 +/- ##
=========================================
Coverage 61.53% 61.53%
Complexity 14 14
=========================================
Files 2 2
Lines 78 78
Branches 4 4
=========================================
Hits 48 48
Misses 24 24
Partials 6 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
6690be6 to
c78d86a
Compare
Head branch was pushed to by a user without write access
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
8c4b6f9 to
39cdeac
Compare
| } finally { | ||
| executor.shutdownNow(); | ||
| try { | ||
| if (!executor.awaitTermination(TIMEOUT_SECONDS, TimeUnit.SECONDS)) { | ||
| throw new AssertionError("Executor did not terminate within " + TIMEOUT_SECONDS + " seconds"); | ||
| } | ||
| } catch (final InterruptedException e) { | ||
| Thread.currentThread().interrupt(); | ||
| throw new AssertionError("Interrupted while waiting for executor termination", e); | ||
| } |
There was a problem hiding this comment.
The finally block can throw a new AssertionError if awaitTermination fails, which may mask an earlier failure from latch.await(...) or a task.get(...) exception (the original assertion/stack trace gets replaced). Consider capturing the primary failure, then performing shutdown/awaitTermination, and only throwing a termination error if there was no prior failure (or add it as a suppressed exception).
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Fixed in 534e3c4. The try block now has an outer catch (AssertionError e) that stores the primary failure. In the finally block, any termination error is added as a suppressed exception on primaryFailure (via addSuppressed) if one exists, so the original stack trace is always preserved. After finally, primaryFailure is rethrown if set.
22643a8 to
caa5285
Compare
087bccb to
5e67efd
Compare
…model/sapere/nodes/LsaNodeConcurrencyTest.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…model/sapere/nodes/LsaNodeConcurrencyTest.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…List import, ensure executor shutdown in finally Agent-Logs-Url: https://github.com/AlchemistSimulator/Alchemist/sessions/6651c551-7d20-4a2c-aebc-a2f43f4054f8 Co-authored-by: DanySK <1991673+DanySK@users.noreply.github.com>
…xplicitly Agent-Logs-Url: https://github.com/AlchemistSimulator/Alchemist/sessions/6651c551-7d20-4a2c-aebc-a2f43f4054f8 Co-authored-by: DanySK <1991673+DanySK@users.noreply.github.com>
…model/sapere/nodes/LsaNodeConcurrencyTest.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
5e67efd to
c965fa7
Compare
…nation also fails If the try block (latch.await or task.get) throws an AssertionError, it is captured as primaryFailure. In the finally block, shutdownNow + awaitTermination runs unconditionally; any termination error is added as a suppressed exception on primaryFailure (preserving its stack trace) rather than replacing it. If there was no prior failure, the termination error is thrown directly as before. Agent-Logs-Url: https://github.com/AlchemistSimulator/Alchemist/sessions/5a82b660-ae48-43f3-a76d-95df9ff49832 Co-authored-by: DanySK <1991673+DanySK@users.noreply.github.com>
Head branch was pushed to by a user without write access
|



No description provided.