> On Jan. 5, 2016, 3:32 p.m., Jarek Cecho wrote: > > 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?
Yes, using a test-only connector is more suitable for the shell test, we only focus on the shell. Just one drawback, the stopCommand can't be tested, because sqoop only support MR now, and the submission will be always failed. But it's acceptable and the other commands will be covered properly. I'll update the patch with the test connector. - Colin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41910/#review112862 ----------------------------------------------------------- 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 > >
