test(framework): fix test isolation and lifecycle defects

[warku123]

What does this PR do?

Fixes test-isolation and lifecycle defects in framework tests so the full test chain can run reliably with retry disabled (retry=0). Today several tests fail on the first attempt and only pass via test-retry, hiding real failures and making JaCoCo coverage non-deterministic. Unless marked victim-side, every change is made in the writer — the test that leaked state or resources — so victims need no defensive code.

1. Backup resource leak (writer-side). What: BackupManagerTest and BackupServerTest now fully close the BackupServer channel/event loop and executors they create. How: keep manager/server instances; exception-safe @After (server close, fallback manager stop, then DNS/Args restore); condition-based readiness instead of fixed sleeps; assert channel/executor termination. Previously they left a BackupServer:WAITING thread per run, and BackupServerTest#test hit a 60 s teardown timeout in the same worker JVM.

2. Transactions async/executor lifecycle (writer and victim are the same class). What: TransactionsMsgHandlerTest no longer injects its private same-named TrxEvent into the production queue, waits deterministically for the scheduler, closes every handler/pool, and asserts on the correct map. How: real production events, a CountDownLatch, finally cleanup including the original executor replaced via reflection, and corrected assertions. Previously it caused a scheduler ClassCastException and its own coverage of lambda$handleSmartContract$2 flipped between zero and partial.

3. VM thread-local snapshot leak (writer-side). What: TransferToAccountTest and RuntimeImplTest now clear the thread-local VMConfig snapshot they install (in finally / @After). How: both run VMActuator in isolated/constant mode, which installs a snapshot that VMConfig.current() prefers over the global one; an audit of all test-side TransactionContext/RuntimeImpl/VMActuator construction found these are the only two unbalanced writers. Victims: AllowTvmLondonTest#testBaseFee, AllowTvmLondonTest#testStartWithEF, and ValidateMultiSignContractTest#testTip854RejectsMalformedCalldata — the three tests that first-failed in 5/5 identical baseline CI runs and only passed via retry.

4. ConfigLoader.disable leak (writer-side). What: IstanbulTest and AllowTvmLondonTest now restore ConfigLoader.disable in @After, matching the convention used by eleven other test classes. How: the flag makes ConfigLoader.load() a no-op, freezing VMConfig for every later DPS-driven test in the same worker JVM. Victim: any subsequent DPS-driven VM test — proven with an ordered suite using a synthetic DPS-only London victim (polluted order failed 3/3; cleanup, reverse, and victim-only controls passed); HistoryBlockHashVmTest and BlockEventGetTest already carry defensive resets for exactly this leak.

5. DPS fork-flag leaks (writer-side). What: UnfreezeBalanceActuatorTest, Create2Test, and TransferFailedEnergyTest now snapshot in @Before and restore in @After only the DPS flags they modify (allowTvmConstantinople, allowTvmSolidity059, allowTvmIstanbul, allowTvmTransferTrc10). How: their unrestored writes became live input to later VM executions once defect 4 was fixed. BandWidthRuntimeTest was the initially observed victim; commit 435e40e759 and verified commit 96d5c35d43 removed its three explicit proposal initializations, so BandWidth no longer has defensive initialization. The latest three retry=0 rounds prove the writer-side snapshot/restore fix is sufficient.

6. allowShieldedTransaction hygiene (writer-side). What: SendCoinShieldTest and ShieldedReceiveTest now snapshot and restore the allowShieldedTransaction DPS flag. No victim is confirmed — this is shared-state hygiene, not the anchor-race cause below.

7. Shielded consensus lifecycle (writer and victim are the same class). What: ShieldedReceiveTest no longer runs a class-wide consensus producer; only the two tests that need an initialized witness schedule start and stop consensus, via a synchronized balanced start()/stop() helper, and the reflection-based DposTask restart workaround is removed. How: background block production could advance the head and roll back the snapshot between a test's Merkle anchor write and transaction validation. Victims: its own methods — testSignWithoutFromAddress, testSignWithoutToAmount, testMemoNotEnough, testSameOutputCm, and testIsolateSignature failed intermittently with Rt is invalid. across CI runs.

8. testStop timeout (victim-side). What: ConditionallyStopTest#testStop's JUnit timeout changes from 30 s to 45 s; test behavior and logic remain unchanged. The test still generates and signs the real 512+1 blocks. Reason: measured durations on loaded GitHub free shared runners reached 32.6 s, so 30 s was below the observed runtime rather than a sign of a defect in the test logic.

9. System.out leak (writer-side). What: BroadcastServletTest and GetTransactionByIdSolidityServletTest now restore the process-wide System.out stream after every test. How: each class snapshots the original PrintStream in @Before and restores it at the beginning of @After, before teardown logging. No failing victim is confirmed; however, leaving the capture stream installed can affect every later test in the same worker JVM, and KeystoreFactoryDeprecationTest can propagate it by saving the already-polluted stream as its own "original" stream.

10. PeerManager process-wide static state (infrastructure-side reset). What: a new test utility PeerManagerStateResetter returns PeerManager's statics to a cold-JVM state before p2p tests — it clears the raw peer list (including disconnected and null-channel phantoms invisible to getPeers()), resets the active/passive counters, and rebuilds the scheduled executor only when it is null or already shut down. How: wired broadly into BaseTest/BaseMethodTest @Before, covering subclasses that reuse Spring contexts or the JVM, plus @BeforeClass in the five standalone p2p test classes that bypass those bases (WalletApiTest, HandShakeServiceTest, MessageHandlerTest, PbftMsgHandlerTest, PeerManagerTest); static reflection is localized inside PeerManagerStateResetter, and shared ReflectUtils remains unchanged. Why: broad wiring protects against unknown preceding process-wide pollution. For tests that do not use PeerManager, reset is idempotent and low-impact — typically clearing an empty list and zeroing counters, with executor replacement only when dead or null and threads created on demand. The wiring may be narrowed later if evidence supports it, but no narrowing is promised here. Victims were confirmed by fault-injection suites that poison exactly one mechanism per JVM: MetricsApiServiceTest#testProcessMessage hit RejectedExecutionException after executor shutdown; PeerStatusCheckTest, HandShakeServiceTest, MessageHandlerTest, PbftMsgHandlerTest, and ResilienceServiceTest failed on residual peers/counters; MessageHandlerTest, PbftMsgHandlerTest, and NodeInfoServiceTest threw NPEs on null-channel peers.

Why are these changes required?

First-attempt failures hidden by retry are real signal: the leaked thread-local snapshot changes which production branches execute (e.g. London-dependent validation), so test outcomes and JaCoCo counters varied with worker assignment. ConfigLoader.disable and unrestored DPS flags are process-wide state that silently affects every later VM test in the same worker. The Shielded race was confirmed directly: instrumentation showed the anchor present right after put but absent before validation while manager/store identities stayed the same and the head/snapshot advanced; with the producer stopped the failure did not occur. Finally, testStop intentionally does expensive real work, and 30 s is below its observed runtime under shared-runner load, so a bounded timeout increase is preferable to weakening the test.

This PR has been tested by:

• Unit Tests
  • Fork-only JDK 8 / Linux x86_64 verifier, retry disabled, default maxParallelForks/forkEvery: verifier commit 96d5c35d43, run 31467609469, completed three independent full clean build → :framework:testWithRocksDb → jacocoTestReport chains successfully, including the localized PeerManager reflection adjustment.
  • Targeted evidence per fix: writer→victim ordered suites fail before and pass after cleanup (including a synthetic-leaker negative control); single-mechanism fault-injection suites confirmed the PeerManager executor/peer/null-channel victims; focused reruns of every touched class pass; the testStop durations were 24.9 s, 28.2 s, and 32.6 s.
• Manual Testing

Follow up

• A general shared-state listener that snapshots global state before/after tests and reports unbalanced changes.
• Evaluate production PeerManager.close()/init() lifecycle so close() clears raw peers and counters, and init() safely rebuilds or restarts an executor after shutdown. After validating production semantics and concurrency, remove or significantly narrow PeerManagerStateResetter.
• JaCoCo reporting issues unrelated to these fixes: module reports reading the wrong execution data, base reports continuing after test failure, and missing report task dependencies.

Extra details

No production code is changed; all modifications are test code under framework/src/test/**. This PR does not change any CI configuration — the existing retry settings stay untouched; whether to disable retry in production CI is a separate decision. What this PR delivers is the prerequisite: a test suite whose first-attempt results are trustworthy, verified by running the full CI test chain with retry disabled, so future first-attempt failures stay visible instead of being masked by retry.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}