Refactor web3 tests code #10293

filev94 · 2025-02-04T14:20:01Z

Description:

Removed similar code from several tests and put it in parent class so that if a change is needed it should be done in one place only. This is the first PR, more might be needed if i find more code that needs to be extracted in a similar way.

Related issue(s):

Fixes #10255

Notes for reviewer:

Checklist

Documented (Code comments, README, etc.)
Tested (unit, integration, etc.)

Signed-off-by: filev94 <todor.filev@limechain.tech>

# Conflicts: # hedera-mirror-web3/src/test/java/com/hedera/mirror/web3/service/ContractCallDynamicCallsTest.java # hedera-mirror-web3/src/test/java/com/hedera/mirror/web3/service/OpcodeServiceTest.java

Signed-off-by: filev94 <todor.filev@limechain.tech>

bilyana-gospodinova · 2025-02-04T14:36:12Z

...a-mirror-web3/src/test/java/com/hedera/mirror/web3/service/ContractCallDynamicCallsTest.java

-                .customize(e -> e.evmAddress(null))
-                .persist()
-                .toEntityId();
+        return accountEntityPersistCustomizable(e -> e.evmAddress(null)).toEntityId();


This test is an example why we needed this refactoring. Usually we want both alias and evmAddress to be null or non-null or with the reusable services integration this will probably fail. We can use directly accountEntityPersist method from the parent.

Changed from customizable method to accountEntityPersist

...a-mirror-web3/src/test/java/com/hedera/mirror/web3/service/ContractCallDynamicCallsTest.java

bilyana-gospodinova · 2025-02-04T14:39:00Z

...ra-mirror-web3/src/test/java/com/hedera/mirror/web3/service/ContractCallNestedCallsTest.java

@@ -84,7 +84,7 @@ class ContractCallNestedCallsTest extends AbstractContractCallServiceOpcodeTrace
    void updateTokenKeysAndGetUpdatedTokenKeyForFungibleToken(final KeyValueType keyValueType, final KeyType keyType)
            throws Exception {
        // Given
-        final var tokenEntityId = fungibleTokenPersist();
+        final var tokenEntityId = fungibleTokenPersistWithTreasuryAccount(domainBuilder.entity().persist());


If the treasury account is generated, can't we use the base method where we don't pass any treasury account?

Yes, you are correct. Fixing it

codecov · 2025-02-04T14:43:52Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.17%. Comparing base (756b4cf) to head (28816fc).

Additional details and impacted files

@@            Coverage Diff            @@
##               main   #10293   +/-   ##
=========================================
  Coverage     92.17%   92.17%           
  Complexity     7966     7966           
=========================================
  Files           975      975           
  Lines         33278    33278           
  Branches       4201     4201           
=========================================
+ Hits          30673    30674    +1     
  Misses         1606     1606           
+ Partials        999      998    -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: filev94 <todor.filev@limechain.tech>

bilyana-gospodinova · 2025-02-05T09:10:43Z

...irror-web3/src/test/java/com/hedera/mirror/web3/service/AbstractContractCallServiceTest.java

+     * @param treasuryEntity - the treasuryEntity that has to be set in the token
+     * @return Token object that is persisted in db
+     */
+    protected Token fungibleTokenPersistWithTreasuryAccount(final Entity treasuryEntity) {


Do we really need both fungibleTokenPersistWithTreasuryAccount(final Entity treasuryEntity) and fungibleTokenPersistWithTreasuryAccount(final EntityId treasuryEntityId)? Can't we change the code to use only 1 of them?

Yes, we can change the code. My idea was to use the Entity as a parameter for the method so that you can extend the functionality in the future, if needed. However, there are tests that use the EntityId and to refactor it back to Entity would be a big change, so that's why i left it with both for now. In the end, i can remove the Entity parameter and just leave the EntityId, that way i should only refactor tests in one class (TokenModificationFunctionsTest)

bilyana-gospodinova · 2025-02-05T09:12:06Z

...irror-web3/src/test/java/com/hedera/mirror/web3/service/AbstractContractCallServiceTest.java

-                        e.type(EntityType.ACCOUNT).evmAddress(null).alias(null).balance(100_000_000_000_000_000L))
-                .persist();
+        return accountEntityPersistCustomizable(
+                e -> e.type(EntityType.ACCOUNT).evmAddress(null).alias(null).balance(100_000_000_000_000_000L));


Is there a reason the balance in this method and in the other one (accountEntityWithEvmAddressPersist()) to differ? If not, we can choose one of the values and extract it into a constant.

Good catch. I tried using one balance for both, the smaller value - the tests passed so I will extract it in a constant.

bilyana-gospodinova · 2025-02-05T09:18:50Z

...ava/com/hedera/mirror/web3/service/ContractCallServiceERCTokenModificationFunctionsTest.java

        final var treasury = accountPersistWithAlias(SENDER_ALIAS, SENDER_PUBLIC_KEY);
-        final var token = fungibleTokenPersist(treasury);
+        final var token = fungibleTokenPersistWithTreasuryAccount(treasury);


Do we really need to create and set a specific treasury account everywhere? If we don't use it in the test later, we can just leave it to the generated one.

You are correct. When i removed the treasuryAccount and used the fungibleTokenPersist(), the tests passed. And thus the need for the fungibleTokenPersistWithTreasuryAccount(Entity ...) method was removed. Two birds with one stone i guess.

bilyana-gospodinova · 2025-02-05T09:20:20Z

hedera-mirror-web3/src/test/java/com/hedera/mirror/web3/service/OpcodeServiceTest.java

@@ -887,7 +870,7 @@ private Token nftPersist(final Entity treasuryEntity) {
    }

    private Entity accountPersist() {
-        return domainBuilder.entity().customize(a -> a.evmAddress(null)).persist();
+        return accountEntityPersistCustomizable(e -> e.evmAddress(null));


The same is valid as in #10293 (comment)

filev94 added 5 commits January 30, 2025 16:30

adding methods for fungibleToken + testing change in test

ce885d5

Signed-off-by: filev94 <todor.filev@limechain.tech>

refactoring fungibleTokenPersist() and accountEntityPersist functions

9e38149

Signed-off-by: filev94 <todor.filev@limechain.tech>

Merge branch 'main' into 10255-hip-1056-refactor-web3-test-code

f28d2ca

# Conflicts: # hedera-mirror-web3/src/test/java/com/hedera/mirror/web3/service/ContractCallDynamicCallsTest.java # hedera-mirror-web3/src/test/java/com/hedera/mirror/web3/service/OpcodeServiceTest.java

fixing merge conflicts

0728a0a

Signed-off-by: filev94 <todor.filev@limechain.tech>

removing unused import

2f71871

Signed-off-by: filev94 <todor.filev@limechain.tech>

filev94 requested a review from a team as a code owner February 4, 2025 14:20

filev94 requested review from steven-sheehy and bilyana-gospodinova February 4, 2025 14:20

bilyana-gospodinova reviewed Feb 4, 2025

View reviewed changes

...a-mirror-web3/src/test/java/com/hedera/mirror/web3/service/ContractCallDynamicCallsTest.java Show resolved Hide resolved

bilyana-gospodinova reviewed Feb 4, 2025

View reviewed changes

filev94 added 2 commits February 4, 2025 17:05

spotlessApply + refactoring

7977182

Signed-off-by: filev94 <todor.filev@limechain.tech>

accountEntityPersist() change

28816fc

Signed-off-by: filev94 <todor.filev@limechain.tech>

filev94 mentioned this pull request Feb 4, 2025

Refactor code in web3 tests #10255

Open

steven-sheehy assigned filev94 Feb 4, 2025

steven-sheehy added enhancement Type: New feature test Test infrastructure, automated tests required, etc web3 Area: Web3 API labels Feb 4, 2025

steven-sheehy added this to the 0.124.0 milestone Feb 4, 2025

bilyana-gospodinova reviewed Feb 5, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor web3 tests code #10293

Refactor web3 tests code #10293

filev94 commented Feb 4, 2025 •

edited by steven-sheehy

Loading

bilyana-gospodinova Feb 4, 2025 •

edited

Loading

filev94 Feb 4, 2025

bilyana-gospodinova Feb 4, 2025 •

edited

Loading

filev94 Feb 4, 2025

codecov bot commented Feb 4, 2025 •

edited

Loading

bilyana-gospodinova Feb 5, 2025

filev94 Feb 5, 2025

bilyana-gospodinova Feb 5, 2025

filev94 Feb 5, 2025

bilyana-gospodinova Feb 5, 2025 •

edited

Loading

filev94 Feb 5, 2025 •

edited

Loading

bilyana-gospodinova Feb 5, 2025

Refactor web3 tests code #10293

Are you sure you want to change the base?

Refactor web3 tests code #10293

Conversation

filev94 commented Feb 4, 2025 • edited by steven-sheehy Loading

bilyana-gospodinova Feb 4, 2025 • edited Loading

Choose a reason for hiding this comment

filev94 Feb 4, 2025

Choose a reason for hiding this comment

bilyana-gospodinova Feb 4, 2025 • edited Loading

Choose a reason for hiding this comment

filev94 Feb 4, 2025

Choose a reason for hiding this comment

codecov bot commented Feb 4, 2025 • edited Loading

Codecov Report

bilyana-gospodinova Feb 5, 2025

Choose a reason for hiding this comment

filev94 Feb 5, 2025

Choose a reason for hiding this comment

bilyana-gospodinova Feb 5, 2025

Choose a reason for hiding this comment

filev94 Feb 5, 2025

Choose a reason for hiding this comment

bilyana-gospodinova Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

filev94 Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

bilyana-gospodinova Feb 5, 2025

Choose a reason for hiding this comment

filev94 commented Feb 4, 2025 •

edited by steven-sheehy

Loading

bilyana-gospodinova Feb 4, 2025 •

edited

Loading

bilyana-gospodinova Feb 4, 2025 •

edited

Loading

codecov bot commented Feb 4, 2025 •

edited

Loading

bilyana-gospodinova Feb 5, 2025 •

edited

Loading

filev94 Feb 5, 2025 •

edited

Loading