----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41910/#review112862 -----------------------------------------------------------
Overall I like the approach - I just have one concern. I'm afraid that using existing connector is quite fragile as those tests will start failing when we add new property to the configuration objects. Would it make sense to create special test-only connector for the shell tests similarly as we have for the classpath isolation? This way any changes to the jdbc connector wouldn't affect unrelated shell tests. What do you think? - Jarek Cecho On Jan. 5, 2016, 4:17 a.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41910/ > ----------------------------------------------------------- > > (Updated Jan. 5, 2016, 4:17 a.m.) > > > Review request for Sqoop. > > > Repository: sqoop-sqoop2 > > > Description > ------- > > For the test of shell, currently, too many mock in test cases, and some bugs > won't be detected. The integration test should be added for shell, and do the > test with sqoop server. > > > Diffs > ----- > > shell/src/main/java/org/apache/sqoop/shell/SetCommand.java 0a04e3d > shell/src/main/java/org/apache/sqoop/shell/ShowCommand.java c148eeb > shell/src/main/java/org/apache/sqoop/shell/StartCommand.java 679c1f7 > shell/src/main/java/org/apache/sqoop/shell/StatusCommand.java 6082799 > shell/src/main/java/org/apache/sqoop/shell/StopCommand.java 83c571a > test/pom.xml bd1680f > test/src/main/java/org/apache/sqoop/test/infrastructure/SqoopTestCase.java > 8fbefd8 > test/src/main/java/org/apache/sqoop/test/testcases/ShellTestCase.java > PRE-CREATION > test/src/test/java/org/apache/sqoop/integration/shell/CloneCommandTest.java > PRE-CREATION > > test/src/test/java/org/apache/sqoop/integration/shell/CreateCommandTest.java > PRE-CREATION > > test/src/test/java/org/apache/sqoop/integration/shell/DeleteCommandTest.java > PRE-CREATION > > test/src/test/java/org/apache/sqoop/integration/shell/DisableCommandTest.java > PRE-CREATION > > test/src/test/java/org/apache/sqoop/integration/shell/EnableCommandTest.java > PRE-CREATION > test/src/test/java/org/apache/sqoop/integration/shell/SetCommandTest.java > PRE-CREATION > test/src/test/java/org/apache/sqoop/integration/shell/ShowCommandTest.java > PRE-CREATION > test/src/test/java/org/apache/sqoop/integration/shell/StartCommandTest.java > PRE-CREATION > > test/src/test/java/org/apache/sqoop/integration/shell/StatusCommandTest.java > PRE-CREATION > test/src/test/java/org/apache/sqoop/integration/shell/StopCommandTest.java > PRE-CREATION > > test/src/test/java/org/apache/sqoop/integration/shell/UpdateCommandTest.java > PRE-CREATION > test/src/test/resources/shell-tests-suite.xml PRE-CREATION > > Diff: https://reviews.apache.org/r/41910/diff/ > > > Testing > ------- > > > Thanks, > > Colin Ma > >
