You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Andras Salamon <an...@melda.info> on 2018/08/17 13:36:33 UTC

Review Request 68411: OOZIE-3264 - Flaky test TestCoordMaterializeTransitionXCommand#testLastOnlyMaterialization

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68411/
-----------------------------------------------------------

Review request for oozie, András Piros, Kinga Marton, and Peter Bacsko.


Repository: oozie-git


Description
-------

OOZIE-3264 - Flaky test TestCoordMaterializeTransitionXCommand#testLastOnlyMaterialization


Diffs
-----

  core/src/test/java/org/apache/oozie/command/coord/TestCoordChangeXCommand.java b784093b 
  core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 8189732e 
  core/src/test/java/org/apache/oozie/test/XDataTestCase.java a8fe7037 


Diff: https://reviews.apache.org/r/68411/diff/1/


Testing
-------

I was using grind. Before the modification it was failing 2 times out of 50 executions, after the patch it works 50/50.


Thanks,

Andras Salamon


Re: Review Request 68411: OOZIE-3264 - Flaky test TestCoordMaterializeTransitionXCommand#testLastOnlyMaterialization

Posted by Andras Salamon <an...@melda.info>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68411/
-----------------------------------------------------------

(Updated Aug. 22, 2018, 3:58 p.m.)


Review request for oozie, András Piros, Kinga Marton, and Peter Bacsko.


Changes
-------

Eliminating local services variable, moving a method to a new test class.


Repository: oozie-git


Description
-------

OOZIE-3264 - Flaky test TestCoordMaterializeTransitionXCommand#testLastOnlyMaterialization


Diffs (updated)
-----

  core/src/test/java/org/apache/oozie/command/coord/TestCoordChangeXCommand.java b784093b 
  core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 8189732e 
  core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommandWithRunningServices.java PRE-CREATION 
  core/src/test/java/org/apache/oozie/test/XDataTestCase.java a8fe7037 


Diff: https://reviews.apache.org/r/68411/diff/2/

Changes: https://reviews.apache.org/r/68411/diff/1-2/


Testing
-------

I was using grind. Before the modification it was failing 2 times out of 50 executions, after the patch it works 50/50.


Thanks,

Andras Salamon


Re: Review Request 68411: OOZIE-3264 - Flaky test TestCoordMaterializeTransitionXCommand#testLastOnlyMaterialization

Posted by Andras Salamon <an...@melda.info>.

> On Aug. 22, 2018, 1:09 p.m., András Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java
> > Line 67 (original), 66 (patched)
> > <https://reviews.apache.org/r/68411/diff/1/?file=2074591#file2074591line68>
> >
> >     `XTestCase` has its own `Services` field. Why do we need one more here?

There is no field in XTestCase which could be used here, but Services.get() can be used instead of the local variable.


> On Aug. 22, 2018, 1:09 p.m., András Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java
> > Lines 540-542 (patched)
> > <https://reviews.apache.org/r/68411/diff/1/?file=2074591#file2074591line542>
> >
> >     Why do we need this at all? Why not rely on `tearDown()` instead?

This method requires real CallableQueueService instead of the fake one. This code was destroying the fake services and initializing the real services.

Moved this method to a new class (TestCoordMaterializeTransitionXCommandWithRunningServices) where special handling is no longer necessary.


- Andras


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68411/#review207745
-----------------------------------------------------------


On Aug. 17, 2018, 1:36 p.m., Andras Salamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68411/
> -----------------------------------------------------------
> 
> (Updated Aug. 17, 2018, 1:36 p.m.)
> 
> 
> Review request for oozie, András Piros, Kinga Marton, and Peter Bacsko.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> OOZIE-3264 - Flaky test TestCoordMaterializeTransitionXCommand#testLastOnlyMaterialization
> 
> 
> Diffs
> -----
> 
>   core/src/test/java/org/apache/oozie/command/coord/TestCoordChangeXCommand.java b784093b 
>   core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 8189732e 
>   core/src/test/java/org/apache/oozie/test/XDataTestCase.java a8fe7037 
> 
> 
> Diff: https://reviews.apache.org/r/68411/diff/1/
> 
> 
> Testing
> -------
> 
> I was using grind. Before the modification it was failing 2 times out of 50 executions, after the patch it works 50/50.
> 
> 
> Thanks,
> 
> Andras Salamon
> 
>


Re: Review Request 68411: OOZIE-3264 - Flaky test TestCoordMaterializeTransitionXCommand#testLastOnlyMaterialization

Posted by András Piros via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68411/#review207745
-----------------------------------------------------------




core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java
Line 67 (original), 66 (patched)
<https://reviews.apache.org/r/68411/#comment291250>

    `XTestCase` has its own `Services` field. Why do we need one more here?



core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java
Lines 540-542 (patched)
<https://reviews.apache.org/r/68411/#comment291249>

    Why do we need this at all? Why not rely on `tearDown()` instead?


- András Piros


On Aug. 17, 2018, 1:36 p.m., Andras Salamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68411/
> -----------------------------------------------------------
> 
> (Updated Aug. 17, 2018, 1:36 p.m.)
> 
> 
> Review request for oozie, András Piros, Kinga Marton, and Peter Bacsko.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> OOZIE-3264 - Flaky test TestCoordMaterializeTransitionXCommand#testLastOnlyMaterialization
> 
> 
> Diffs
> -----
> 
>   core/src/test/java/org/apache/oozie/command/coord/TestCoordChangeXCommand.java b784093b 
>   core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 8189732e 
>   core/src/test/java/org/apache/oozie/test/XDataTestCase.java a8fe7037 
> 
> 
> Diff: https://reviews.apache.org/r/68411/diff/1/
> 
> 
> Testing
> -------
> 
> I was using grind. Before the modification it was failing 2 times out of 50 executions, after the patch it works 50/50.
> 
> 
> Thanks,
> 
> Andras Salamon
> 
>