----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55466/#review166822 -----------------------------------------------------------
Fix it, then Ship it! Hey Bogi, The change looks good. I've only raised one concern: The mechanism won't be indentical as before b/c we will lose the text messages were included in the fail calls. Would you please consider the usage of reportMissingExceptionWithMessage method? Also: IMHO it would make sense to open JIRAs for your previous cleanup changes to add back those information in the same way. Thanks, Attila src/test/com/cloudera/sqoop/TestIncrementalImport.java <https://reviews.apache.org/r/55466/#comment238897> Hey Bogi, Would you please consider using the reportMissingExceptionWithMessage method of ExpectedException not to loose these kind of information in the new version either? Thanks - Attila Szabo On Feb. 25, 2017, 5:46 p.m., Boglarka Egyed wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55466/ > ----------------------------------------------------------- > > (Updated Feb. 25, 2017, 5:46 p.m.) > > > Review request for Sqoop, Attila Szabo, Anna Szonyi, and Liz Szilagyi. > > > Bugs: SQOOP-3100 > https://issues.apache.org/jira/browse/SQOOP-3100 > > > Repository: sqoop-trunk > > > Description > ------- > > Normalizing test cases where we except an exception using JUnit's > ExpectedException rule to make the code more clean and self-explanatory. > > > Diffs > ----- > > src/test/com/cloudera/sqoop/TestIncrementalImport.java > 57f4433d8f50b37f102a8a8ada7196d9287b5557 > src/test/com/cloudera/sqoop/lib/TestRecordParser.java > 57bdb5fe057f74dcaef9e2e3d6e1d26d3af7833a > src/test/com/cloudera/sqoop/metastore/TestSavedJobs.java > 1fb732439dd2c8f187af726935a335c993895bc5 > src/test/com/cloudera/sqoop/orm/TestClassWriter.java > a27353df4ed84a7dfe7612662f3170c00951d2cf > > Diff: https://reviews.apache.org/r/55466/diff/ > > > Testing > ------- > > ant test, ant clean test > > > Thanks, > > Boglarka Egyed > >
