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:
|
DanySK
force-pushed
the
test/incarnation-sapere/hardenlsanodecon
branch
6 times, most recently
from
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". |
DanySK
force-pushed
the
test/incarnation-sapere/hardenlsanodecon
branch
2 times, most recently
from
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).
…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>
DanySK
force-pushed
the
test/incarnation-sapere/hardenlsanodecon
branch
from
2aa54ea to
22643a8
Compare
|
No description provided.