You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2019/10/02 14:46:01 UTC

svn commit: r1867889 - in /ofbiz/ofbiz-framework/trunk/applications: datamodel/data/demo/OrderDemoData.xml order/minilang/test/OrderTest.xml order/servicedef/secas.xml order/testdef/OrderTest.xml order/testdef/data/OrderTestData.xml

Author: jleroux
Date: Wed Oct  2 14:46:00 2019
New Revision: 1867889

URL: http://svn.apache.org/viewvc?rev=1867889&view=rev
Log:
Improved: Unit test case for service - SendOrderBackorderNotification
(OFBIZ-8810)(OFBIZ-9647)(OFBIZ-9671)

While working on this I stumbled upon an issue related with 
webSiteId="OrderEntry" well related by Ratnesh Upadhyay in OFBIZ-9647.

Unlike him I decided not to remove the webSiteId="OrderEntry" entries but to
replace them by webSiteId="WebStore"

Tests pass!

Added:
    ofbiz/ofbiz-framework/trunk/applications/order/minilang/test/OrderTest.xml   (with props)
Modified:
    ofbiz/ofbiz-framework/trunk/applications/datamodel/data/demo/OrderDemoData.xml
    ofbiz/ofbiz-framework/trunk/applications/order/servicedef/secas.xml
    ofbiz/ofbiz-framework/trunk/applications/order/testdef/OrderTest.xml
    ofbiz/ofbiz-framework/trunk/applications/order/testdef/data/OrderTestData.xml

Modified: ofbiz/ofbiz-framework/trunk/applications/datamodel/data/demo/OrderDemoData.xml
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/datamodel/data/demo/OrderDemoData.xml?rev=1867889&r1=1867888&r2=1867889&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/datamodel/data/demo/OrderDemoData.xml (original)
+++ ofbiz/ofbiz-framework/trunk/applications/datamodel/data/demo/OrderDemoData.xml Wed Oct  2 14:46:00 2019
@@ -2361,7 +2361,7 @@ under the License.
     <!--PartyTaxAuthInfo partyId="Company" taxAuthGeoId="FRA" taxAuthPartyId="FRA_TA" fromDate="2014-01-01 00:00:00.000" partyTaxId="FR-12345678901" isExempt="N"/-->
 
     <!--Demo data for a sales order -->
-    <OrderHeader orderId="DEMO10090" orderTypeId="SALES_ORDER" salesChannelEnumId="WEB_SALES_CHANNEL" orderDate="2008-04-23 16:49:27.392" entryDate="2008-04-23 16:49:27.392" priority="2" visitId="10002" statusId="ORDER_APPROVED" createdBy="admin" currencyUom="USD" productStoreId="9000" remainingSubTotal="38.40" grandTotal="50.85"  invoicePerShipment="Y"/>
+    <OrderHeader orderId="DEMO10090" orderTypeId="SALES_ORDER" salesChannelEnumId="WEB_SALES_CHANNEL" orderDate="2008-04-23 16:49:27.392" entryDate="2008-04-23 16:49:27.392" priority="2" visitId="10002" statusId="ORDER_APPROVED" createdBy="admin" currencyUom="USD" productStoreId="9000" remainingSubTotal="38.40" grandTotal="50.85"  invoicePerShipment="Y" webSiteId="WebStore"/>
     <OrderItem orderId="DEMO10090" orderItemSeqId="00001" orderItemTypeId="PRODUCT_ORDER_ITEM" productId="GZ-2644" prodCatalogId="DemoCatalog" isPromo="N" quantity="2.0" selectedAmount="0.0" unitPrice="38.4" unitListPrice="48.0" isModifiedPrice="N" itemDescription="Round Gizmo" statusId="ITEM_APPROVED"/>
     <OrderItemPriceInfo orderItemPriceInfoId="9000" orderId="DEMO10090" orderItemSeqId="00001" productPriceRuleId="9000" productPriceActionSeqId="01" modifyAmount="-9.600" description="[PRODUCT_CATEGORY_IDIsPROMOTIONS] [list:48.0;avgCost:48.0;margin:0.0] [type:PRICE_POL]"/>
 
@@ -2400,7 +2400,7 @@ under the License.
 
     <!-- Purchase order test data -->
     <!--for jira issue - 1782-->
-    <OrderHeader orderId="DEMO10091" orderTypeId="PURCHASE_ORDER" orderName="New Purchase Order" salesChannelEnumId="UNKNWN_SALES_CHANNEL" orderDate="2008-06-10 13:27:07.024" entryDate="2008-06-10 13:27:07.024" visitId="10000" statusId="ORDER_CREATED" createdBy="admin" currencyUom="USD" productStoreId="9000" remainingSubTotal="108.0" grandTotal="108.0"/>
+    <OrderHeader orderId="DEMO10091" orderTypeId="PURCHASE_ORDER" orderName="New Purchase Order" salesChannelEnumId="UNKNWN_SALES_CHANNEL" orderDate="2008-06-10 13:27:07.024" entryDate="2008-06-10 13:27:07.024" visitId="10000" statusId="ORDER_CREATED" createdBy="admin" currencyUom="USD" productStoreId="9000" remainingSubTotal="108.0" grandTotal="108.0" webSiteId="WebStore"/>
     <OrderItem orderId="DEMO10091" orderItemSeqId="00001" orderItemTypeId="PRODUCT_ORDER_ITEM" productId="GZ-2644" prodCatalogId="DemoCatalog" isPromo="N" quantity="5.0" selectedAmount="0.0" unitPrice="21.6" unitListPrice="0.0" isModifiedPrice="N" itemDescription="GZ-2644-5 Round Gizmo" statusId="ITEM_CREATED"/>
     <OrderItemPriceInfo orderItemPriceInfoId="10001" orderId="DEMO10091" orderItemSeqId="00001" description="SupplierProduct [minimumOrderQuantity:5.0, lastPrice: 21.6]"/>
     <OrderItemPriceInfo orderItemPriceInfoId="10002" orderId="DEMO10091" orderItemSeqId="00001" description="SupplierProduct [minimumOrderQuantity:0.0, lastPrice: 24.0]"/>
@@ -2423,7 +2423,7 @@ under the License.
     <ItemIssuance itemIssuanceId="9003" orderId="DEMO10091" orderItemSeqId="00001" shipGroupSeqId="00001" shipmentId="9999" shipmentItemSeqId="00001" issuedDateTime="2009-08-13 17:46:29.603" quantity="5"/>
 
     <!--Demo Purchase Order data for Accounting-->
-    <OrderHeader orderId="Demo1001" orderTypeId="PURCHASE_ORDER" orderName="Demo Purchase Order " salesChannelEnumId="UNKNWN_SALES_CHANNEL" orderDate="2009-08-13 17:45:50.419" priority="2" entryDate="2009-08-13 17:45:50.419" statusId="ORDER_COMPLETED" currencyUom="USD" remainingSubTotal="48.00" grandTotal="48.00"/>
+    <OrderHeader orderId="Demo1001" orderTypeId="PURCHASE_ORDER" orderName="Demo Purchase Order " salesChannelEnumId="UNKNWN_SALES_CHANNEL" orderDate="2009-08-13 17:45:50.419" priority="2" entryDate="2009-08-13 17:45:50.419" statusId="ORDER_COMPLETED" currencyUom="USD" remainingSubTotal="48.00" grandTotal="48.00" webSiteId="WebStore" />
     <OrderItem orderId="Demo1001" orderItemSeqId="00001" orderItemTypeId="PRODUCT_ORDER_ITEM" productId="GZ-2644" prodCatalogId="DemoCatalog" isPromo="N" quantity="2.000000" selectedAmount="0.000000" unitPrice="24.00" unitListPrice="0.00" isModifiedPrice="N" itemDescription="GZ-2644-0 Round Gizmo" statusId="ITEM_COMPLETED" estimatedDeliveryDate="2009-08-13 17:43:53.0"/>
     <OrderItemPriceInfo orderItemPriceInfoId="9001" orderId="Demo1001" orderItemSeqId="00001" description="SupplierProduct [minimumOrderQuantity:0.000000, lastPrice: 24.000]"/>
     <OrderRole orderId="Demo1001" partyId="Company" roleTypeId="BILL_TO_CUSTOMER" />
@@ -2467,7 +2467,7 @@ under the License.
     <AcctgTransEntry acctgTransId="9001" acctgTransEntrySeqId="00002" acctgTransEntryTypeId="_NA_" partyId="DemoSupplier" roleTypeId="BILL_FROM_VENDOR" glAccountTypeId="ACCOUNTS_PAYABLE" glAccountId="210000" organizationPartyId="Company" amount="48.00" currencyUomId="USD" origAmount="48.00" origCurrencyUomId="USD" debitCreditFlag="C" reconcileStatusId="AES_NOT_RECONCILED"/>
 
     <!--Demo Sales Order for Accounting-->
-    <OrderHeader orderId="Demo1002" orderTypeId="SALES_ORDER" orderName="Demo Sales Order" salesChannelEnumId="WEB_SALES_CHANNEL" orderDate="2009-08-17 14:23:49.475" priority="2" entryDate="2009-08-17 14:23:49.475" visitId="10000" statusId="ORDER_COMPLETED" createdBy="admin" currencyUom="USD" productStoreId="9000" remainingSubTotal="107.98" grandTotal="127.09"/>
+    <OrderHeader orderId="Demo1002" orderTypeId="SALES_ORDER" orderName="Demo Sales Order" salesChannelEnumId="WEB_SALES_CHANNEL" orderDate="2009-08-17 14:23:49.475" priority="2" entryDate="2009-08-17 14:23:49.475" visitId="10000" statusId="ORDER_COMPLETED" createdBy="admin" currencyUom="USD" productStoreId="9000" remainingSubTotal="107.98" grandTotal="127.09" webSiteId="WebStore"/>
     <OrderItem orderId="Demo1002" orderItemSeqId="00001" orderItemTypeId="PRODUCT_ORDER_ITEM" productId="WG-1111" prodCatalogId="DemoCatalog" isPromo="N" quantity="2.000000" selectedAmount="0.000000" unitPrice="59.99" unitListPrice="60.00" isModifiedPrice="N" itemDescription="Micro Chrome Widget" statusId="ITEM_COMPLETED"/>
     <OrderItem orderId="Demo1002" orderItemSeqId="00004" orderItemTypeId="PRODUCT_ORDER_ITEM" productId="WG-1111" isPromo="Y" quantity="1.000000" selectedAmount="0.000000" unitPrice="59.99" unitListPrice="60.00" isModifiedPrice="N" itemDescription="Micro Chrome Widget" statusId="ITEM_COMPLETED"/>
     <PartyRole partyId="DemoCustomer" roleTypeId="END_USER_CUSTOMER"/>

Added: ofbiz/ofbiz-framework/trunk/applications/order/minilang/test/OrderTest.xml
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/order/minilang/test/OrderTest.xml?rev=1867889&view=auto
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/order/minilang/test/OrderTest.xml (added)
+++ ofbiz/ofbiz-framework/trunk/applications/order/minilang/test/OrderTest.xml Wed Oct  2 14:46:00 2019
@@ -0,0 +1,42 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+<simple-methods xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                xmlns="http://ofbiz.apache.org/Simple-Method" xsi:schemaLocation="http://ofbiz.apache.org/Simple-Method http://ofbiz.apache.org/dtds/simple-methods.xsd">
+
+    <simple-method method-name="testSendOrderChangeNotification" short-description="Sends order change confirmation mail" login-required="false">
+        <entity-one entity-name="UserLogin" value-field="userLogin">
+            <field-map field-name="userLoginId" value="system"/>
+        </entity-one>
+        <set-current-user-login value-field="userLogin"/>
+        <set field="serviceCtx.orderId" value="TEST_DEMO10090"/>
+        <set field="serviceCtx.note" value="Test Note"/>
+        <set field="serviceCtx.comments" value="Test comments"/>
+        <set field="serviceCtx.sendTo" value="newtest_email@example.com"/>
+        <call-service service-name="sendOrderChangeNotification" in-map-name="serviceCtx">
+            <results-to-map map-name="serviceResult"/>
+        </call-service>
+        <assert>
+            <not><if-empty field="serviceResult"/></not>
+            <if-compare operator="equals" field="serviceResult.emailType" value="PRDS_ODR_CHANGE" />
+        </assert>
+        <check-errors/>
+    </simple-method>
+</simple-methods>
\ No newline at end of file

Propchange: ofbiz/ofbiz-framework/trunk/applications/order/minilang/test/OrderTest.xml
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: ofbiz/ofbiz-framework/trunk/applications/order/minilang/test/OrderTest.xml
------------------------------------------------------------------------------
    svn:keywords = Date Rev Author URL Id

Propchange: ofbiz/ofbiz-framework/trunk/applications/order/minilang/test/OrderTest.xml
------------------------------------------------------------------------------
    svn:mime-type = text/xml

Modified: ofbiz/ofbiz-framework/trunk/applications/order/servicedef/secas.xml
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/order/servicedef/secas.xml?rev=1867889&r1=1867888&r2=1867889&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/order/servicedef/secas.xml (original)
+++ ofbiz/ofbiz-framework/trunk/applications/order/servicedef/secas.xml Wed Oct  2 14:46:00 2019
@@ -403,6 +403,7 @@ under the License.
         <condition field-name="oldStatusId" operator="not-equals" value="CRQ_ACCEPTED"/>
         <condition field-name="oldStatusId" operator="not-equals" value="CRQ_PENDING"/>
         <condition field-name="statusId" operator="equals" value="CRQ_ACCEPTED"/>
+        <condition field-name="webSiteId" operator="equals" value="WebStore"/>
         <set field-name="bodyParameters.custRequestId" env-name="custRequestId"/>
         <set field-name="bodyParameters.custRequestName" env-name="custRequestName"/>
         <set field-name="partyIdTo" env-name="fromPartyId"/>
@@ -413,6 +414,7 @@ under the License.
     <eca service="setCustRequestStatus" event="commit">
         <condition field-name="oldStatusId" operator="not-equals" value="CRQ_COMPLETED"/>
         <condition field-name="statusId" operator="equals" value="CRQ_COMPLETED"/>
+        <condition field-name="webSiteId" operator="equals" value="WebStore"/>
         <set field-name="bodyParameters.custRequestId" env-name="custRequestId"/>
         <set field-name="partyIdTo" env-name="fromPartyId"/>
         <set field-name="emailTemplateSettingId" value="CUST_REQ_COMPLETED"/>

Modified: ofbiz/ofbiz-framework/trunk/applications/order/testdef/OrderTest.xml
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/order/testdef/OrderTest.xml?rev=1867889&r1=1867888&r2=1867889&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/order/testdef/OrderTest.xml (original)
+++ ofbiz/ofbiz-framework/trunk/applications/order/testdef/OrderTest.xml Wed Oct  2 14:46:00 2019
@@ -24,6 +24,9 @@ under the License.
     <test-case case-name="order-tests-data-load">
         <entity-xml action="load" entity-xml-url="component://order/testdef/data/OrderTestData.xml"/>
     </test-case>
+    <test-case case-name="order-tests-data-load">
+        <entity-xml action="load" entity-xml-url="component://order/testdef/data/OrderTestData.xml"/>
+    </test-case>
     <test-case case-name="purchaseOrder-test">
         <junit-test-suite class-name="org.apache.ofbiz.order.test.PurchaseOrderTest"/>
     </test-case>
@@ -31,6 +34,9 @@ under the License.
         <junit-test-suite class-name="org.apache.ofbiz.order.test.SalesOrderTest"/>
     </test-case>
     <test-case case-name="order-test">
+        <simple-method-test location="component://order/minilang/test/OrderTest.xml"/>
+    </test-case>
+    <test-case case-name="order-test">
         <simple-method-test location="component://order/minilang/test/OrderTests.xml"/>
     </test-case>
     <test-case case-name="order-tests">

Modified: ofbiz/ofbiz-framework/trunk/applications/order/testdef/data/OrderTestData.xml
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/order/testdef/data/OrderTestData.xml?rev=1867889&r1=1867888&r2=1867889&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/order/testdef/data/OrderTestData.xml (original)
+++ ofbiz/ofbiz-framework/trunk/applications/order/testdef/data/OrderTestData.xml Wed Oct  2 14:46:00 2019
@@ -30,7 +30,7 @@ under the License.
     <ProductStoreEmailSetting productStoreId="9000" emailType="PRDS_ODR_CONFIRM" bodyScreenLocation="component://ecommerce/widget/EmailOrderScreens.xml#OrderConfirmNotice" xslfoAttachScreenLocation="component://ecommerce/widget/EmailOrderScreens.xml#OrderConfirmNoticePdf" subject="OFBiz Demo - Order Confirmation #${orderId}" bccAddress="ofbiztest@example.com" fromAddress="ofbiztest@example.com"/>
 
     <!--Demo data for a sales order -->
-    <OrderHeader orderId="TEST_DEMO10090" orderTypeId="SALES_ORDER" salesChannelEnumId="WEB_SALES_CHANNEL" orderDate="2008-04-23 16:49:27.392" entryDate="2008-04-23 16:49:27.392" priority="2" visitId="10002" statusId="ORDER_APPROVED" currencyUom="USD" productStoreId="9000" remainingSubTotal="38.40" grandTotal="50.85"  invoicePerShipment="Y"/>
+    <OrderHeader orderId="TEST_DEMO10090" orderTypeId="SALES_ORDER" salesChannelEnumId="WEB_SALES_CHANNEL" orderDate="2008-04-23 16:49:27.392" entryDate="2008-04-23 16:49:27.392" priority="2" visitId="10002" statusId="ORDER_APPROVED" currencyUom="USD" productStoreId="9000" remainingSubTotal="38.40" grandTotal="50.85"  invoicePerShipment="Y" webSiteId="WebStore"/>
     <OrderItem orderId="TEST_DEMO10090" orderItemSeqId="00001" orderItemTypeId="PRODUCT_ORDER_ITEM" productId="GZ-2644" prodCatalogId="DemoCatalog" isPromo="N" quantity="2.0" selectedAmount="0.0" unitPrice="38.4" unitListPrice="48.0" isModifiedPrice="N" itemDescription="Round Gizmo" statusId="ITEM_APPROVED"/>
 
     <Party partyId="TestDemoCustomer" partyTypeId="PERSON" statusId="PARTY_ENABLED"/>



Re: Providing utilities for integration tests

Posted by Jacques Le Roux <ja...@les7arts.com>.
e 13/10/2019 à 14:08, Mathieu Lirzin a écrit :
> Le 08/10/2019 à 13:12, Jacques Le Roux a écrit :
>>> Hi Mathieu
>>>
>>> I forgot to mention that I put this method in GroovyUtil.java.
>>>
>>> It was hastily done because doing so I rapidly thought that "Anyway it would not add much, just a cover function"
>>>
>>> It works with a complete/correct version:
>>>
>>>      public static Map testGroovy(Delegator delegator, LocalDispatcher dispatcher, Map<String, Object> serviceCtx,
>>>              String serviceName) throws GenericEntityException, GenericServiceException {
>>>          GenericValue userLogin = EntityQuery.use(delegator)
>>>                  .from("UserLogin")
>>>                  .where("userLoginId", "system")
>>>                  .cache()
>>>                  .queryOne();
>>>          serviceCtx.put("userLogin", userLogin);
>>>          return dispatcher.runSync(serviceName, serviceCtx);
>>>      }
>>>
>>> I still wonder if it of much use, ie compare:
>>>
>>>      void testSendOrderChangeNotification() {
>>>          Map serviceCtx = [
>>>              orderId: 'TEST_DEMO10090',
>>>              sendTo: 'test_email@example.com',
>>>          ]
>>>          Map serviceResult = GroovyUtil.testGroovy(delegator, dispatcher, serviceCtx, 'sendOrderChangeNotification');
>>>          assert ServiceUtil.isSuccess(serviceResult)
>>>          assert serviceResult.emailType.equals("PRDS_ODR_CHANGE")
>>>      }
>>>
>>> with
>>>
>>>      void testSendOrderChangeNotification() {
>>>          Map serviceCtx = [
>>>              orderId: 'TEST_DEMO10090',
>>>              sendTo: 'test_email@example.com',
>>>              userLogin: EntityQuery.use(delegator).from('UserLogin').where('userLoginId', 'system').cache().queryOne()
>>>          ]
>>>          Map serviceResult = dispatcher.runSync('sendOrderChangeNotification', serviceCtx)
>>>          assert ServiceUtil.isSuccess(serviceResult)
>>>          assert serviceResult.emailType.equals("PRDS_ODR_CHANGE")
>>>      }
>>>
>>> It's shorter and can be used for most Groovy tests. But you have to
>>> pass the delegator, dispatcher and serviceCtx which are else
>>> transparent. Not sure it's of better use.
>>>
>>> What do you think?
> I agree that that having to pass the delegator, dispatcher and
> serviceCtx is not ideal and tend make the test less clear.
>
> In order to avoid repetitive code a nice first helper method would be
> for example one for retrieving the default userLogin like what is done
> in ‘QuoteTests.groovy’
>
>      // Retrieves a particular login record.
>      GenericValue getUserLogin(String userLoginId) {
>          GenericValue userLogin = EntityQuery.use(delegator)
>                  .from('UserLogin').where(userLoginId: userLoginId).queryOne()
>          assert userLogin
>          return userLogin
>      }
>
> We could even add a default login user.
>
>      // Retrieves the default login record.
>      GenericValue getUserLogin() {
>          return getUserLogin('system');
>      }
>
> I guess we should add such method directly in the ‘OFBizTestCase’ class
> to be able to reuse it in all test cases and avoid having to pass the
> ‘delegator’ and ‘dispatcher’ as method arguments.
>
> The creation of the service input map of your example would look like
> this:
>
>       void testSendOrderChangeNotification() {
>           Map serviceCtx = [
>               orderId: 'TEST_DEMO10090',
>               sendTo: 'test_email@example.com',
>               userLogin: getUserLogin()
>           ]
>           Map serviceResult = dispatcher.runSync('sendOrderChangeNotification', serviceCtx)
>           assert ServiceUtil.isSuccess(serviceResult)
>           assert serviceResult.emailType.equals("PRDS_ODR_CHANGE")
>       }
>
> In any case I think that finding the generic and reusable helper methods
> can be done incrementally.
>
> What do you think?

Thanks Mathieu,

That sounds like a good idea to me. I have created OFBIZ-11247 for that

Jacques


Re: Providing utilitaries for integration tests

Posted by Mathieu Lirzin <ma...@nereide.fr>.
Hello Jacques,

Jacques Le Roux <ja...@les7arts.com> writes:

> So we give up here, right?

I think providing helper methods for integration tests is a good long
term idea, So I don't think we should give up. :-)

> Le 08/10/2019 à 13:12, Jacques Le Roux a écrit :
>
>> Hi Mathieu
>>
>> I forgot to mention that I put this method in GroovyUtil.java.
>>
>> It was hastily done because doing so I rapidly thought that "Anyway it would not add much, just a cover function"
>>
>> It works with a complete/correct version:
>>
>>     public static Map testGroovy(Delegator delegator, LocalDispatcher dispatcher, Map<String, Object> serviceCtx,
>>             String serviceName) throws GenericEntityException, GenericServiceException {
>>         GenericValue userLogin = EntityQuery.use(delegator)
>>                 .from("UserLogin")
>>                 .where("userLoginId", "system")
>>                 .cache()
>>                 .queryOne();
>>         serviceCtx.put("userLogin", userLogin);
>>         return dispatcher.runSync(serviceName, serviceCtx);
>>     }
>>
>> I still wonder if it of much use, ie compare:
>>
>>     void testSendOrderChangeNotification() {
>>         Map serviceCtx = [
>>             orderId: 'TEST_DEMO10090',
>>             sendTo: 'test_email@example.com',
>>         ]
>>         Map serviceResult = GroovyUtil.testGroovy(delegator, dispatcher, serviceCtx, 'sendOrderChangeNotification');
>>         assert ServiceUtil.isSuccess(serviceResult)
>>         assert serviceResult.emailType.equals("PRDS_ODR_CHANGE")
>>     }
>>
>> with
>>
>>     void testSendOrderChangeNotification() {
>>         Map serviceCtx = [
>>             orderId: 'TEST_DEMO10090',
>>             sendTo: 'test_email@example.com',
>>             userLogin: EntityQuery.use(delegator).from('UserLogin').where('userLoginId', 'system').cache().queryOne()
>>         ]
>>         Map serviceResult = dispatcher.runSync('sendOrderChangeNotification', serviceCtx)
>>         assert ServiceUtil.isSuccess(serviceResult)
>>         assert serviceResult.emailType.equals("PRDS_ODR_CHANGE")
>>     }
>>
>> It's shorter and can be used for most Groovy tests. But you have to
>> pass the delegator, dispatcher and serviceCtx which are else
>> transparent. Not sure it's of better use.
>>
>> What do you think?

I agree that that having to pass the delegator, dispatcher and
serviceCtx is not ideal and tend make the test less clear.

In order to avoid repetitive code a nice first helper method would be
for example one for retrieving the default userLogin like what is done
in ‘QuoteTests.groovy’

    // Retrieves a particular login record.
    GenericValue getUserLogin(String userLoginId) {
        GenericValue userLogin = EntityQuery.use(delegator)
                .from('UserLogin').where(userLoginId: userLoginId).queryOne()
        assert userLogin
        return userLogin
    }

We could even add a default login user.

    // Retrieves the default login record.
    GenericValue getUserLogin() {
        return getUserLogin('system');
    }

I guess we should add such method directly in the ‘OFBizTestCase’ class
to be able to reuse it in all test cases and avoid having to pass the
‘delegator’ and ‘dispatcher’ as method arguments.

The creation of the service input map of your example would look like
this:

     void testSendOrderChangeNotification() {
         Map serviceCtx = [
             orderId: 'TEST_DEMO10090',
             sendTo: 'test_email@example.com',
             userLogin: getUserLogin()
         ]
         Map serviceResult = dispatcher.runSync('sendOrderChangeNotification', serviceCtx)
         assert ServiceUtil.isSuccess(serviceResult)
         assert serviceResult.emailType.equals("PRDS_ODR_CHANGE")
     }

In any case I think that finding the generic and reusable helper methods
can be done incrementally.

What do you think?

-- 
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37

Re: Providing utilitaries for integration tests

Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Mathieu,

So we give up here, right?

Jacques

Le 08/10/2019 à 13:12, Jacques Le Roux a écrit :

> Hi Mathieu
>
> I forgot to mention that I put this method in GroovyUtil.java.
>
> It was hastily done because doing so I rapidly thought that "Anyway it would not add much, just a cover function"
>
> It works with a complete/correct version:
>
>     public static Map testGroovy(Delegator delegator, LocalDispatcher dispatcher, Map<String, Object> serviceCtx,
>             String serviceName) throws GenericEntityException, GenericServiceException {
>         GenericValue userLogin = EntityQuery.use(delegator)
>                 .from("UserLogin")
>                 .where("userLoginId", "system")
>                 .cache()
>                 .queryOne();
>         serviceCtx.put("userLogin", userLogin);
>         return dispatcher.runSync(serviceName, serviceCtx);
>     }
>
> I still wonder if it of much use, ie compare:
>
>     void testSendOrderChangeNotification() {
>         Map serviceCtx = [
>             orderId: 'TEST_DEMO10090',
>             sendTo: 'test_email@example.com',
>         ]
>         Map serviceResult = GroovyUtil.testGroovy(delegator, dispatcher, serviceCtx, 'sendOrderChangeNotification');
>         assert ServiceUtil.isSuccess(serviceResult)
>         assert serviceResult.emailType.equals("PRDS_ODR_CHANGE")
>     }
>
> with
>
>     void testSendOrderChangeNotification() {
>         Map serviceCtx = [
>             orderId: 'TEST_DEMO10090',
>             sendTo: 'test_email@example.com',
>             userLogin: EntityQuery.use(delegator).from('UserLogin').where('userLoginId', 'system').cache().queryOne()
>         ]
>         Map serviceResult = dispatcher.runSync('sendOrderChangeNotification', serviceCtx)
>         assert ServiceUtil.isSuccess(serviceResult)
>         assert serviceResult.emailType.equals("PRDS_ODR_CHANGE")
>     }
>
> It's shorter and can be used for most Groovy tests. But you have to pass the delegator, dispatcher and serviceCtx which are else transparent. Not 
> sure it's of better use.
>
> What do you think?
>
> Jacques
>
> Le 07/10/2019 à 13:49, Mathieu Lirzin a écrit :
>> Hello Jacques,
>>
>> Jacques Le Roux <ja...@les7arts.com> writes:
>>
>>> I had a look at this idea but I finally gave up. I thought about creating
>>>
>>>      public static Map testGroovy(Map<String, String> paramNames, String serviceName) {
>>>          userLogin = EntityQuery.use(delegator).from('UserLogin').where('userLoginId', 'system').cache().queryOne()
>>>          Map serviceCtx = paramNames.put("userLogin", EntityQuery.use(delegator).from('UserLogin').where('userLoginId', 'system').cache().queryOne())
>>>          Map result = dispatcher.runSync(serviceName, serviceCtx)
>>>      }
>>>
>>> And only pass params and service name, but EntityQuery is not yet available in base component when compiling.
>> What do you mean by “EntityQuery is not yet available in base component
>> when compiling”?
>>
>> Can you provide a patch and the command you run to get to this failing
>> scenario?
>>
>>> Anyway it would add much, just a cover function
>>>
>>> Le 03/10/2019 à 14:29, Jacques Le Roux a écrit :
>>>> BTW looking at OrderTests.groovy it seems we can refactor Groovy tests, a lot of is common...
>>>>
>> -- 
>> Jacques Le Roux 


Re: Providing utilitaries for integration tests

Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Mathieu

I forgot to mention that I put this method in GroovyUtil.java.

It was hastily done because doing so I rapidly thought that "Anyway it would not add much, just a cover function"

It works with a complete/correct version:

     public static Map testGroovy(Delegator delegator, LocalDispatcher dispatcher, Map<String, Object> serviceCtx,
             String serviceName) throws GenericEntityException, GenericServiceException {
         GenericValue userLogin = EntityQuery.use(delegator)
                 .from("UserLogin")
                 .where("userLoginId", "system")
                 .cache()
                 .queryOne();
         serviceCtx.put("userLogin", userLogin);
         return dispatcher.runSync(serviceName, serviceCtx);
     }

I still wonder if it of much use, ie compare:

     void testSendOrderChangeNotification() {
         Map serviceCtx = [
             orderId: 'TEST_DEMO10090',
             sendTo: 'test_email@example.com',
         ]
         Map serviceResult = GroovyUtil.testGroovy(delegator, dispatcher, serviceCtx, 'sendOrderChangeNotification');
         assert ServiceUtil.isSuccess(serviceResult)
         assert serviceResult.emailType.equals("PRDS_ODR_CHANGE")
     }

with

     void testSendOrderChangeNotification() {
         Map serviceCtx = [
             orderId: 'TEST_DEMO10090',
             sendTo: 'test_email@example.com',
             userLogin: EntityQuery.use(delegator).from('UserLogin').where('userLoginId', 'system').cache().queryOne()
         ]
         Map serviceResult = dispatcher.runSync('sendOrderChangeNotification', serviceCtx)
         assert ServiceUtil.isSuccess(serviceResult)
         assert serviceResult.emailType.equals("PRDS_ODR_CHANGE")
     }

It's shorter and can be used for most Groovy tests. But you have to pass the delegator, dispatcher and serviceCtx which are else transparent. Not sure 
it's of better use.

What do you think?

Jacques

Le 07/10/2019 à 13:49, Mathieu Lirzin a écrit :
> Hello Jacques,
>
> Jacques Le Roux <ja...@les7arts.com> writes:
>
>> I had a look at this idea but I finally gave up. I thought about creating
>>
>>      public static Map testGroovy(Map<String, String> paramNames, String serviceName) {
>>          userLogin = EntityQuery.use(delegator).from('UserLogin').where('userLoginId', 'system').cache().queryOne()
>>          Map serviceCtx = paramNames.put("userLogin", EntityQuery.use(delegator).from('UserLogin').where('userLoginId', 'system').cache().queryOne())
>>          Map result = dispatcher.runSync(serviceName, serviceCtx)
>>      }
>>
>> And only pass params and service name, but EntityQuery is not yet available in base component when compiling.
> What do you mean by “EntityQuery is not yet available in base component
> when compiling”?
>
> Can you provide a patch and the command you run to get to this failing
> scenario?
>
>> Anyway it would add much, just a cover function
>>
>> Le 03/10/2019 à 14:29, Jacques Le Roux a écrit :
>>> BTW looking at OrderTests.groovy it seems we can refactor Groovy tests, a lot of is common...
>>>
> -- 
> Jacques Le Roux
> 400E Chemin de la Mouline
> 34560 Poussan
> 04 67 51 19 38
> 06 11 79 50 28

Providing utilitaries for integration tests (was: svn commit: r1867889 …)

Posted by Mathieu Lirzin <ma...@nereide.fr>.
Hello Jacques,

Jacques Le Roux <ja...@les7arts.com> writes:

> I had a look at this idea but I finally gave up. I thought about creating
>
>     public static Map testGroovy(Map<String, String> paramNames, String serviceName) {
>         userLogin = EntityQuery.use(delegator).from('UserLogin').where('userLoginId', 'system').cache().queryOne()
>         Map serviceCtx = paramNames.put("userLogin", EntityQuery.use(delegator).from('UserLogin').where('userLoginId', 'system').cache().queryOne())
>         Map result = dispatcher.runSync(serviceName, serviceCtx)
>     }
>
> And only pass params and service name, but EntityQuery is not yet available in base component when compiling.

What do you mean by “EntityQuery is not yet available in base component
when compiling”?

Can you provide a patch and the command you run to get to this failing
scenario?

> Anyway it would add much, just a cover function
>
> Le 03/10/2019 à 14:29, Jacques Le Roux a écrit :
>> BTW looking at OrderTests.groovy it seems we can refactor Groovy tests, a lot of is common...
>>

-- 
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37

Re: svn commit: r1867889 - in /ofbiz/ofbiz-framework/trunk/applications: datamodel/data/demo/OrderDemoData.xml order/minilang/test/OrderTest.xml order/servicedef/secas.xml order/testdef/OrderTest.xml order/testdef/data/OrderTestData.xml

Posted by Jacques Le Roux <ja...@les7arts.com>.
I had a look at this idea but I finally gave up. I thought about creating

     public static Map testGroovy(Map<String, String> paramNames, String serviceName) {
         userLogin = EntityQuery.use(delegator).from('UserLogin').where('userLoginId', 'system').cache().queryOne()
         Map serviceCtx = paramNames.put("userLogin", EntityQuery.use(delegator).from('UserLogin').where('userLoginId', 'system').cache().queryOne())
         Map result = dispatcher.runSync(serviceName, serviceCtx)
     }

And only pass params and service name, but EntityQuery is not yet available in base component when compiling.

Anyway it would add much, just a cover function

Jacques

Le 03/10/2019 à 14:29, Jacques Le Roux a écrit :
> BTW looking at OrderTests.groovy it seems we can refactor Groovy tests, a lot of is common...
>
> Jacques
>
> Le 03/10/2019 à 13:33, Jacques Le Roux a écrit :
>> r1867927 is the answer
>>
>> Jacques
>>
>> Le 03/10/2019 à 12:10, Mathieu Lirzin a écrit :
>>> Hello Jacques,
>>>
>>> Jacques Le Roux <ja...@les7arts.com> writes:
>>>
>>>> Le 02/10/2019 à 17:21, Mathieu Lirzin a écrit :
>>>>
>>>>> jleroux@apache.org writes:
>>>>>
>>>>>> Author: jleroux
>>>>>> Date: Wed Oct  2 14:46:00 2019
>>>>>> New Revision: 1867889
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=1867889&view=rev
>>>>>> Log:
>>>>>> Improved: Unit test case for service - SendOrderBackorderNotification
>>>>>> (OFBIZ-8810)(OFBIZ-9647)(OFBIZ-9671)
>>>>>>
>>>>>> While working on this I stumbled upon an issue related with
>>>>>> webSiteId="OrderEntry" well related by Ratnesh Upadhyay in OFBIZ-9647.
>>>>>>
>>>>>> Unlike him I decided not to remove the webSiteId="OrderEntry" entries but to
>>>>>> replace them by webSiteId="WebStore"
>>>>>>
>>>>>> Added:
>>>>>> ofbiz/ofbiz-framework/trunk/applications/order/minilang/test/OrderTest.xml (with props)
>>>>> It would be nice if you could rewrite this "integration" test in Groovy
>>>>> or Java.
>>>> Yes of course (Groovy preferably IMO)
>>>>
>>>>
>>>>> If I remember correctly we have decided to not accept new minilang code
>>>>> in our codebase. Am I overlooking something?
>>>>>
>>>>>> Modified:
>>>>>> ofbiz/ofbiz-framework/trunk/applications/datamodel/data/demo/OrderDemoData.xml
>>>>>> ofbiz/ofbiz-framework/trunk/applications/order/servicedef/secas.xml
>>>>>> ofbiz/ofbiz-framework/trunk/applications/order/testdef/OrderTest.xml
>>>>>> ofbiz/ofbiz-framework/trunk/applications/order/testdef/data/OrderTestData.xml
>>>>> Thanks.
>>>>>
>>>> We have discussed this already and here is a special case as commented at bottom of OFBIZ-1463 and at https://markmail.org/message/ute4ulnetz2h4ed5
>>>>
>>>> I don't want to lose the work embedded in this and 15 others patches available at OFBIZ-1463 (to be checked one by one).
>>>>
>>>> Integration tests are data driven, I mean that when you have defined the data the test itself mostly follows.
>>>> Most of the time it's not a big deal to switch the test from Minilang to Groovy.
>>>> I/we can do it after a 1st pass where we value the current patches (already old for 4 years)
>>>>
>>>> BTW, working on this one I discovered an issue with <<webSiteId="orderEntry">>. It was not really part of the patch, but the patch revealed it.
>>>> Hence (OFBIZ-9647)(OFBIZ-9671) also in this improvement, almost a bug actually for these 2 Jiras. It was 90% of the time I passed on this commit.
>>>> It's incidental but part of the process and I don't want to loose the efforts put in the remaining available patches.
>>>> It's also a matter of not overloading my mind,  step by step, if you want.
>>>>
>>>> So I'll continue to check these patches, apply them and later migrate tests to Groovy.
>>>> Then I expect the tests will have been validated and it will be almost mechanical to migrate them.
>>>>
>>>> But you are right and I'll open new Jiras for migrating them, knowing
>>>> that we have a 1300+ others tests to migrate[1]! (ie here it's  1%+,
>>>> and I expect simple ones :))
>>> Sorry, but I still don't buy your arguments. :-)
>>>
>>> I will repeat what I said in the previous discussion:
>>>
>>> * If this is not a big deal to migrate integration tests *why don't you
>>>    just™* do the migration work before committing those patches?
>>>
>>> * If you don't have time to do it right now, chances are that you won't
>>>    have time for it later, so adding more minilang simply means more
>>>    burden on others.
>>>
>>> I would prefer that we settle this disagreement before you go ahead.
>>>
>>
>

Re: svn commit: r1867889 - in /ofbiz/ofbiz-framework/trunk/applications: datamodel/data/demo/OrderDemoData.xml order/minilang/test/OrderTest.xml order/servicedef/secas.xml order/testdef/OrderTest.xml order/testdef/data/OrderTestData.xml

Posted by Jacques Le Roux <ja...@les7arts.com>.
BTW looking at OrderTests.groovy it seems we can refactor Groovy tests, a lot of is common...

Jacques

Le 03/10/2019 à 13:33, Jacques Le Roux a écrit :
> r1867927 is the answer
>
> Jacques
>
> Le 03/10/2019 à 12:10, Mathieu Lirzin a écrit :
>> Hello Jacques,
>>
>> Jacques Le Roux <ja...@les7arts.com> writes:
>>
>>> Le 02/10/2019 à 17:21, Mathieu Lirzin a écrit :
>>>
>>>> jleroux@apache.org writes:
>>>>
>>>>> Author: jleroux
>>>>> Date: Wed Oct  2 14:46:00 2019
>>>>> New Revision: 1867889
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1867889&view=rev
>>>>> Log:
>>>>> Improved: Unit test case for service - SendOrderBackorderNotification
>>>>> (OFBIZ-8810)(OFBIZ-9647)(OFBIZ-9671)
>>>>>
>>>>> While working on this I stumbled upon an issue related with
>>>>> webSiteId="OrderEntry" well related by Ratnesh Upadhyay in OFBIZ-9647.
>>>>>
>>>>> Unlike him I decided not to remove the webSiteId="OrderEntry" entries but to
>>>>> replace them by webSiteId="WebStore"
>>>>>
>>>>> Added:
>>>>> ofbiz/ofbiz-framework/trunk/applications/order/minilang/test/OrderTest.xml (with props)
>>>> It would be nice if you could rewrite this "integration" test in Groovy
>>>> or Java.
>>> Yes of course (Groovy preferably IMO)
>>>
>>>
>>>> If I remember correctly we have decided to not accept new minilang code
>>>> in our codebase. Am I overlooking something?
>>>>
>>>>> Modified:
>>>>> ofbiz/ofbiz-framework/trunk/applications/datamodel/data/demo/OrderDemoData.xml
>>>>> ofbiz/ofbiz-framework/trunk/applications/order/servicedef/secas.xml
>>>>> ofbiz/ofbiz-framework/trunk/applications/order/testdef/OrderTest.xml
>>>>> ofbiz/ofbiz-framework/trunk/applications/order/testdef/data/OrderTestData.xml
>>>> Thanks.
>>>>
>>> We have discussed this already and here is a special case as commented at bottom of OFBIZ-1463 and at https://markmail.org/message/ute4ulnetz2h4ed5
>>>
>>> I don't want to lose the work embedded in this and 15 others patches available at OFBIZ-1463 (to be checked one by one).
>>>
>>> Integration tests are data driven, I mean that when you have defined the data the test itself mostly follows.
>>> Most of the time it's not a big deal to switch the test from Minilang to Groovy.
>>> I/we can do it after a 1st pass where we value the current patches (already old for 4 years)
>>>
>>> BTW, working on this one I discovered an issue with <<webSiteId="orderEntry">>. It was not really part of the patch, but the patch revealed it.
>>> Hence (OFBIZ-9647)(OFBIZ-9671) also in this improvement, almost a bug actually for these 2 Jiras. It was 90% of the time I passed on this commit.
>>> It's incidental but part of the process and I don't want to loose the efforts put in the remaining available patches.
>>> It's also a matter of not overloading my mind,  step by step, if you want.
>>>
>>> So I'll continue to check these patches, apply them and later migrate tests to Groovy.
>>> Then I expect the tests will have been validated and it will be almost mechanical to migrate them.
>>>
>>> But you are right and I'll open new Jiras for migrating them, knowing
>>> that we have a 1300+ others tests to migrate[1]! (ie here it's  1%+,
>>> and I expect simple ones :))
>> Sorry, but I still don't buy your arguments. :-)
>>
>> I will repeat what I said in the previous discussion:
>>
>> * If this is not a big deal to migrate integration tests *why don't you
>>    just™* do the migration work before committing those patches?
>>
>> * If you don't have time to do it right now, chances are that you won't
>>    have time for it later, so adding more minilang simply means more
>>    burden on others.
>>
>> I would prefer that we settle this disagreement before you go ahead.
>>
>

Re: svn commit: r1867889 - in /ofbiz/ofbiz-framework/trunk/applications: datamodel/data/demo/OrderDemoData.xml order/minilang/test/OrderTest.xml order/servicedef/secas.xml order/testdef/OrderTest.xml order/testdef/data/OrderTestData.xml

Posted by Jacques Le Roux <ja...@les7arts.com>.
r1867927 is the answer

Jacques

Le 03/10/2019 à 12:10, Mathieu Lirzin a écrit :
> Hello Jacques,
>
> Jacques Le Roux <ja...@les7arts.com> writes:
>
>> Le 02/10/2019 à 17:21, Mathieu Lirzin a écrit :
>>
>>> jleroux@apache.org writes:
>>>
>>>> Author: jleroux
>>>> Date: Wed Oct  2 14:46:00 2019
>>>> New Revision: 1867889
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1867889&view=rev
>>>> Log:
>>>> Improved: Unit test case for service - SendOrderBackorderNotification
>>>> (OFBIZ-8810)(OFBIZ-9647)(OFBIZ-9671)
>>>>
>>>> While working on this I stumbled upon an issue related with
>>>> webSiteId="OrderEntry" well related by Ratnesh Upadhyay in OFBIZ-9647.
>>>>
>>>> Unlike him I decided not to remove the webSiteId="OrderEntry" entries but to
>>>> replace them by webSiteId="WebStore"
>>>>
>>>> Added:
>>>>       ofbiz/ofbiz-framework/trunk/applications/order/minilang/test/OrderTest.xml   (with props)
>>> It would be nice if you could rewrite this "integration" test in Groovy
>>> or Java.
>> Yes of course (Groovy preferably IMO)
>>
>>
>>> If I remember correctly we have decided to not accept new minilang code
>>> in our codebase. Am I overlooking something?
>>>
>>>> Modified:
>>>>       ofbiz/ofbiz-framework/trunk/applications/datamodel/data/demo/OrderDemoData.xml
>>>>       ofbiz/ofbiz-framework/trunk/applications/order/servicedef/secas.xml
>>>>       ofbiz/ofbiz-framework/trunk/applications/order/testdef/OrderTest.xml
>>>>       ofbiz/ofbiz-framework/trunk/applications/order/testdef/data/OrderTestData.xml
>>> Thanks.
>>>
>> We have discussed this already and here is a special case as commented at bottom of OFBIZ-1463 and at https://markmail.org/message/ute4ulnetz2h4ed5
>>
>> I don't want to lose the work embedded in this and 15 others patches available at OFBIZ-1463 (to be checked one by one).
>>
>> Integration tests are data driven, I mean that when you have defined the data the test itself mostly follows.
>> Most of the time it's not a big deal to switch the test from Minilang to Groovy.
>> I/we can do it after a 1st pass where we value the current patches (already old for 4 years)
>>
>> BTW, working on this one I discovered an issue with <<webSiteId="orderEntry">>. It was not really part of the patch, but the patch revealed it.
>> Hence (OFBIZ-9647)(OFBIZ-9671) also in this improvement, almost a bug actually for these 2 Jiras. It was 90% of the time I passed on this commit.
>> It's incidental but part of the process and I don't want to loose the efforts put in the remaining available patches.
>> It's also a matter of not overloading my mind,  step by step, if you want.
>>
>> So I'll continue to check these patches, apply them and later migrate tests to Groovy.
>> Then I expect the tests will have been validated and it will be almost mechanical to migrate them.
>>
>> But you are right and I'll open new Jiras for migrating them, knowing
>> that we have a 1300+ others tests to migrate[1]! (ie here it's  1%+,
>> and I expect simple ones :))
> Sorry, but I still don't buy your arguments. :-)
>
> I will repeat what I said in the previous discussion:
>
> * If this is not a big deal to migrate integration tests *why don't you
>    just™* do the migration work before committing those patches?
>
> * If you don't have time to do it right now, chances are that you won't
>    have time for it later, so adding more minilang simply means more
>    burden on others.
>
> I would prefer that we settle this disagreement before you go ahead.
>

Re: svn commit: r1867889 - in /ofbiz/ofbiz-framework/trunk/applications: datamodel/data/demo/OrderDemoData.xml order/minilang/test/OrderTest.xml order/servicedef/secas.xml order/testdef/OrderTest.xml order/testdef/data/OrderTestData.xml

Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Suraj,

Actually those assigned to Yogesh Naroliya have XML patches and needs cleaning.

After doing it for OFBIZ-8810 and OFBIZ-8811, I find quite easy to convert those tests to Groovy, WIP...

Jacques

Le 03/10/2019 à 11:25, Suraj Khurana a écrit :
> Hello Jacques/Mathieu,
>
> I think we can use the same ticket with new patch, WDYT. (If it is not
> closed)
> We have been following this pattern since last many commits under
> OFBIZ-1463.
>
> Currently, I think most of the Patch Available tickets under OFBIZ-1463
> have converted groovy patch. Only 2 tickets are pending with Patch
> Available status and not assigned to anyone (means they have XML patch).
> Few tickets under OFBIZ-1463 with Patch available and assigned to me have
> groovy patch, I will commit them shortly if we agree on this.
>
> OFBIZ-11232 <https://issues.apache.org/jira/browse/OFBIZ-11232> will be
> used for all existing XML to groovy conversion. +1.
>
> Please share your thoughts on the same. Am I missing something?
> --
> Best Regards,
> Suraj Khurana
> Technical Consultant
> HotWax Systems
>
>
>
>
>
>
> On Thu, Oct 3, 2019 at 1:57 PM Jacques Le Roux <ja...@les7arts.com>
> wrote:
>
>> Le 03/10/2019 à 09:34, Jacques Le Roux a écrit :
>>> But you are right and I'll open new Jiras for migrating them, knowing
>> that we have a 1300+ others tests to migrate[1]! (ie here it's 1%+, and I
>>> expect simple ones :))
>> Started with OFBIZ-11232
>>
>> Jacques
>>
>>

Re: svn commit: r1867889 - in /ofbiz/ofbiz-framework/trunk/applications: datamodel/data/demo/OrderDemoData.xml order/minilang/test/OrderTest.xml order/servicedef/secas.xml order/testdef/OrderTest.xml order/testdef/data/OrderTestData.xml

Posted by Suraj Khurana <su...@hotwax.co>.
Sure Jacques. You can definitely proceed with the available patches.

Not an issue at all. We all are part of this dynamic community where we
work together as a team. :)

Thanks a lot for asking and taking care of this.
--
Best Regards,
Suraj Khurana
Technical Consultant
HotWax Systems






On Sat, Oct 5, 2019 at 2:33 PM Jacques Le Roux <ja...@les7arts.com>
wrote:

> If you don't mind Suraj,
>
> I'll commit the available patches
>
> Jacques
>
> Le 03/10/2019 à 13:40, Jacques Le Roux a écrit :
> > I agree Suraj,
> >
> > Jacques
> >
> > Le 03/10/2019 à 11:25, Suraj Khurana a écrit :
> >> Hello Jacques/Mathieu,
> >>
> >> I think we can use the same ticket with new patch, WDYT. (If it is not
> >> closed)
> >> We have been following this pattern since last many commits under
> >> OFBIZ-1463.
> >>
> >> Currently, I think most of the Patch Available tickets under OFBIZ-1463
> >> have converted groovy patch. Only 2 tickets are pending with Patch
> >> Available status and not assigned to anyone (means they have XML patch).
> >> Few tickets under OFBIZ-1463 with Patch available and assigned to me
> have
> >> groovy patch, I will commit them shortly if we agree on this.
> >>
> >> OFBIZ-11232 <https://issues.apache.org/jira/browse/OFBIZ-11232> will be
> >> used for all existing XML to groovy conversion. +1.
> >>
> >> Please share your thoughts on the same. Am I missing something?
> >> --
> >> Best Regards,
> >> Suraj Khurana
> >> Technical Consultant
> >> HotWax Systems
> >>
> >>
> >>
> >>
> >>
> >>
> >> On Thu, Oct 3, 2019 at 1:57 PM Jacques Le Roux <
> jacques.le.roux@les7arts.com>
> >> wrote:
> >>
> >>> Le 03/10/2019 à 09:34, Jacques Le Roux a écrit :
> >>>> But you are right and I'll open new Jiras for migrating them, knowing
> >>> that we have a 1300+ others tests to migrate[1]! (ie here it's 1%+,
> and I
> >>>> expect simple ones :))
> >>> Started with OFBIZ-11232
> >>>
> >>> Jacques
> >>>
> >>>
> >
>

Re: svn commit: r1867889 - in /ofbiz/ofbiz-framework/trunk/applications: datamodel/data/demo/OrderDemoData.xml order/minilang/test/OrderTest.xml order/servicedef/secas.xml order/testdef/OrderTest.xml order/testdef/data/OrderTestData.xml

Posted by Jacques Le Roux <ja...@les7arts.com>.
If you don't mind Suraj,

I'll commit the available patches

Jacques

Le 03/10/2019 à 13:40, Jacques Le Roux a écrit :
> I agree Suraj,
>
> Jacques
>
> Le 03/10/2019 à 11:25, Suraj Khurana a écrit :
>> Hello Jacques/Mathieu,
>>
>> I think we can use the same ticket with new patch, WDYT. (If it is not
>> closed)
>> We have been following this pattern since last many commits under
>> OFBIZ-1463.
>>
>> Currently, I think most of the Patch Available tickets under OFBIZ-1463
>> have converted groovy patch. Only 2 tickets are pending with Patch
>> Available status and not assigned to anyone (means they have XML patch).
>> Few tickets under OFBIZ-1463 with Patch available and assigned to me have
>> groovy patch, I will commit them shortly if we agree on this.
>>
>> OFBIZ-11232 <https://issues.apache.org/jira/browse/OFBIZ-11232> will be
>> used for all existing XML to groovy conversion. +1.
>>
>> Please share your thoughts on the same. Am I missing something?
>> -- 
>> Best Regards,
>> Suraj Khurana
>> Technical Consultant
>> HotWax Systems
>>
>>
>>
>>
>>
>>
>> On Thu, Oct 3, 2019 at 1:57 PM Jacques Le Roux <ja...@les7arts.com>
>> wrote:
>>
>>> Le 03/10/2019 à 09:34, Jacques Le Roux a écrit :
>>>> But you are right and I'll open new Jiras for migrating them, knowing
>>> that we have a 1300+ others tests to migrate[1]! (ie here it's 1%+, and I
>>>> expect simple ones :))
>>> Started with OFBIZ-11232
>>>
>>> Jacques
>>>
>>>
>

Re: svn commit: r1867889 - in /ofbiz/ofbiz-framework/trunk/applications: datamodel/data/demo/OrderDemoData.xml order/minilang/test/OrderTest.xml order/servicedef/secas.xml order/testdef/OrderTest.xml order/testdef/data/OrderTestData.xml

Posted by Jacques Le Roux <ja...@les7arts.com>.
I agree Suraj,

Jacques

Le 03/10/2019 à 11:25, Suraj Khurana a écrit :
> Hello Jacques/Mathieu,
>
> I think we can use the same ticket with new patch, WDYT. (If it is not
> closed)
> We have been following this pattern since last many commits under
> OFBIZ-1463.
>
> Currently, I think most of the Patch Available tickets under OFBIZ-1463
> have converted groovy patch. Only 2 tickets are pending with Patch
> Available status and not assigned to anyone (means they have XML patch).
> Few tickets under OFBIZ-1463 with Patch available and assigned to me have
> groovy patch, I will commit them shortly if we agree on this.
>
> OFBIZ-11232 <https://issues.apache.org/jira/browse/OFBIZ-11232> will be
> used for all existing XML to groovy conversion. +1.
>
> Please share your thoughts on the same. Am I missing something?
> --
> Best Regards,
> Suraj Khurana
> Technical Consultant
> HotWax Systems
>
>
>
>
>
>
> On Thu, Oct 3, 2019 at 1:57 PM Jacques Le Roux <ja...@les7arts.com>
> wrote:
>
>> Le 03/10/2019 à 09:34, Jacques Le Roux a écrit :
>>> But you are right and I'll open new Jiras for migrating them, knowing
>> that we have a 1300+ others tests to migrate[1]! (ie here it's 1%+, and I
>>> expect simple ones :))
>> Started with OFBIZ-11232
>>
>> Jacques
>>
>>

Re: svn commit: r1867889 - in /ofbiz/ofbiz-framework/trunk/applications: datamodel/data/demo/OrderDemoData.xml order/minilang/test/OrderTest.xml order/servicedef/secas.xml order/testdef/OrderTest.xml order/testdef/data/OrderTestData.xml

Posted by Suraj Khurana <su...@hotwax.co>.
Hello Jacques/Mathieu,

I think we can use the same ticket with new patch, WDYT. (If it is not
closed)
We have been following this pattern since last many commits under
OFBIZ-1463.

Currently, I think most of the Patch Available tickets under OFBIZ-1463
have converted groovy patch. Only 2 tickets are pending with Patch
Available status and not assigned to anyone (means they have XML patch).
Few tickets under OFBIZ-1463 with Patch available and assigned to me have
groovy patch, I will commit them shortly if we agree on this.

OFBIZ-11232 <https://issues.apache.org/jira/browse/OFBIZ-11232> will be
used for all existing XML to groovy conversion. +1.

Please share your thoughts on the same. Am I missing something?
--
Best Regards,
Suraj Khurana
Technical Consultant
HotWax Systems






On Thu, Oct 3, 2019 at 1:57 PM Jacques Le Roux <ja...@les7arts.com>
wrote:

> Le 03/10/2019 à 09:34, Jacques Le Roux a écrit :
> > But you are right and I'll open new Jiras for migrating them, knowing
> that we have a 1300+ others tests to migrate[1]! (ie here it's 1%+, and I
> > expect simple ones :))
> Started with OFBIZ-11232
>
> Jacques
>
>

Re: svn commit: r1867889 - in /ofbiz/ofbiz-framework/trunk/applications: datamodel/data/demo/OrderDemoData.xml order/minilang/test/OrderTest.xml order/servicedef/secas.xml order/testdef/OrderTest.xml order/testdef/data/OrderTestData.xml

Posted by Jacques Le Roux <ja...@les7arts.com>.
Le 03/10/2019 à 09:34, Jacques Le Roux a écrit :
> But you are right and I'll open new Jiras for migrating them, knowing that we have a 1300+ others tests to migrate[1]! (ie here it's 1%+, and I 
> expect simple ones :)) 
Started with OFBIZ-11232

Jacques


Re: svn commit: r1867889 - in /ofbiz/ofbiz-framework/trunk/applications: datamodel/data/demo/OrderDemoData.xml order/minilang/test/OrderTest.xml order/servicedef/secas.xml order/testdef/OrderTest.xml order/testdef/data/OrderTestData.xml

Posted by Mathieu Lirzin <ma...@nereide.fr>.
Hello Jacques,

Jacques Le Roux <ja...@les7arts.com> writes:

> Le 02/10/2019 à 17:21, Mathieu Lirzin a écrit :
>
>> jleroux@apache.org writes:
>>
>>> Author: jleroux
>>> Date: Wed Oct  2 14:46:00 2019
>>> New Revision: 1867889
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1867889&view=rev
>>> Log:
>>> Improved: Unit test case for service - SendOrderBackorderNotification
>>> (OFBIZ-8810)(OFBIZ-9647)(OFBIZ-9671)
>>>
>>> While working on this I stumbled upon an issue related with
>>> webSiteId="OrderEntry" well related by Ratnesh Upadhyay in OFBIZ-9647.
>>>
>>> Unlike him I decided not to remove the webSiteId="OrderEntry" entries but to
>>> replace them by webSiteId="WebStore"
>>>
>>> Added:
>>>      ofbiz/ofbiz-framework/trunk/applications/order/minilang/test/OrderTest.xml   (with props)
>> It would be nice if you could rewrite this "integration" test in Groovy
>> or Java.
>
> Yes of course (Groovy preferably IMO)
>
>
>>
>> If I remember correctly we have decided to not accept new minilang code
>> in our codebase. Am I overlooking something?
>>
>>> Modified:
>>>      ofbiz/ofbiz-framework/trunk/applications/datamodel/data/demo/OrderDemoData.xml
>>>      ofbiz/ofbiz-framework/trunk/applications/order/servicedef/secas.xml
>>>      ofbiz/ofbiz-framework/trunk/applications/order/testdef/OrderTest.xml
>>>      ofbiz/ofbiz-framework/trunk/applications/order/testdef/data/OrderTestData.xml
>> Thanks.
>>
>
> We have discussed this already and here is a special case as commented at bottom of OFBIZ-1463 and at https://markmail.org/message/ute4ulnetz2h4ed5
>
> I don't want to lose the work embedded in this and 15 others patches available at OFBIZ-1463 (to be checked one by one).
>
> Integration tests are data driven, I mean that when you have defined the data the test itself mostly follows.
> Most of the time it's not a big deal to switch the test from Minilang to Groovy.
> I/we can do it after a 1st pass where we value the current patches (already old for 4 years)
>
> BTW, working on this one I discovered an issue with <<webSiteId="orderEntry">>. It was not really part of the patch, but the patch revealed it.
> Hence (OFBIZ-9647)(OFBIZ-9671) also in this improvement, almost a bug actually for these 2 Jiras. It was 90% of the time I passed on this commit.
> It's incidental but part of the process and I don't want to loose the efforts put in the remaining available patches.
> It's also a matter of not overloading my mind,  step by step, if you want.
>
> So I'll continue to check these patches, apply them and later migrate tests to Groovy.
> Then I expect the tests will have been validated and it will be almost mechanical to migrate them.
>
> But you are right and I'll open new Jiras for migrating them, knowing
> that we have a 1300+ others tests to migrate[1]! (ie here it's  1%+,
> and I expect simple ones :))

Sorry, but I still don't buy your arguments. :-)

I will repeat what I said in the previous discussion:

* If this is not a big deal to migrate integration tests *why don't you
  just™* do the migration work before committing those patches?

* If you don't have time to do it right now, chances are that you won't
  have time for it later, so adding more minilang simply means more
  burden on others.

I would prefer that we settle this disagreement before you go ahead.

-- 
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37

Re: svn commit: r1867889 - in /ofbiz/ofbiz-framework/trunk/applications: datamodel/data/demo/OrderDemoData.xml order/minilang/test/OrderTest.xml order/servicedef/secas.xml order/testdef/OrderTest.xml order/testdef/data/OrderTestData.xml

Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Mathieu,

Le 02/10/2019 à 17:21, Mathieu Lirzin a écrit :

> Hello Jacques,
>
> jleroux@apache.org writes:
>
>> Author: jleroux
>> Date: Wed Oct  2 14:46:00 2019
>> New Revision: 1867889
>>
>> URL: http://svn.apache.org/viewvc?rev=1867889&view=rev
>> Log:
>> Improved: Unit test case for service - SendOrderBackorderNotification
>> (OFBIZ-8810)(OFBIZ-9647)(OFBIZ-9671)
>>
>> While working on this I stumbled upon an issue related with
>> webSiteId="OrderEntry" well related by Ratnesh Upadhyay in OFBIZ-9647.
>>
>> Unlike him I decided not to remove the webSiteId="OrderEntry" entries but to
>> replace them by webSiteId="WebStore"
>>
>> Added:
>>      ofbiz/ofbiz-framework/trunk/applications/order/minilang/test/OrderTest.xml   (with props)
> It would be nice if you could rewrite this "integration" test in Groovy
> or Java.

Yes of course (Groovy preferably IMO)


>
> If I remember correctly we have decided to not accept new minilang code
> in our codebase. Am I overlooking something?
>
>> Modified:
>>      ofbiz/ofbiz-framework/trunk/applications/datamodel/data/demo/OrderDemoData.xml
>>      ofbiz/ofbiz-framework/trunk/applications/order/servicedef/secas.xml
>>      ofbiz/ofbiz-framework/trunk/applications/order/testdef/OrderTest.xml
>>      ofbiz/ofbiz-framework/trunk/applications/order/testdef/data/OrderTestData.xml
> Thanks.
>

We have discussed this already and here is a special case as commented at bottom of OFBIZ-1463 and at https://markmail.org/message/ute4ulnetz2h4ed5

I don't want to lose the work embedded in this and 15 others patches available at OFBIZ-1463 (to be checked one by one).

Integration tests are data driven, I mean that when you have defined the data the test itself mostly follows.
Most of the time it's not a big deal to switch the test from Minilang to Groovy.
I/we can do it after a 1st pass where we value the current patches (already old for 4 years)

BTW, working on this one I discovered an issue with <<webSiteId="orderEntry">>. It was not really part of the patch, but the patch revealed it.
Hence (OFBIZ-9647)(OFBIZ-9671) also in this improvement, almost a bug actually for these 2 Jiras. It was 90% of the time I passed on this commit.
It's incidental but part of the process and I don't want to loose the efforts put in the remaining available patches.
It's also a matter of not overloading my mind,  step by step, if you want.

So I'll continue to check these patches, apply them and later migrate tests to Groovy.
Then I expect the tests will have been validated and it will be almost mechanical to migrate them.

But you are right and I'll open new Jiras for migrating them, knowing that we have a 1300+ others tests to migrate[1]! (ie here it's  1%+, and I 
expect simple ones :))

[1] https://ci.apache.org/projects/ofbiz/logs/trunk/plugins/html/

Jacques


Re: svn commit: r1867889 - in /ofbiz/ofbiz-framework/trunk/applications: datamodel/data/demo/OrderDemoData.xml order/minilang/test/OrderTest.xml order/servicedef/secas.xml order/testdef/OrderTest.xml order/testdef/data/OrderTestData.xml

Posted by Mathieu Lirzin <ma...@nereide.fr>.
Hello Jacques,

jleroux@apache.org writes:

> Author: jleroux
> Date: Wed Oct  2 14:46:00 2019
> New Revision: 1867889
>
> URL: http://svn.apache.org/viewvc?rev=1867889&view=rev
> Log:
> Improved: Unit test case for service - SendOrderBackorderNotification
> (OFBIZ-8810)(OFBIZ-9647)(OFBIZ-9671)
>
> While working on this I stumbled upon an issue related with 
> webSiteId="OrderEntry" well related by Ratnesh Upadhyay in OFBIZ-9647.
>
> Unlike him I decided not to remove the webSiteId="OrderEntry" entries but to
> replace them by webSiteId="WebStore"
>
> Added:
>     ofbiz/ofbiz-framework/trunk/applications/order/minilang/test/OrderTest.xml   (with props)

It would be nice if you could rewrite this "integration" test in Groovy
or Java.

If I remember correctly we have decided to not accept new minilang code
in our codebase. Am I overlooking something?

> Modified:
>     ofbiz/ofbiz-framework/trunk/applications/datamodel/data/demo/OrderDemoData.xml
>     ofbiz/ofbiz-framework/trunk/applications/order/servicedef/secas.xml
>     ofbiz/ofbiz-framework/trunk/applications/order/testdef/OrderTest.xml
>     ofbiz/ofbiz-framework/trunk/applications/order/testdef/data/OrderTestData.xml

Thanks.

-- 
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37