Re: svn commit: r1867889 - in /ofbiz/ofbiz-framework/trunk/a
Locked
Re: svn commit: r1867889 - in /ofbiz/ofbiz-framework/trunk/a
 
|
Hello Jacques,
[hidden email] 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 |
Re: svn commit: r1867889 - in /ofbiz/ofbiz-framework/trunk/a
Administrator
|
Hi Mathieu,
Le 02/10/2019 à 17:21, Mathieu Lirzin a écrit : > Hello Jacques, > > [hidden email] 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/a
|
Administrator
|
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/a
|
|
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 <[hidden email]> 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/a
|
|
In reply to this post by Jacques Le Roux
Hello Jacques,
Jacques Le Roux <[hidden email]> writes: > Le 02/10/2019 à 17:21, Mathieu Lirzin a écrit : > >> [hidden email] 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/a
|
Administrator
|
In reply to this post by Suraj Khurana-2
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 <[hidden email]> > 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/a
|
Administrator
|
In reply to this post by Mathieu Lirzin
r1867927 is the answer
Jacques Le 03/10/2019 à 12:10, Mathieu Lirzin a écrit : > Hello Jacques, > > Jacques Le Roux <[hidden email]> writes: > >> Le 02/10/2019 à 17:21, Mathieu Lirzin a écrit : >> >>> [hidden email] 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/a
|
Administrator
|
In reply to this post by Suraj Khurana-2
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 <[hidden email]> > 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/a
|
Administrator
|
In reply to this post by Jacques Le Roux
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 <[hidden email]> writes: >> >>> Le 02/10/2019 à 17:21, Mathieu Lirzin a écrit : >>> >>>> [hidden email] 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/a
|
Administrator
|
In reply to this post by Jacques Le Roux
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 <[hidden email]> >> 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/a
|
|
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 <[hidden email]> 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 < > [hidden email]> > >> 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/a
|
Administrator
|
In reply to this post by Jacques Le Roux
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 <[hidden email]> writes: >>> >>>> Le 02/10/2019 à 17:21, Mathieu Lirzin a écrit : >>>> >>>>> [hidden email] 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. >>> >> > |
Providing utilitaries for integration tests (was: svn commit: r1867889 …)
|
|
Hello Jacques,
Jacques Le Roux <[hidden email]> 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: Providing utilitaries for integration tests
|
Administrator
|
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: '[hidden email]', ] 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: '[hidden email]', 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 <[hidden email]> 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 |
Re: Providing utilitaries for integration tests
|
Administrator
|
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: '[hidden email]', > ] > 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: '[hidden email]', > 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 <[hidden email]> 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
|
|
Hello Jacques,
Jacques Le Roux <[hidden email]> 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: '[hidden email]', >> ] >> 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: '[hidden email]', >> 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: '[hidden email]', 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 utilities for integration tests
|
Administrator
|
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: '[hidden email]', >>> ] >>> 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: '[hidden email]', >>> 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: '[hidden email]', > 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 |
«
Return to OFBiz - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |