> On Jan. 24, 2017, 6:14 p.m., András Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java,
> >  lines 591-606
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line591>
> >
> >     For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >     
> >     ```
> >     @Rule
> >     public ExpectedException expected = ExpectedException.none();
> >     ...
> >     expected.expect(MyOoziException.class);
> >     expected.expectMessage(myExceptionMessageString);
> >     performAndThrow();
> >     ```

The exception that we need to check is a **wrapped exception**. We can check 
the exception with expected.expectCause from junit 4.11; but we cannot check 
the exception message string. Dropping this.


> On Jan. 24, 2017, 6:14 p.m., András Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java,
> >  lines 610-626
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line610>
> >
> >     For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >     
> >     ```
> >     @Rule
> >     public ExpectedException expected = ExpectedException.none();
> >     ...
> >     expected.expect(MyOoziException.class);
> >     expected.expectMessage(myExceptionMessageString);
> >     performAndThrow();
> >     ```

Here I can use ExpectedException since exception is not wrapped. But as i 
mentioned in my earlier comments. ExpectedException is going to be removed from 
junit sooner. So I would rather stick to the try-catch.


> On Jan. 24, 2017, 6:14 p.m., András Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java,
> >  lines 683-697
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line683>
> >
> >     For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >     
> >     ```
> >     @Rule
> >     public ExpectedException expected = ExpectedException.none();
> >     ...
> >     expected.expect(MyOoziException.class);
> >     expected.expectMessage(myExceptionMessageString);
> >     performAndThrow();
> >     ```

The exception that we need to check is a **wrapped exception**. We can check 
the exception with expected.expectCause from junit 4.11; but we cannot check 
the exception message string. Dropping this.


> On Jan. 24, 2017, 6:14 p.m., András Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java,
> >  lines 702-717
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line702>
> >
> >     For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >     
> >     ```
> >     @Rule
> >     public ExpectedException expected = ExpectedException.none();
> >     ...
> >     expected.expect(MyOoziException.class);
> >     expected.expectMessage(myExceptionMessageString);
> >     performAndThrow();
> >     ```

The exception that we need to check is a **wrapped exception**. We can check 
the exception with expected.expectCause from junit 4.11; but we cannot check 
the exception message string. Dropping this.


> On Jan. 24, 2017, 6:14 p.m., András Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java,
> >  lines 721-736
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line721>
> >
> >     For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >     
> >     ```
> >     @Rule
> >     public ExpectedException expected = ExpectedException.none();
> >     ...
> >     expected.expect(MyOoziException.class);
> >     expected.expectMessage(myExceptionMessageString);
> >     performAndThrow();
> >     ```

Here I can use ExpectedException since exception is not wrapped. But as i 
mentioned in my earlier comments. ExpectedException is going to be removed from 
junit sooner. So I would rather stick to the try-catch.


> On Jan. 24, 2017, 6:14 p.m., András Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java,
> >  lines 802-817
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line802>
> >
> >     For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >     
> >     ```
> >     @Rule
> >     public ExpectedException expected = ExpectedException.none();
> >     ...
> >     expected.expect(MyOoziException.class);
> >     expected.expectMessage(myExceptionMessageString);
> >     performAndThrow();
> >     ```

The exception that we need to check is a **wrapped exception**. We can check 
the exception with expected.expectCause from junit 4.11; but we cannot check 
the exception message string. Dropping this.


> On Jan. 24, 2017, 6:14 p.m., András Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java,
> >  lines 838-854
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line838>
> >
> >     For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >     
> >     ```
> >     @Rule
> >     public ExpectedException expected = ExpectedException.none();
> >     ...
> >     expected.expect(MyOoziException.class);
> >     expected.expectMessage(myExceptionMessageString);
> >     performAndThrow();
> >     ```

Here I can use ExpectedException since exception is not wrapped. But as i 
mentioned in my earlier comments. ExpectedException is going to be removed from 
junit sooner. So I would rather stick to the try-catch.


> On Jan. 24, 2017, 6:14 p.m., András Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java,
> >  lines 572-586
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line572>
> >
> >     For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >     
> >     ```
> >     @Rule
> >     public ExpectedException expected = ExpectedException.none();
> >     ...
> >     expected.expect(MyOoziException.class);
> >     expected.expectMessage(myExceptionMessageString);
> >     performAndThrow();
> >     ```

The exception that we need to check is a **wrapped exception**. We can check 
the exception with expected.expectCause from junit 4.11; but we cannot check 
the exception message string. Dropping this.


- Satish


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


On Jan. 24, 2017, 2:49 p.m., Satish Saley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51029/
> -----------------------------------------------------------
> 
> (Updated Jan. 24, 2017, 2:49 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2630
>     https://issues.apache.org/jira/browse/OOZIE-2630
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> Oozie Coordinator EL Functions to get first day of the week/month
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 
> 0af7edc 
>   core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 
> 969336d 
>   core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 925a7aa 
>   core/src/main/java/org/apache/oozie/coord/TimeUnit.java 5b37639 
>   core/src/main/java/org/apache/oozie/util/DateUtils.java 3caf0a2 
>   core/src/main/resources/oozie-default.xml ad10386 
>   
> core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 
> 7062e69 
>   
> core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java
>  29e7ca1 
>   core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java be60133 
>   core/src/test/resources/coord-dataset-endOfDays.xml PRE-CREATION 
>   core/src/test/resources/coord-dataset-endOfMonths.xml PRE-CREATION 
>   core/src/test/resources/coord-dataset-endOfWeeks.xml PRE-CREATION 
>   docs/src/site/twiki/CoordinatorFunctionalSpec.twiki e3ac514 
> 
> Diff: https://reviews.apache.org/r/51029/diff/
> 
> 
> Testing
> -------
> 
> Tested locally
> 
> 
> Thanks,
> 
> Satish Saley
> 
>

Reply via email to