[DISCUSSION] Committing Minilang test patches under OFBIZ-1463

[Jacques Le Roux]

Hi All,

We started a discussion in OFBIZ-1463 about committing or not the Minilang test patches.

There are already few mixed opinions there (Michael, Aditya, Suraj and I).

Before voting I'd like to know if we can come to a consensus.

Please read in OFBIZ-1463 and come back with your opinion.

I have just changed mine because I believe using the tests as soon they are reading is a good thing.

Waiting would be a waste of not only work done but also time for code safety. We can still move them to Groovy later, it's not more work, I guess it's
even less.

Jacques

[Mathieu Lirzin]

Hello Jacques,

Jacques Le Roux <[hidden email]> writes:

> We started a discussion in OFBIZ-1463 about committing or not the Minilang test patches.
>
> There are already few mixed opinions there (Michael, Aditya, Suraj and I).
>
> Before voting I'd like to know if we can come to a consensus.
>
> Please read in OFBIZ-1463 and come back with your opinion.
>
> I have just changed mine because I believe using the tests as soon they are reading is a good thing.
>
> Waiting would be a waste of not only work done but also time for code
> safety. We can still move them to Groovy later, it's not more work, I
> guess it's even less.

If it's less work, then why people interested in having those patches
applied has not migrate those tests yet? :-)

Having contributed to the migration of the “quoteTests” I know for a
fact that this is painful work. Applying those patches in their current
state would simply mean putting the burden on someone else to do the
work. So I am strongly opposed to committing thoses patches before the
tests are migrated.

As far as I am concerned, if people want stuff to be committed then they
have to do their homework first.

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

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Jacques Le Roux]

Le 02/06/2019 à 15:50, Mathieu Lirzin a écrit :

> Hello Jacques,
>
> Jacques Le Roux <[hidden email]> writes:
>
>> We started a discussion in OFBIZ-1463 about committing or not the Minilang test patches.
>>
>> There are already few mixed opinions there (Michael, Aditya, Suraj and I).
>>
>> Before voting I'd like to know if we can come to a consensus.
>>
>> Please read in OFBIZ-1463 and come back with your opinion.
>>
>> I have just changed mine because I believe using the tests as soon they are reading is a good thing.
>>
>> Waiting would be a waste of not only work done but also time for code
>> safety. We can still move them to Groovy later, it's not more work, I
>> guess it's even less.
> If it's less work, then why people interested in having those patches
> applied has not migrate those tests yet? :-)
>
> Having contributed to the migration of the “quoteTests” I know for a
> fact that this is painful work. Applying those patches in their current
> state would simply mean putting the burden on someone else to do the
> work. So I am strongly opposed to committing thoses patches before the
> tests are migrated.
>
> As far as I am concerned, if people want stuff to be committed then they
> have to do their homework first.

Wait Mathieu,

Could you explain why it would me more work to migrate from a patch than to migrate from code already in the repo?

Maybe you mean that migrating is a burden anyway, and it's better to directly write test in Groovy?

Then I see no problems doing that and having already Minilang tests present. We "just have" to drop Minilang tests when Groovy ones are ready.

What I'm missing?

Thanks

--
Jacques

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Michael Brohl-3]

-1 to introduce more minilang code to the codebase. New code should be provided in either Java or Groovy code.

Thanks,
Michael

> Am 02.06.2019 um 12:56 schrieb Jacques Le Roux <[hidden email]>:
>
> Hi All,
>
> We started a discussion in OFBIZ-1463 about committing or not the Minilang test patches.
>
> There are already few mixed opinions there (Michael, Aditya, Suraj and I).
>
> Before voting I'd like to know if we can come to a consensus.
>
> Please read in OFBIZ-1463 and come back with your opinion.
>
> I have just changed mine because I believe using the tests as soon they are reading is a good thing.
>
> Waiting would be a waste of not only work done but also time for code safety. We can still move them to Groovy later, it's not more work, I guess it's
> even less.
>
> Jacques
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Jacques Le Roux]

OK if this is a veto, no need to continue the discussion.-

Else could you explain your POV Michael, notably about missing to put in some new tests that could be helpful in the meantime?

Thanks

Le 02/06/2019 à 21:27, Michael Brohl a écrit :

> -1 to introduce more minilang code to the codebase. New code should be provided in either Java or Groovy code.
>
> Thanks,
> Michael
>
>
>
>> Am 02.06.2019 um 12:56 schrieb Jacques Le Roux <[hidden email]>:
>>
>> Hi All,
>>
>> We started a discussion in OFBIZ-1463 about committing or not the Minilang test patches.
>>
>> There are already few mixed opinions there (Michael, Aditya, Suraj and I).
>>
>> Before voting I'd like to know if we can come to a consensus.
>>
>> Please read in OFBIZ-1463 and come back with your opinion.
>>
>> I have just changed mine because I believe using the tests as soon they are reading is a good thing.
>>
>> Waiting would be a waste of not only work done but also time for code safety. We can still move them to Groovy later, it's not more work, I guess it's
>> even less.
>>
>> Jacques
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Michael Brohl-3]

I explained my POV in the Jira [1].

Why not encourage the contributors to move their minilang tests to
Groovy code? I can see that this has already been done, e.g. here [2]
(thanks everyone involved!).

I'm sure that the remaining patches will get converted soon, no need to
choose the "easy way" and commit the minilang versions.

If we will allow more minilang commits, we will always have the
discussion and won't ever get rid of it.

Thanks,

Michael

[1] https://issues.apache.org/jira/projects/OFBIZ/issues/OFBIZ-1463

[2] https://issues.apache.org/jira/browse/OFBIZ-9002



Am 03.06.19 um 13:21 schrieb Jacques Le Roux:

> OK if this is a veto, no need to continue the discussion.-
>
> Else could you explain your POV Michael, notably about missing to put
> in some new tests that could be helpful in the meantime?
>
> Thanks
>
> Le 02/06/2019 à 21:27, Michael Brohl a écrit :
>> -1 to introduce more minilang code to the codebase. New code should
>> be provided in either Java or Groovy code.
>>
>> Thanks,
>> Michael
>>
>>
>>
>>> Am 02.06.2019 um 12:56 schrieb Jacques Le Roux
>>> <[hidden email]>:
>>>
>>> Hi All,
>>>
>>> We started a discussion in OFBIZ-1463 about committing or not the
>>> Minilang test patches.
>>>
>>> There are already few mixed opinions there (Michael, Aditya, Suraj
>>> and I).
>>>
>>> Before voting I'd like to know if we can come to a consensus.
>>>
>>> Please read in OFBIZ-1463 and come back with your opinion.
>>>
>>> I have just changed mine because I believe using the tests as soon
>>> they are reading is a good thing.
>>>
>>> Waiting would be a waste of not only work done but also time for
>>> code safety. We can still move them to Groovy later, it's not more
>>> work, I guess it's
>>> even less.
>>>
>>> Jacques
>>>
>>

[taher]

As a general rule, minilang adds to the technical debt of this project. It
is hard to understand or maintain even simple constructs in minilang.

To generalize this concept... Patch != Good patch. So I recommend that
everything goes through the funnel of good old reviews. Good code has no
shortcuts, it is blood sweat and tears both for the provider and the
reviewer of patches.

On Mon, Jun 3, 2019, 4:20 PM Michael Brohl <[hidden email]> wrote:

> I explained my POV in the Jira [1].
>
> Why not encourage the contributors to move their minilang tests to
> Groovy code? I can see that this has already been done, e.g. here [2]
> (thanks everyone involved!).
>
> I'm sure that the remaining patches will get converted soon, no need to
> choose the "easy way" and commit the minilang versions.
>
> If we will allow more minilang commits, we will always have the
> discussion and won't ever get rid of it.
>
> Thanks,
>
> Michael
>
> [1] https://issues.apache.org/jira/projects/OFBIZ/issues/OFBIZ-1463
>
> [2] https://issues.apache.org/jira/browse/OFBIZ-9002
>
>
>
> Am 03.06.19 um 13:21 schrieb Jacques Le Roux:
> > OK if this is a veto, no need to continue the discussion.-
> >
> > Else could you explain your POV Michael, notably about missing to put
> > in some new tests that could be helpful in the meantime?
> >
> > Thanks
> >
> > Le 02/06/2019 à 21:27, Michael Brohl a écrit :
> >> -1 to introduce more minilang code to the codebase. New code should
> >> be provided in either Java or Groovy code.
> >>
> >> Thanks,
> >> Michael
> >>
> >>
> >>
> >>> Am 02.06.2019 um 12:56 schrieb Jacques Le Roux
> >>> <[hidden email]>:
> >>>
> >>> Hi All,
> >>>
> >>> We started a discussion in OFBIZ-1463 about committing or not the
> >>> Minilang test patches.
> >>>
> >>> There are already few mixed opinions there (Michael, Aditya, Suraj
> >>> and I).
> >>>
> >>> Before voting I'd like to know if we can come to a consensus.
> >>>
> >>> Please read in OFBIZ-1463 and come back with your opinion.
> >>>
> >>> I have just changed mine because I believe using the tests as soon
> >>> they are reading is a good thing.
> >>>
> >>> Waiting would be a waste of not only work done but also time for
> >>> code safety. We can still move them to Groovy later, it's not more
> >>> work, I guess it's
> >>> even less.
> >>>
> >>> Jacques
> >>>
> >>
>
>

[Jacques Le Roux]

Le 03/06/2019 à 15:20, Michael Brohl a écrit :
> I explained my POV in the Jira [1].
>
> Why not encourage the contributors to move their minilang tests to Groovy code? I can see that this has already been done, e.g. here [2] (thanks
> everyone involved!).
>
> I'm sure that the remaining patches will get converted soon, no need to choose the "easy way" and commit the minilang versions.
I hope you are right for now I don't see a lot of such cases, but it's hard to track
>
> If we will allow more minilang commits, we will always have the discussion and won't ever get rid of it.

That's a possible issue indeed

Jacques

>
> Thanks,
>
> Michael
>
> [1] https://issues.apache.org/jira/projects/OFBIZ/issues/OFBIZ-1463
>
> [2] https://issues.apache.org/jira/browse/OFBIZ-9002
>
>
>
> Am 03.06.19 um 13:21 schrieb Jacques Le Roux:
>> OK if this is a veto, no need to continue the discussion.-
>>
>> Else could you explain your POV Michael, notably about missing to put in some new tests that could be helpful in the meantime?
>>
>> Thanks
>>
>> Le 02/06/2019 à 21:27, Michael Brohl a écrit :
>>> -1 to introduce more minilang code to the codebase. New code should be provided in either Java or Groovy code.
>>>
>>> Thanks,
>>> Michael
>>>
>>>
>>>
>>>> Am 02.06.2019 um 12:56 schrieb Jacques Le Roux <[hidden email]>:
>>>>
>>>> Hi All,
>>>>
>>>> We started a discussion in OFBIZ-1463 about committing or not the Minilang test patches.
>>>>
>>>> There are already few mixed opinions there (Michael, Aditya, Suraj and I).
>>>>
>>>> Before voting I'd like to know if we can come to a consensus.
>>>>
>>>> Please read in OFBIZ-1463 and come back with your opinion.
>>>>
>>>> I have just changed mine because I believe using the tests as soon they are reading is a good thing.
>>>>
>>>> Waiting would be a waste of not only work done but also time for code safety. We can still move them to Groovy later, it's not more work, I guess
>>>> it's
>>>> even less.
>>>>
>>>> Jacques
>>>>
>>>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Mathieu Lirzin]

Hello

Jacques Le Roux <[hidden email]> writes:

> Le 02/06/2019 à 15:50, Mathieu Lirzin a écrit :
>>
>> Jacques Le Roux <[hidden email]> writes:
>>
>>> We started a discussion in OFBIZ-1463 about committing or not the Minilang test patches.
>>>
>>> There are already few mixed opinions there (Michael, Aditya, Suraj and I).
>>>
>>> Before voting I'd like to know if we can come to a consensus.
>>>
>>> Please read in OFBIZ-1463 and come back with your opinion.
>>>
>>> I have just changed mine because I believe using the tests as soon they are reading is a good thing.
>>>
>>> Waiting would be a waste of not only work done but also time for code
>>> safety. We can still move them to Groovy later, it's not more work, I
>>> guess it's even less.
>> If it's less work, then why people interested in having those patches
>> applied has not migrate those tests yet? :-)
>>
>> Having contributed to the migration of the “quoteTests” I know for a
>> fact that this is painful work. Applying those patches in their current
>> state would simply mean putting the burden on someone else to do the
>> work. So I am strongly opposed to committing thoses patches before the
>> tests are migrated.
>>
>> As far as I am concerned, if people want stuff to be committed then they
>> have to do their homework first.
>
> Wait Mathieu,
>
> Could you explain why it would me more work to migrate from a patch than to migrate from code already in the repo?

I was not saying either solution is more work, I was just acknowledging
that right now, nobody wants to do the work of migration for those
tests.

> Maybe you mean that migrating is a burden anyway, and it's better to
> directly write test in Groovy?

Yes I expect people having proposed the patches to update their patches
with the groovy migration before applying those patches.

> Then I see no problems doing that and having already Minilang tests
> present. We "just have" to drop Minilang tests when Groovy ones are
> ready.
>
> What I'm missing?

The problem I see is that if the people proposing the patches are not
willing to do the migration work right now, I see no reason why they
would want to do it afterwards when there will be no incentives to do
so.

If we apply those patches as they are, then the rest of us will
eventually have to understand/maintain/debug them when they fail. Given
the current sad state of OFBiz integration tests we cannot afford to let
more “stuff to be cleaned up later” enter the codebase.

To explain my hard feeling regarding OFBiz integration tests. I find
them really hard to understand/debug due to the following points:

- Logs are unreadable! I mean understanding which test has failed is
  already an endeavour.

- The global nature of the data loaded during the tests makes things
  brittle. We saw a lot of that in previous weeks where tests were
  succeding when having both the framework and the plugins but failing
  when having only the framework.

- People tend to be confused between integration and unit tests,
  resulting in integration tests written in an atomic way which is not a
  good thing. For example for testing services manipulating an
  hypothetical entity ‘foo’, we would have a ‘testCreateFooService’ that
  adds some stuff in the database and ‘testUpdateFooService’ that
  retrieve the stuff added by the other.  This is bad since it
  introduces a order dependency between those tests.  Instead of small
  integration tests we should have bigger independant ones following a
  scenario with multiple asserts.

Sorry for being long but I wanted to make clear that this is a real
issue.

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

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Jacques Le Roux]

Hi Mathieu,

[snip]
>> Then I see no problems doing that and having already Minilang tests
>> present. We "just have" to drop Minilang tests when Groovy ones are
>> ready.
>>
>> What I'm missing?
> The problem I see is that if the people proposing the patches are not
> willing to do the migration work right now, I see no reason why they
> would want to do it afterwards when there will be no incentives to do
> so.

Could be indeed


> If we apply those patches as they are, then the rest of us will
> eventually have to understand/maintain/debug them when they fail. Given
> the current sad state of OFBiz integration tests we cannot afford to let
> more “stuff to be cleaned up later” enter the codebase.

I see your point, you kinda expect people having created Minilang test patches to convert them themselves.  Not sure that will we work.
If I get empathic (something I easily do when reviewing) I can understand the frustration of those people.
They create a patch before the decision is made and bam, it's no good :/
So it does not help to "put the blame" (OK I emphasise a bit here ;)) on them
Also we can't even rely on an expectation they will do the work. Some people come and go...
Anyway at some point someone will have to do it
And at least, I believe it's better to inspire from those patches than to start anew


> To explain my hard feeling regarding OFBiz integration tests. I find
> them really hard to understand/debug due to the following points:
>
> - Logs are unreadable! I mean understanding which test has failed is
>    already an endeavour.

Oh that! I never look at integration test logs, it's impossible indeed.
I simply look at the result of the tests where the error logs are.

> - The global nature of the data loaded during the tests makes things
>    brittle. We saw a lot of that in previous weeks where tests were
>    succeding when having both the framework and the plugins but failing
>    when having only the framework.
>
> - People tend to be confused between integration and unit tests,
>    resulting in integration tests written in an atomic way which is not a
>    good thing. For example for testing services manipulating an
>    hypothetical entity ‘foo’, we would have a ‘testCreateFooService’ that
>    adds some stuff in the database and ‘testUpdateFooService’ that
>    retrieve the stuff added by the other.  This is bad since it
>    introduces a order dependency between those tests.  Instead of small
>    integration tests we should have bigger independant ones following a
>    scenario with multiple asserts.

Makes totally sense, unfortunately with 1300+ tests [1] I don't expect that to happen soon

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

> Sorry for being long but I wanted to make clear that this is a real
> issue.

Thanks for expression your opinion, that helps!

After 3 days, only people against expressed their opinions, so I think the discussion is closed.

Jacques


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Jacques Le Roux]

Le 05/06/2019 à 09:24, Jacques Le Roux a écrit :
>> To explain my hard feeling regarding OFBiz integration tests. I find
>> them really hard to understand/debug due to the following points:
>>
>> - Logs are unreadable! I mean understanding which test has failed is
>>    already an endeavour.
>
> Oh that! I never look at integration test logs, it's impossible indeed.
> I simply look at the result of the tests where the error logs are.

BTW, thinking about it, I agree it does not help to fix wrong tests.

Not sure how to do that, but by running suspected culprits one by one

Jacques

[Suraj Khurana-2]

Hello everyone,

I am inclined with Jacques opinion (Initially what we proposed to commit
those patches into the repo) here, on the flip side I would like to mention
that we are gradually working on moving these XML's to groovy.
So whatever we conclude, I am up for it.

--
Best Regards,
Suraj Khurana
Technical Consultant
HotWax Systems







On Wed, Jun 5, 2019 at 8:18 PM Jacques Le Roux <[hidden email]>
wrote:

> Le 05/06/2019 à 09:24, Jacques Le Roux a écrit :
> >> To explain my hard feeling regarding OFBiz integration tests. I find
> >> them really hard to understand/debug due to the following points:
> >>
> >> - Logs are unreadable! I mean understanding which test has failed is
> >>    already an endeavour.
> >
> > Oh that! I never look at integration test logs, it's impossible indeed.
> > I simply look at the result of the tests where the error logs are.
>
> BTW, thinking about it, I agree it does not help to fix wrong tests.
>
> Not sure how to do that, but by running suspected culprits one by one
>
> Jacques
>
>