Re: svn commit: r1554064 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/test/FinAccountTests.java

[Jacopo Cappellato-4]

Adrian,

thanks for fixing this. Before we go on and we fix similarly the other unit tests, I would like to propose an alternative approach (with pros and cons); see below. What do you think?

Jacopo


Index: applications/accounting/src/org/ofbiz/accounting/test/FinAccountTests.java
===================================================================
--- applications/accounting/src/org/ofbiz/accounting/test/FinAccountTests.java (revision 1579194)
+++ applications/accounting/src/org/ofbiz/accounting/test/FinAccountTests.java (working copy)
@@ -51,7 +51,7 @@
 
     public void testCreateFinAccount() throws Exception {
         Map<String, Object> ctx = FastMap.newInstance();
-        ctx.put("finAccountId", "TESTACCOUNT1");
+        ctx.put("finAccountId", "TESTACCOUNT2");
         ctx.put("finAccountName", "Test Financial Account");
         ctx.put("finAccountTypeId", "BANK_ACCOUNT");
         ctx.put("userLogin", userLogin);
@@ -60,6 +60,12 @@
     }
 
     public void testDeposit() throws Exception {
+        // ============ Setup =============
+        try {
+            delegator.create("FinAccount", "finAccountId", "TESTACCOUNT1", "finAccountName", "Test Financial Account", "finAccountTypeId", "BANK_ACCOUNT", "statusId", "FNACT_ACTIVE", "currencyUomId", "USD", "isRefundable", "Y");
+        } catch (Exception e) {
+        }
+        // ================================
         Map<String, Object> ctx = FastMap.newInstance();
         ctx.put("finAccountId", "TESTACCOUNT1");
         ctx.put("amount", new BigDecimal("100.00"));
@@ -70,6 +76,12 @@
     }
 
     public void testWithdraw() throws Exception {
+        // ============ Setup =============
+        try {
+            delegator.create("FinAccount", "finAccountId", "TESTACCOUNT1", "finAccountName", "Test Financial Account", "finAccountTypeId", "BANK_ACCOUNT", "statusId", "FNACT_ACTIVE", "currencyUomId", "USD", "isRefundable", "Y");
+        } catch (Exception e) {
+        }
+        // ================================
         Map<String, Object> ctx = FastMap.newInstance();
         ctx.put("finAccountId", "TESTACCOUNT1");
         ctx.put("amount", new BigDecimal("50.00"));


On Dec 29, 2013, at 9:42 PM, [hidden email] wrote:

> Author: adrianc
> Date: Sun Dec 29 20:42:16 2013
> New Revision: 1554064
>
> URL: http://svn.apache.org/r1554064
> Log:
> Fixed the FinAccountTests - which assumed the tests would be run in the order they appear in source code, which JUnit does not guarantee:
>
> http://stackoverflow.com/questions/3693626/how-to-run-test-methods-in-specific-order-in-junit4
>
> Modified:
>    ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/test/FinAccountTests.java
>
> Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/test/FinAccountTests.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/test/FinAccountTests.java?rev=1554064&r1=1554063&r2=1554064&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/test/FinAccountTests.java (original)
> +++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/test/FinAccountTests.java Sun Dec 29 20:42:16 2013
> @@ -20,13 +20,12 @@
> package org.ofbiz.accounting.test;
>
> import java.math.BigDecimal;
> +import java.util.HashMap;
> import java.util.Map;
>
> -import javolution.util.FastMap;
> -
> import org.ofbiz.base.util.UtilMisc;
> import org.ofbiz.entity.GenericValue;
> -import org.ofbiz.service.ModelService;
> +import org.ofbiz.service.ServiceUtil;
> import org.ofbiz.service.testtools.OFBizTestCase;
>
> /**
> @@ -34,49 +33,35 @@ import org.ofbiz.service.testtools.OFBiz
>  */
> public class FinAccountTests extends OFBizTestCase {
>
> -    protected GenericValue userLogin = null;
> -
>     public FinAccountTests(String name) {
>         super(name);
>     }
>
> -    @Override
> -    protected void setUp() throws Exception {
> -        userLogin = delegator.findOne("UserLogin", UtilMisc.toMap("userLoginId", "system"), false);
> -    }
> -
> -    @Override
> -    protected void tearDown() throws Exception {
> -    }
> -
> -    public void testCreateFinAccount() throws Exception {
> -        Map<String, Object> ctx = FastMap.newInstance();
> +    public void testFinAccountOperations() throws Exception {
> +        GenericValue userLogin = delegator.findOne("UserLogin", UtilMisc.toMap("userLoginId", "system"), false);
> +        Map<String, Object> ctx = new HashMap<String, Object>();
>         ctx.put("finAccountId", "TESTACCOUNT1");
>         ctx.put("finAccountName", "Test Financial Account");
>         ctx.put("finAccountTypeId", "BANK_ACCOUNT");
>         ctx.put("userLogin", userLogin);
>         Map<String, Object> resp = dispatcher.runSync("createFinAccount", ctx);
> -        assertEquals("Service result success", ModelService.RESPOND_SUCCESS, resp.get(ModelService.RESPONSE_MESSAGE));
> -    }
> -
> -    public void testDeposit() throws Exception {
> -        Map<String, Object> ctx = FastMap.newInstance();
> +        assertTrue("Service 'createFinAccount' result success", ServiceUtil.isSuccess(resp));
> +        ctx.clear();
>         ctx.put("finAccountId", "TESTACCOUNT1");
>         ctx.put("amount", new BigDecimal("100.00"));
>         ctx.put("userLogin", userLogin);
> -        Map<String, Object> resp = dispatcher.runSync("finAccountDeposit", ctx);
> +        resp = dispatcher.runSync("finAccountDeposit", ctx);
> +        assertTrue("Service 'finAccountDeposit' result success", ServiceUtil.isSuccess(resp));
>         BigDecimal balance = (BigDecimal) resp.get("balance");
>         assertEquals(balance.toPlainString(), "100.00");
> -    }
> -
> -    public void testWithdraw() throws Exception {
> -        Map<String, Object> ctx = FastMap.newInstance();
> +        ctx.clear();
>         ctx.put("finAccountId", "TESTACCOUNT1");
>         ctx.put("amount", new BigDecimal("50.00"));
>         ctx.put("userLogin", userLogin);
> -        Map<String, Object> resp = dispatcher.runSync("finAccountWithdraw", ctx);
> +        resp = dispatcher.runSync("finAccountWithdraw", ctx);
> +        assertTrue("Service 'finAccountWithdraw' result success", ServiceUtil.isSuccess(resp));
>         BigDecimal previousBalance = (BigDecimal) resp.get("previousBalance");
> -        BigDecimal balance = ((BigDecimal) resp.get("balance"));
> +        balance = ((BigDecimal) resp.get("balance"));
>         assertEquals(balance.add(new BigDecimal("50.00")).toPlainString(), previousBalance.toPlainString());
>     }
> }
>
>

[Jacopo Cappellato-4]

please ignore, I will send a better one.

Jacopo

On Mar 19, 2014, at 3:23 PM, Jacopo Cappellato <[hidden email]> wrote:

> Adrian,
>
> thanks for fixing this. Before we go on and we fix similarly the other unit tests, I would like to propose an alternative approach (with pros and cons); see below. What do you think?
>
> Jacopo
>
>
> Index: applications/accounting/src/org/ofbiz/accounting/test/FinAccountTests.java
> ===================================================================
> --- applications/accounting/src/org/ofbiz/accounting/test/FinAccountTests.java (revision 1579194)
> +++ applications/accounting/src/org/ofbiz/accounting/test/FinAccountTests.java (working copy)
> @@ -51,7 +51,7 @@
>
>     public void testCreateFinAccount() throws Exception {
>         Map<String, Object> ctx = FastMap.newInstance();
> -        ctx.put("finAccountId", "TESTACCOUNT1");
> +        ctx.put("finAccountId", "TESTACCOUNT2");
>         ctx.put("finAccountName", "Test Financial Account");
>         ctx.put("finAccountTypeId", "BANK_ACCOUNT");
>         ctx.put("userLogin", userLogin);
> @@ -60,6 +60,12 @@
>     }
>
>     public void testDeposit() throws Exception {
> +        // ============ Setup =============
> +        try {
> +            delegator.create("FinAccount", "finAccountId", "TESTACCOUNT1", "finAccountName", "Test Financial Account", "finAccountTypeId", "BANK_ACCOUNT", "statusId", "FNACT_ACTIVE", "currencyUomId", "USD", "isRefundable", "Y");
> +        } catch (Exception e) {
> +        }
> +        // ================================
>         Map<String, Object> ctx = FastMap.newInstance();
>         ctx.put("finAccountId", "TESTACCOUNT1");
>         ctx.put("amount", new BigDecimal("100.00"));
> @@ -70,6 +76,12 @@
>     }
>
>     public void testWithdraw() throws Exception {
> +        // ============ Setup =============
> +        try {
> +            delegator.create("FinAccount", "finAccountId", "TESTACCOUNT1", "finAccountName", "Test Financial Account", "finAccountTypeId", "BANK_ACCOUNT", "statusId", "FNACT_ACTIVE", "currencyUomId", "USD", "isRefundable", "Y");
> +        } catch (Exception e) {
> +        }
> +        // ================================
>         Map<String, Object> ctx = FastMap.newInstance();
>         ctx.put("finAccountId", "TESTACCOUNT1");
>         ctx.put("amount", new BigDecimal("50.00"));
>
>
> On Dec 29, 2013, at 9:42 PM, [hidden email] wrote:
>
>> Author: adrianc
>> Date: Sun Dec 29 20:42:16 2013
>> New Revision: 1554064
>>
>> URL: http://svn.apache.org/r1554064
>> Log:
>> Fixed the FinAccountTests - which assumed the tests would be run in the order they appear in source code, which JUnit does not guarantee:
>>
>> http://stackoverflow.com/questions/3693626/how-to-run-test-methods-in-specific-order-in-junit4
>>
>> Modified:
>>   ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/test/FinAccountTests.java
>>
>> Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/test/FinAccountTests.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/test/FinAccountTests.java?rev=1554064&r1=1554063&r2=1554064&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/test/FinAccountTests.java (original)
>> +++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/test/FinAccountTests.java Sun Dec 29 20:42:16 2013
>> @@ -20,13 +20,12 @@
>> package org.ofbiz.accounting.test;
>>
>> import java.math.BigDecimal;
>> +import java.util.HashMap;
>> import java.util.Map;
>>
>> -import javolution.util.FastMap;
>> -
>> import org.ofbiz.base.util.UtilMisc;
>> import org.ofbiz.entity.GenericValue;
>> -import org.ofbiz.service.ModelService;
>> +import org.ofbiz.service.ServiceUtil;
>> import org.ofbiz.service.testtools.OFBizTestCase;
>>
>> /**
>> @@ -34,49 +33,35 @@ import org.ofbiz.service.testtools.OFBiz
>> */
>> public class FinAccountTests extends OFBizTestCase {
>>
>> -    protected GenericValue userLogin = null;
>> -
>>    public FinAccountTests(String name) {
>>        super(name);
>>    }
>>
>> -    @Override
>> -    protected void setUp() throws Exception {
>> -        userLogin = delegator.findOne("UserLogin", UtilMisc.toMap("userLoginId", "system"), false);
>> -    }
>> -
>> -    @Override
>> -    protected void tearDown() throws Exception {
>> -    }
>> -
>> -    public void testCreateFinAccount() throws Exception {
>> -        Map<String, Object> ctx = FastMap.newInstance();
>> +    public void testFinAccountOperations() throws Exception {
>> +        GenericValue userLogin = delegator.findOne("UserLogin", UtilMisc.toMap("userLoginId", "system"), false);
>> +        Map<String, Object> ctx = new HashMap<String, Object>();
>>        ctx.put("finAccountId", "TESTACCOUNT1");
>>        ctx.put("finAccountName", "Test Financial Account");
>>        ctx.put("finAccountTypeId", "BANK_ACCOUNT");
>>        ctx.put("userLogin", userLogin);
>>        Map<String, Object> resp = dispatcher.runSync("createFinAccount", ctx);
>> -        assertEquals("Service result success", ModelService.RESPOND_SUCCESS, resp.get(ModelService.RESPONSE_MESSAGE));
>> -    }
>> -
>> -    public void testDeposit() throws Exception {
>> -        Map<String, Object> ctx = FastMap.newInstance();
>> +        assertTrue("Service 'createFinAccount' result success", ServiceUtil.isSuccess(resp));
>> +        ctx.clear();
>>        ctx.put("finAccountId", "TESTACCOUNT1");
>>        ctx.put("amount", new BigDecimal("100.00"));
>>        ctx.put("userLogin", userLogin);
>> -        Map<String, Object> resp = dispatcher.runSync("finAccountDeposit", ctx);
>> +        resp = dispatcher.runSync("finAccountDeposit", ctx);
>> +        assertTrue("Service 'finAccountDeposit' result success", ServiceUtil.isSuccess(resp));
>>        BigDecimal balance = (BigDecimal) resp.get("balance");
>>        assertEquals(balance.toPlainString(), "100.00");
>> -    }
>> -
>> -    public void testWithdraw() throws Exception {
>> -        Map<String, Object> ctx = FastMap.newInstance();
>> +        ctx.clear();
>>        ctx.put("finAccountId", "TESTACCOUNT1");
>>        ctx.put("amount", new BigDecimal("50.00"));
>>        ctx.put("userLogin", userLogin);
>> -        Map<String, Object> resp = dispatcher.runSync("finAccountWithdraw", ctx);
>> +        resp = dispatcher.runSync("finAccountWithdraw", ctx);
>> +        assertTrue("Service 'finAccountWithdraw' result success", ServiceUtil.isSuccess(resp));
>>        BigDecimal previousBalance = (BigDecimal) resp.get("previousBalance");
>> -        BigDecimal balance = ((BigDecimal) resp.get("balance"));
>> +        balance = ((BigDecimal) resp.get("balance"));
>>        assertEquals(balance.add(new BigDecimal("50.00")).toPlainString(), previousBalance.toPlainString());
>>    }
>> }
>>
>>
>