> 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? > > Colin Ma wrote: > 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.
Make sense, thank you! - Jarek ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41910/#review112862 ----------------------------------------------------------- On Jan. 6, 2016, 1:51 p.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41910/ > ----------------------------------------------------------- > > (Updated Jan. 6, 2016, 1:51 p.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 > c300b33 > > test/src/main/java/org/apache/sqoop/test/minicluster/JettySqoopMiniClusterWithExternalConnector.java > PRE-CREATION > > test/src/main/java/org/apache/sqoop/test/testcases/ConnectorClasspathTestCase.java > 6db1db8 > test/src/main/java/org/apache/sqoop/test/testcases/ShellTestCase.java > PRE-CREATION > test/src/main/java/org/apache/sqoop/test/utils/ConnectorUtils.java > PRE-CREATION > > test/src/test/java/org/apache/sqoop/integration/connectorloading/ClasspathTest.java > 4bb6aa1 > > test/src/test/java/org/apache/sqoop/integration/connectorloading/ConnectorClasspathIsolationTest.java > 5b95631 > > test/src/test/java/org/apache/sqoop/integration/server/SubmissionWithDisabledModelObjectsTest.java > 9adebea > 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/TestConnectorForShell/TestConnectorForShell.java > PRE-CREATION > test/src/test/resources/TestConnectorForShell/TestExtractorForShell.java > PRE-CREATION > > test/src/test/resources/TestConnectorForShell/TestFromDestroyerForShell.java > PRE-CREATION > > test/src/test/resources/TestConnectorForShell/TestFromInitializerForShell.java > PRE-CREATION > > test/src/test/resources/TestConnectorForShell/TestFromJobConfigForShell.java > PRE-CREATION > > test/src/test/resources/TestConnectorForShell/TestFromJobConfigurationForShell.java > PRE-CREATION > test/src/test/resources/TestConnectorForShell/TestLinkConfigForShell.java > PRE-CREATION > > test/src/test/resources/TestConnectorForShell/TestLinkConfigurationForShell.java > PRE-CREATION > test/src/test/resources/TestConnectorForShell/TestLoaderForShell.java > PRE-CREATION > test/src/test/resources/TestConnectorForShell/TestPartitionForShell.java > PRE-CREATION > test/src/test/resources/TestConnectorForShell/TestPartitionerForShell.java > PRE-CREATION > test/src/test/resources/TestConnectorForShell/TestToDestroyerForShell.java > PRE-CREATION > > test/src/test/resources/TestConnectorForShell/TestToInitializerForShell.java > PRE-CREATION > test/src/test/resources/TestConnectorForShell/TestToJobConfigForShell.java > PRE-CREATION > > test/src/test/resources/TestConnectorForShell/TestToJobConfigurationForShell.java > PRE-CREATION > test/src/test/resources/TestConnectorForShell/sqoopconnector.properties > PRE-CREATION > > test/src/test/resources/TestConnectorForShell/test-connector-for-shell.properties > PRE-CREATION > test/src/test/resources/shell-tests-suite.xml PRE-CREATION > > Diff: https://reviews.apache.org/r/41910/diff/ > > > Testing > ------- > > > Thanks, > > Colin Ma > >
