test(framework): fix test quality issues by Little-Peony · Pull Request #6655 · tronprotocol/java-tron

Little-Peony · 2026-04-08T07:46:22Z

What does this PR do?

Systematically fix five categories of test quality issues across framework, actuator, and plugins modules (44 files, 6 commits):

1. Silent-pass tests

Add Assert.fail() after code expected to throw exceptions (FetchInvDataMsgHandlerTest, MerkleTreeTest, CredentialsTest, etc.)
Fix assertEquals(actual, expected) argument order → assertEquals(expected, actual)

2. Thread / resource leaks

Replace bare Executors.newFixedThreadPool() / new Thread[] with ExecutorServiceManager in LogBlockQueryTest, TransactionRegisterTest, ConcurrentHashMapTest; add shutdownAndAwaitTermination
Fix InterruptedException swallowed silently → restore interrupt flag in PropUtilTest, DBIteratorTest, SnapshotImplTest, ProposalControllerTest, BlockStoreTest, PbftTest
Fix SolidityNode not shutting down correctly in SolidityNodeTest; patch SolidityNode.java shutdown path

3. Dead code / phantom fixtures

Remove @After methods cleaning paths never written by any test (DbMoveTest)
Remove never-called private static helpers (writeProperty(), deleteDir()) in ArchiveManifestTest, DbArchiveTest
Remove unreachable test code in FreezeTest, EventParserTest, RelayServiceTest, etc.

4. Stale / obsolete tests

Remove moreThanFrozenNumber test in FreezeBalanceActuatorTest — the corresponding actuator error path no longer exists

5. Test infrastructure unification

Extract ClassLevelAppContextFixture (with shutdownChannel / shutdownChannels helpers) to unify AppContext and gRPC channel lifecycle across service tests
Migrate CredentialsTest from JUnit 3 (junit.framework.TestCase) to JUnit 4 + Mockito; remove dependency on real SecureRandom
Fix package typo: keystroe → keystore
Add Mockito.verify() assertions in InventoryMsgHandlerTest, PeerStatusCheckMockTest

Why are these changes required?

These issues caused tests to pass silently even when the code under test was broken, leaked threads between test runs (causing flakiness in parallel execution), and kept dead code that obscured the real test intent.

This PR has been tested by:

All changed tests pass locally (./gradlew :framework:test :actuator:test :plugins:test)

Breaking Changes

None. Test-only changes except SolidityNode.java shutdown fix (additive only).

- FetchInvDataMsgHandlerTest: add Assert.fail() before catch blocks so tests fail when expected exceptions are not thrown; fix assertEquals argument order (expected, actual) - ConcurrentHashMapTest: restore commented-out Assert.fail() for ItemNotFoundException; replace printStackTrace() with interrupt flag restoration and proper logging; fail main thread on interrupted join - InventoryMsgHandlerTest: add Mockito.verify() to assert isBlockUnsolidified() is called in the unsolidified-block scenario - PeerStatusCheckMockTest: add Mockito.verify() to assert statusCheck() is invoked by the scheduler after init() - MerkleTreeTest: replace Assert.assertFalse(true) with Assert.fail() for clearer failure message Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@after

- DbLiteTest: remove dead null-dbPath reassignment before init() call; restore thread interrupt flag on InterruptedException in generateSomeTransactions() - DbMoveTest: remove legacy @after destroy() that cleaned a hardcoded path never written by any test; remove dead deleteDir() helper and OUTPUT_DIRECTORY constant; remove now-unused After import - DbTest: narrow INPUT_DIRECTORY and cli fields from public to protected - ArchiveManifestTest, DbArchiveTest: remove copy-pasted writeProperty() and deleteDir() private static methods that were never called; clean up unused imports - DbLiteRocksDbV2Test: rename testToolsWithRocksDB to testToolsWithRocksDbV2 to match the LevelDB v1/v2 naming convention Note: testMvForRocksDB is a pre-existing failure unrelated to these changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…lanceActuatorTest The method tested a "max frozen number is: X" error that no longer exists in FreezeBalanceActuator. The actuator was updated to check "frozenCount must be 0 or 1" instead, and a second freeze with frozenCount=1 now succeeds rather than throwing. Remove the now-unused ChainConstant import. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…nd fix test quality issues - Replace Executors.newFixedThreadPool / bare new Thread[] with ExecutorServiceManager in LogBlockQueryTest, TransactionRegisterTest, ConcurrentHashMapTest; add proper shutdownAndAwaitTermination - Fix InterruptedException handling: restore interrupt flag instead of swallowing in PropUtilTest, DBIteratorTest, SnapshotImplTest, ProposalControllerTest, BlockStoreTest, PbftTest - Remove dead/unreachable test code in FreezeTest, EventParserTest, EventParserJsonTest, BlockFilterCapsuleTest, MessageTest, RelayServiceTest, DelegationServiceTest - Fix package typo: rename keystroe.CredentialsTest → keystore.CredentialsTest; rewrite test using JUnit Assert and Mockito instead of junit.framework.TestCase - Minor cleanup: unused imports, whitespace, AccountAssetStoreTest assertion style Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Move to local .git/info/exclude instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ityNodeTest Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

halibobo1205 · 2026-04-08T07:53:21Z

framework/src/test/java/org/tron/core/db/AccountAssetStoreTest.java

      ownerCapsule.addAssetV2(ByteArray.fromString(String.valueOf(id)), TOTAL_SUPPLY);
    } catch (Exception e) {
-      e.printStackTrace();
+      Assert.fail(e.getMessage());


Is it necessary to catch the Exception here? What exception could addAssetV2 actually throw?

halibobo1205 · 2026-04-08T08:00:42Z

framework/src/test/java/org/tron/core/db/DBIteratorTest.java

-        iterator.seekToLast();
-      } catch (Exception e) {
-        Assert.assertTrue(e instanceof  IllegalStateException);
+      try (RockStoreIterator iterator =


new ReadOptions() is a JNI-backed object that requires explicit closing. Here it's passed inline and never closed, leaking native memory.

halibobo1205 · 2026-04-08T08:03:27Z

framework/src/test/java/org/tron/core/net/messagehandler/FetchInvDataMsgHandlerTest.java

        .thenReturn(new BlockCapsule.BlockId(Sha256Hash.ZERO_HASH, 1000L));
      fetchInvDataMsgHandler.processMessage(peer, msg);
+      Assert.fail("Expected exception was not thrown: maxBlockNum: 1000, blockNum: 10000");
    } catch (Exception e) {


Exception e1 = assertThrows(Exception.class, () -> fetchInvDataMsgHandler.processMessage(peer, msg)); assertEquals("maxBlockNum: 1000, blockNum: 10000", e1.getMessage());

plugins/src/test/java/org/tron/plugins/DbMoveTest.java

halibobo1205 · 2026-04-08T08:30:11Z

framework/src/main/java/org/tron/program/SolidityNode.java

  }

  private Block getBlockByNum(long blockNum) {
    while (true) {


while (true) -> while (true && flag)? ,same as getLastSolidityBlockNum.

@after

- BandWidthRuntimeTest: deploy contract before setDebug(true) to avoid CPU timeout triggered by debug mode during contract deployment - TronStatsManagerTest: capture P2pStats snapshot before invoking work() and compare against snapshot instead of hardcoded 0, preventing failures when another test starts the real P2pService with non-zero stats - DbMoveTest: add @after cleanup of output-directory-toolkit so LevelDB and RocksDB tests don't share the same relative destination path - PeerStatusCheckMockTest: replace 5-second sleep with mock executor; verify scheduleWithFixedDelay args and run the captured Runnable directly Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ection Extract relayNodes from a local variable inside setChannel() to a lazy- initialized instance field. setChannel() now only fetches from Args when the field is still null, so tests can pre-inject a custom list via reflection before calling setChannel(). This restores testability of PeerManagerTest without touching the test itself. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Little-Peony and others added 10 commits April 6, 2026 16:23

test(framework): fix test cleanup and solidity node shutdown

485fbc2

test(framework): unify app context fixtures in service tests

9a33391

style(test): fix import order to satisfy checkstyle

c99df5e

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

chore(config): remove /.dev/ from .gitignore

8d22ffb

Move to local .git/info/exclude instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

style(test): fix import order in SolidityNodeTest

b17f73d

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

test(framework): use assertSame for exception identity check in Solid…

441c716

…ityNodeTest Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Little-Peony requested review from bladehan1, halibobo1205 and xxo1shine as code owners April 8, 2026 07:46

Sunny6889 changed the title ~~test: fix test quality issues in framework, actuator and plugins~~ test(framework): fix test quality issues Apr 8, 2026

halibobo1205 reviewed Apr 8, 2026

View reviewed changes

Sunny6889 requested a review from 3for April 8, 2026 07:53

halibobo1205 reviewed Apr 8, 2026

View reviewed changes

plugins/src/test/java/org/tron/plugins/DbMoveTest.java Show resolved Hide resolved

fix(net): guard relay nodes in peer connection

a8d650f

halibobo1205 reviewed Apr 8, 2026

View reviewed changes

Little-Peony requested a review from 317787106 as a code owner April 8, 2026 09:22

Little-Peony force-pushed the fix_tests_v2 branch from 76fe5da to 9d99aca Compare April 8, 2026 09:43

bladehan1 approved these changes Apr 8, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(framework): fix test quality issues#6655

test(framework): fix test quality issues#6655
Little-Peony wants to merge 13 commits intotronprotocol:developfrom
Little-Peony:fix_tests_v2

Little-Peony commented Apr 8, 2026

Uh oh!

halibobo1205 Apr 8, 2026

Uh oh!

halibobo1205 Apr 8, 2026

Uh oh!

halibobo1205 Apr 8, 2026

Uh oh!

Uh oh!

halibobo1205 Apr 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Little-Peony commented Apr 8, 2026

Uh oh!

halibobo1205 Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

halibobo1205 Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

halibobo1205 Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

halibobo1205 Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants