> On Jan. 23, 2017, 11:47 a.m., András Piros wrote: > > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, > > lines 582-585 > > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line582> > > > > Use JUnit's `ExpectedException` instead: > > > > https://github.com/junit-team/junit4/wiki/exception-testing
I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/ > On Jan. 23, 2017, 11:47 a.m., András Piros wrote: > > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, > > lines 598-601 > > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line598> > > > > Use JUnit's `ExpectedException` instead: > > > > https://github.com/junit-team/junit4/wiki/exception-testing I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/ > On Jan. 23, 2017, 11:47 a.m., András Piros wrote: > > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, > > lines 681-684 > > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line681> > > > > Use JUnit's `ExpectedException` instead: > > > > https://github.com/junit-team/junit4/wiki/exception-testing I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/ > On Jan. 23, 2017, 11:47 a.m., András Piros wrote: > > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, > > lines 697-700 > > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line697> > > > > Use JUnit's `ExpectedException` instead: > > > > https://github.com/junit-team/junit4/wiki/exception-testing I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/ > On Jan. 23, 2017, 11:47 a.m., András Piros wrote: > > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, > > line 712 > > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line712> > > > > Use JUnit's `ExpectedException` instead: > > > > https://github.com/junit-team/junit4/wiki/exception-testing I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/ > On Jan. 23, 2017, 11:47 a.m., András Piros wrote: > > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, > > lines 714-717 > > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line714> > > > > Use JUnit's `ExpectedException` instead: > > > > https://github.com/junit-team/junit4/wiki/exception-testing I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/ > On Jan. 23, 2017, 11:47 a.m., András Piros wrote: > > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, > > line 781 > > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line781> > > > > Use JUnit's `ExpectedException` instead: > > > > https://github.com/junit-team/junit4/wiki/exception-testing I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/ > On Jan. 23, 2017, 11:47 a.m., András Piros wrote: > > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, > > lines 783-786 > > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line783> > > > > Use JUnit's `ExpectedException` instead: > > > > https://github.com/junit-team/junit4/wiki/exception-testing I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/ > On Jan. 23, 2017, 11:47 a.m., András Piros wrote: > > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, > > line 797 > > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line797> > > > > Use JUnit's `ExpectedException` instead: > > > > https://github.com/junit-team/junit4/wiki/exception-testing I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/ > On Jan. 23, 2017, 11:47 a.m., András Piros wrote: > > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, > > lines 799-802 > > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line799> > > > > Use JUnit's `ExpectedException` instead: > > > > https://github.com/junit-team/junit4/wiki/exception-testing I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/ > On Jan. 23, 2017, 11:47 a.m., András Piros wrote: > > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, > > line 814 > > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line814> > > > > Use JUnit's `ExpectedException` instead: > > > > https://github.com/junit-team/junit4/wiki/exception-testing I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/ > On Jan. 23, 2017, 11:47 a.m., András Piros wrote: > > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, > > lines 816-819 > > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line816> > > > > Use JUnit's `ExpectedException` instead: > > > > https://github.com/junit-team/junit4/wiki/exception-testing I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/ > On Jan. 23, 2017, 11:47 a.m., András Piros wrote: > > core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java, > > line 746 > > <https://reviews.apache.org/r/51029/diff/3/?file=1612951#file1612951line746> > > > > You perform too much testing in one single test method. > > > > I'd have as many methods as testing use cases with appropriate naming > > strategy like > > http://osherove.com/blog/2005/4/3/naming-standards-for-unit-tests.html so > > that for every unit test we exactly know what it's testing and what is the > > expected behavior without needing to read the code, just reading the > > `Exception` logs. This is a single test. We check that 4 actions should get materialized and time of materialization is correct. Dropping this. > On Jan. 23, 2017, 11:47 a.m., András Piros wrote: > > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, > > lines 615-619 > > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line615> > > > > Use JUnit's `ExpectedException` instead: > > > > https://github.com/junit-team/junit4/wiki/exception-testing I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/ > On Jan. 23, 2017, 11:47 a.m., András Piros wrote: > > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, > > line 534 > > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line534> > > > > Extract `DateUtils.parseDateOozieTZ("2009-08-20T01:00Z")` to a > > well-named variable. Dropping this. With re-restructing of test cases, it won't be necessary. > On Jan. 23, 2017, 11:47 a.m., András Piros wrote: > > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, > > line 545 > > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line545> > > > > Extract `DateUtils.parseDateOozieTZ("2009-08-01T01:00Z")` to a > > well-named variable. Dropping this. With re-restructing of test cases, it won't be necessary. > On Jan. 23, 2017, 11:47 a.m., András Piros wrote: > > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, > > line 551 > > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line551> > > > > Extract `DateUtils.parseDateOozieTZ("2009-08-01T01:00Z")` to a > > well-named variable. Dropping this. With re-restructing of test cases, it won't be necessary. > On Jan. 23, 2017, 11:47 a.m., András Piros wrote: > > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, > > line 559 > > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line559> > > > > Extract `DateUtils.parseDateOozieTZ("2009-08-01T01:00Z")` to a > > well-named variable. Dropping this. With re-restructing of test cases, it won't be necessary. > On Jan. 23, 2017, 11:47 a.m., András Piros wrote: > > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, > > line 568 > > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line568> > > > > Extract `DateUtils.parseDateOozieTZ("2009-07-01T01:00Z")` to a > > well-named variable. Dropping this. With re-restructing of test cases, it won't be necessary. > On Jan. 23, 2017, 11:47 a.m., András Piros wrote: > > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, > > line 577 > > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line577> > > > > Extract `DateUtils.parseDateOozieTZ()` to a well-named variable. Dropping this. With re-restructing of test cases, it won't be necessary. - Satish ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51029/#review162674 ----------------------------------------------------------- On Jan. 23, 2017, 9:22 a.m., Satish Saley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51029/ > ----------------------------------------------------------- > > (Updated Jan. 23, 2017, 9:22 a.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 > >
