> On Aug. 10, 2015, 8:34 a.m., Dian Fu wrote: > > test/src/test/java/org/apache/sqoop/integration/server/InvalidRESTCallsTest.java, > > line 63 > > <https://reviews.apache.org/r/37261/diff/1/?file=1035287#file1035287line63> > > > > I think it would be better if we can move method "assertResponseCode" > > out of class Validator. > > Jarek Cecho wrote: > I've put validation methods inside the Validator class because they are > specific to the validator's instance - they are depending on the internal > state of the object (HttpURL Connection for example and in the future on > error/input streams). > > My goal here was to allow as simple implementation of the Validator class > as possible, so that the testcases themselves are simple to read and follow. > So when you look at line 98 for example, I have only: > > assertResponseCode(405); > > And the method is smart enough to go to the read the current response > code internally from the instance. My earlier iterations of the code was: > > assertEquals(connection.getResponseCode(), 405); > > Which at least for me is much harder to read, especially if we will need > to repeat this in every single test :-/ > > Having said that, would you agree to keep the method in the Validator > object?
Make sense to me. :) - Dian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37261/#review94710 ----------------------------------------------------------- On Aug. 12, 2015, 8:20 p.m., Jarek Cecho wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37261/ > ----------------------------------------------------------- > > (Updated Aug. 12, 2015, 8:20 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-2469 > https://issues.apache.org/jira/browse/SQOOP-2469 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > Added infrastructure to run arbitrary REST requests with two small tests. > Interestingly enough doing POST to /version leads to tomcat response rather > then Sqoop response which we will want to fix in future patch - I want to > keep this one simple. > > > Diffs > ----- > > > test/src/test/java/org/apache/sqoop/integration/server/InvalidRESTCallsTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/37261/diff/ > > > Testing > ------- > > The test case is passing. > > > Thanks, > > Jarek Cecho > >
