Added tests for AccountClientImpl.getAccountBalance functionality #123

Lemeri123 · 2024-12-10T18:32:22Z

I created a new test file and added the necessary tests needed. #109

Ndacyayisenga-droid · 2024-12-10T18:40:22Z

@Lemeri123 you didn't sign off your commit.

That you now pushed a commit already, you can run these commands to get them signed off

In your local branch, run: git rebase HEAD~1 --signoff
Force push your changes to overwrite the branch: git push --force-with-lease origin main

Signed-off-by: Lemeri123 <132246079+Lemeri123@users.noreply.github.com>

Lemeri123 · 2024-12-10T18:44:21Z

Okay thank you @Ndacyayisenga-droid

Ndacyayisenga-droid · 2024-12-10T20:27:47Z

Okay thank you @Ndacyayisenga-droid

Thanks for your contribution @Lemeri123 . I added a few comments

hendrikebbers

Some changes are needed

hendrikebbers · 2024-12-12T10:22:36Z

...erprise-base/src/main/java/com/openelements/hiero/base/implementation/AccountClientImpl.java


    @NonNull
    @Override
-    public Hbar getAccountBalance(@NonNull AccountId account) throws HieroException {


This change do not make sense to me. Can you please revert it.

hendrikebbers · 2024-12-12T10:23:53Z

hiero-enterprise-base/pom.xml

+    <dependency>
+  <groupId>org.mockito</groupId>
+  <artifactId>mockito-core</artifactId>
+  <version> 5.11.0</version>


We define the versions for all dependencies in the root pom. Please remove the version at this place and add it to the root pom.xml

pom.xml

hendrikebbers · 2024-12-12T10:25:55Z

hiero-enterprise-base/src/test/java/com/openelements/hiero/base/test/AccountClientImplTest.java

+    @Test
+    public void testGetAccountBalance_ValidPositiveBalance() throws HieroException {
+        // Fully qualify AccountId to ensure no ambiguity
+        com.hedera.hashgraph.sdk.AccountId accountId = com.hedera.hashgraph.sdk.AccountId.fromString("0.0.12345");


There is only one AccountId on the classpath. Based on that I would favor to do a regular import instead of a full qualified class in code.

hendrikebbers · 2024-12-12T10:28:30Z

@Lemeri123 please have a look at my review

Signed-off-by: Lemeri123 <132246079+Lemeri123@users.noreply.github.com>

hendrikebbers · 2024-12-14T19:58:43Z

@Lemeri123 thank you for your contribution!

Ndacyayisenga-droid · 2024-12-10T19:07:07Z

hiero-enterprise-base/src/test/java/com/openelements/hiero/base/test/AccountClientImplTest.java

+import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentMatchers;
+
+import static org.junit.jupiter.api.Assertions.*;


Remove the asterisk wildcard

But if I remove it then I get errors since they set up the testing framework and mocking utilities for the test method

Ndacyayisenga-droid · 2024-12-10T19:07:21Z

hiero-enterprise-base/src/test/java/com/openelements/hiero/base/test/AccountClientImplTest.java

+
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.Mockito.*;
+


Remove the asterisk wildcard

Ndacyayisenga-droid · 2024-12-10T19:13:45Z

hiero-enterprise-base/src/test/java/com/openelements/hiero/base/test/AccountClientImplTest.java

+    @Test
+    public void testGetAccountBalance_ValidPositiveBalance() throws HieroException {
+        // Fully qualify AccountId to ensure no ambiguity
+        com.hedera.hashgraph.sdk.AccountId accountId = com.hedera.hashgraph.sdk.AccountId.fromString("0.0.12345");


@Lemeri123 I recommend importing AccountId directly to simplify the code and enhance readability by eliminating the need for full qualifications. You can change the above line to something like
AccountId accountId = AccountId.fromString("0.0.12345");

Ndacyayisenga-droid · 2024-12-10T19:15:35Z

hiero-enterprise-base/src/test/java/com/openelements/hiero/base/test/AccountClientImplTest.java

+
+    @Test
+    public void testGetAccountBalance_ZeroBalance() throws HieroException {
+        com.hedera.hashgraph.sdk.AccountId accountId = com.hedera.hashgraph.sdk.AccountId.fromString("0.0.67890");


I recommend importing AccountId directly to simplify the code and enhance readability by eliminating the need for full qualifications. You can change the above line to something like
AccountId accountId = AccountId.fromString("0.0.67890");

Ndacyayisenga-droid · 2024-12-10T20:17:19Z

hiero-enterprise-base/src/test/java/com/openelements/hiero/base/test/AccountClientImplTest.java

+    @Test
+    public void testGetAccountBalance_InvalidAccount_ThrowsException() throws HieroException {
+        // Use a valid but non-existing account ID to simulate invalid behavior
+        com.hedera.hashgraph.sdk.AccountId invalidAccountId = com.hedera.hashgraph.sdk.AccountId.fromString("0.0.9999999"); // Simulate invalid or non-existing account


Same review as the above

Ndacyayisenga-droid · 2024-12-10T20:20:14Z

hiero-enterprise-base/src/test/java/com/openelements/hiero/base/test/AccountClientImplTest.java

+    @Test
+    public void testGetAccountBalance_NullThrowsException() {
+        assertThrows(NullPointerException.class, () -> {
+            accountClientImpl.getAccountBalance((com.hedera.hashgraph.sdk.AccountId) null);


Just pass in the AccountId not com.hedera.hashgraph.sdk.AccountId

Ndacyayisenga-droid · 2024-12-10T20:23:03Z

hiero-enterprise-base/src/test/java/com/openelements/hiero/base/test/AccountClientImplTest.java

+
+    @Test
+    public void testGetAccountBalance_ProtocolLayerClientFails() throws HieroException {
+        com.hedera.hashgraph.sdk.AccountId accountId = com.hedera.hashgraph.sdk.AccountId.fromString("0.0.12345");


Use AccountId accountId = AccountId.fromString("0.0.12345"); not com.hedera.hashgraph.sdk.AccountId accountId = com.hedera.hashgraph.sdk.AccountId.fromString("0.0.12345");

Ndacyayisenga-droid · 2024-12-10T20:24:45Z

...erprise-base/src/main/java/com/openelements/hiero/base/implementation/AccountClientImpl.java

@@ -1,6 +1,4 @@
 package com.openelements.hiero.base.implementation;
-
-import com.hedera.hashgraph.sdk.AccountId;


I don't think we need this change

Ndacyayisenga-droid · 2024-12-10T20:26:06Z

...erprise-base/src/main/java/com/openelements/hiero/base/implementation/AccountClientImpl.java

    @NonNull
    @Override
-    public Hbar getAccountBalance(@NonNull AccountId account) throws HieroException {
+    public Hbar getAccountBalance(com.hedera.hashgraph.sdk.AccountId account) throws HieroException {


I don't think we need this change as well

Added tests for AccountClientImpl.getAccountBalance functionality

df81ee5

Signed-off-by: Lemeri123 <132246079+Lemeri123@users.noreply.github.com>

Ndacyayisenga-droid requested a review from hendrikebbers December 10, 2024 20:27

hendrikebbers reviewed Dec 12, 2024

View reviewed changes

Made changes

c7c2fd1

Signed-off-by: Lemeri123 <132246079+Lemeri123@users.noreply.github.com>

Lemeri123 requested a review from hendrikebbers December 12, 2024 19:19

hendrikebbers merged commit 8ed92d1 into OpenElements:main Dec 14, 2024
3 of 4 checks passed

Ndacyayisenga-droid reviewed Dec 16, 2024

View reviewed changes


		import static org.junit.jupiter.api.Assertions.*;
		import static org.mockito.Mockito.*;

		@@ -1,6 +1,4 @@
		package com.openelements.hiero.base.implementation;

		import com.hedera.hashgraph.sdk.AccountId;

Added tests for AccountClientImpl.getAccountBalance functionality #123

Added tests for AccountClientImpl.getAccountBalance functionality #123

Uh oh!

Conversation

Lemeri123 commented Dec 10, 2024

Uh oh!

Ndacyayisenga-droid commented Dec 10, 2024

Uh oh!

Lemeri123 commented Dec 10, 2024

Uh oh!

Ndacyayisenga-droid commented Dec 10, 2024

Uh oh!

hendrikebbers left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hendrikebbers commented Dec 12, 2024

Uh oh!

Uh oh!

hendrikebbers commented Dec 14, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants