Re: Review Request 52911: OOZIE-2685 OYA: Test new LauncherAM

[András Piros]

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


Ship it!




Ship It!

- András Piros


On Oct. 17, 2016, 1:49 p.m., Peter Bacsko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52911/
> ---
> 
> (Updated Oct. 17, 2016, 1:49 p.m.)
> 
> 
> Review request for oozie, András Piros, Attila Sasvari, Peter Cseh, and 
> Robert Kanter.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> New unit tests for LauncherAM class.
> 
> These tests already provide good coverage of the class (around ~80%). Some 
> code paths are missing but the most important scenarios are covered.
> 
> 
> Diffs
> -
> 
>   pom.xml ef22b39 
>   sharelib/oozie/pom.xml 3ea10a5 
>   
> sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/LauncherAMTestMainClass.java
>  PRE-CREATION 
>   
> sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestHdfsOperations.java
>  PRE-CREATION 
>   
> sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52911/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Peter Bacsko
> 
>

Re: Review Request 52911: OOZIE-2685 OYA: Test new LauncherAM

[Peter Bacsko]

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




pom.xml (line 273)


Had to change this because of Hamcrest



sharelib/oozie/pom.xml (line 66)


mockito-all contains hamcrest as well, so using core only to avoid class 
loading problems


- Peter Bacsko


On okt. 17, 2016, 1:49 du, Peter Bacsko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52911/
> ---
> 
> (Updated okt. 17, 2016, 1:49 du)
> 
> 
> Review request for oozie, András Piros, Attila Sasvari, Peter Cseh, and 
> Robert Kanter.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> New unit tests for LauncherAM class.
> 
> These tests already provide good coverage of the class (around ~80%). Some 
> code paths are missing but the most important scenarios are covered.
> 
> 
> Diffs
> -
> 
>   pom.xml ef22b39 
>   sharelib/oozie/pom.xml 3ea10a5 
>   
> sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/LauncherAMTestMainClass.java
>  PRE-CREATION 
>   
> sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestHdfsOperations.java
>  PRE-CREATION 
>   
> sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52911/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Peter Bacsko
> 
>

Re: Review Request 52911: OOZIE-2685 OYA: Test new LauncherAM

[Peter Bacsko]

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

(Updated okt. 17, 2016, 1:49 du)


Review request for oozie, András Piros, Attila Sasvari, Peter Cseh, and Robert 
Kanter.


Changes
---

Addressing comments by Andras


Repository: oozie-git


Description
---

New unit tests for LauncherAM class.

These tests already provide good coverage of the class (around ~80%). Some code 
paths are missing but the most important scenarios are covered.


Diffs (updated)
-

  pom.xml ef22b39 
  sharelib/oozie/pom.xml 3ea10a5 
  
sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/LauncherAMTestMainClass.java
 PRE-CREATION 
  
sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestHdfsOperations.java
 PRE-CREATION 
  
sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java 
PRE-CREATION 

Diff: https://reviews.apache.org/r/52911/diff/


Testing
---


Thanks,

Peter Bacsko

Re: Review Request 52911: OOZIE-2685 OYA: Test new LauncherAM

[András Piros]

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




sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestHdfsOperations.java
 (line 65)


U"rkorszak :D



sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestHdfsOperations.java
 (lines 103 - 114)


Nagyon tetszik a BDD megkozelites! Mindenesetre a @Before-t raraknam, es az 
osszes @Test method-bol kivennem.



sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java 
(lines 206 - 207)


Either rename `e` to `expected`, or use `@Rule ExpectedException`, but drop 
that comment in any case.



sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java 
(line 211)


Not needed w/ `@Rule ExpectedException`.



sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java 
(lines 224 - 225)


Either rename `e` to `expected`, or use `@Rule ExpectedException`, but drop 
that comment in any case.



sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java 
(line 229)


Not needed w/ `@Rule ExpectedException`.



sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java 
(lines 314 - 315)


Either rename `e` to `expected`, or use `@Rule ExpectedException`, but drop 
the comment in any case.



sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java 
(line 319)


Not needed w/ `@Rule ExpectedException`.



sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java 
(lines 334 - 335)


Either rename `e` to `expected`, or use `@Rule ExpectedException`, but drop 
the comment in any case.



sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java 
(line 339)


Not needed w/ `@Rule ExpectedException`.



sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java 
(line 341)



http://junit.sourceforge.net/javadoc/org/junit/Assert.html#assertFalse(java.lang.String,
 boolean)



sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java 
(lines 346 - 347)


Let's call it `configureMocksForHappyPath()` and drop the comment line, 
then.



sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java 
(line 369)


I'd extract method and call 4 times.



sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java 
(line 388)


Either use `OozieActionResult` instead of `boolean`, or use always 
variables where this method is called like that:

`final boolean background = true;
assertSuccessfulExecution(background);`

or

`final OozieActionResult resultOnNotification = OozieActionResult.RUNNING;
assertSuccessfulExecution(resultOnNotification);`



sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java 
(line 401)


This can also be put simpler if you go for using `OozieActionResult` 
instead of `boolean`.



sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java 
(line 405)


Rename to `assertActionOutputDataPresentAndCorrect()`.



sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java 
(lines 412 - 415)


Use instead>


http://junit.sourceforge.net/javadoc/org/junit/Assert.html#assertEquals(java.lang.String,
 java.lang.Object, java.lang.Object)



sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java 
(line 418)


Rename to `assertActionOutputDataNotPresent()`.



sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java 
(lines 425 - 428)


Instead:


http://junit.sourceforge.net/javadoc/org/junit/Assert.html#assertNull(java.lang.String,
 java.lang.Object)



sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM

Review Request 52911: OOZIE-2685 OYA: Test new LauncherAM

[Peter Bacsko]

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

Review request for oozie, András Piros, Attila Sasvari, Peter Cseh, and Robert 
Kanter.


Repository: oozie-git


Description
---

New unit tests for LauncherAM class.

These tests already provide good coverage of the class (around ~80%). Some code 
paths are missing but the most important scenarios are covered.


Diffs
-

  
sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/LauncherAMTestMainClass.java
 PRE-CREATION 
  
sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestHdfsOperations.java
 PRE-CREATION 
  
sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java 
PRE-CREATION 

Diff: https://reviews.apache.org/r/52911/diff/


Testing
---


Thanks,

Peter Bacsko