> On April 21, 2015, 2:13 a.m., Jarek Cecho wrote: > > Hi Abe, > > thank you very much for working on this one! I have several > > questions/suggestions inline and a high level questions: > > > > It seems that when running integration tests, all that user sees is this > > output on the console: > > > > Running TestSuite > > Tests run: 26, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 905.703 sec > > > > I'm wondering if there is a way how to report more granual progress then > > "working" and "done"? Like "Executed 5 from 26 tests" or something similar? > > I'm a bit concerned that this is step back in usability as don't have easy > > way to see what are the tests doing - we would have to wait entire time for > > them to finish to even noticed that something is failing. > > Abraham Elmahrek wrote: > Yeah good point. I can dig a bit to find out what we can do here.
I think that it's not "absolute must have" if this will help us to speed up the tests, but I think that it would be quite desirable :) > On April 21, 2015, 2:13 a.m., Jarek Cecho wrote: > > test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java, > > lines 82-94 > > <https://reviews.apache.org/r/31824/diff/3/?file=934923#file934923line82> > > > > I'm wondering why we are removing this one? > > Abraham Elmahrek wrote: > It turns out overriding a method that is annotated with @BeforeSuite is > not correct. So I've removed it and am using HadoopMiniClusterRunner.class as > the default in TomcatTestCase. Got it - so even the tests that do not necessary requires the HadoopMiniCluster will run on it. I guess that it's fair even though not absolutely required. > On April 21, 2015, 2:13 a.m., Jarek Cecho wrote: > > test/src/main/java/org/apache/sqoop/test/testcases/TomcatTestCase.java, > > lines 98-103 > > <https://reviews.apache.org/r/31824/diff/3/?file=934925#file934925line98> > > > > If we are defining the method name in the parent class of all tests, > > let's drop it from the children test classes? I know that several tests are > > using this functionality. > > Abraham Elmahrek wrote: > Indeed. I've changed the getTestName() methods in the other classes as > well. Now, it uses context.getName() everywhere to ensure that when this > method is called, there won't be an NPE. Maybe we can move it higher up in > the inheritance chain? Yeah I think so :) > On April 21, 2015, 2:13 a.m., Jarek Cecho wrote: > > test/src/main/java/org/apache/sqoop/test/testcases/TomcatTestCase.java, > > line 108 > > <https://reviews.apache.org/r/31824/diff/3/?file=934925#file934925line108> > > > > We're changing the default value here, so I'm wondering if it's > > intentional? > > Abraham Elmahrek wrote: > Completely intentional. ConnectorTestCase was overriding this and using a > different default. I've removed the method overriding this one in > ConnectorTestCase and changed the default here. Got it, I'm +1 then. > On April 21, 2015, 2:13 a.m., Jarek Cecho wrote: > > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/AllTypesTest.java, > > lines 49-177 > > <https://reviews.apache.org/r/31824/diff/3/?file=934927#file934927line49> > > > > Seems as a lot of unrelated changes and moves around? > > Abraham Elmahrek wrote: > Actually, "beforeMethod" was completely removed because it wasn't being > executed in time any more. The test name will be context.getName() until > TomcatTestCase.startServer is ran. I did move getTestName() unintentionally. > It took me a while to get any of this working correctly. Let me know if you'd > like to move getTestName() back to its original place. Not necessarily - the change just wasn't obvious to me. My apologies for that. - Jarek ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31824/#review80876 ----------------------------------------------------------- On April 19, 2015, 9:02 a.m., Abraham Elmahrek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31824/ > ----------------------------------------------------------- > > (Updated April 19, 2015, 9:02 a.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-1952 > https://issues.apache.org/jira/browse/SQOOP-1952 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > commit d489e803cc01127ded5270bbe7cf640e674648aa > Author: Abraham Elmahrek <[email protected]> > Date: Fri Mar 6 17:15:06 2015 -0800 > > SQOOP-1952 > > :100755 100755 b1acdc4... 4a5ce80... M dev-support/test-patch.py > :100644 100644 32afc77... 1b30782... M pom.xml > :100644 100644 f743d25... d2761f6... M test/pom.xml > :100644 100644 ce6af6e... 90afef1... M > test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java > :000000 100644 0000000... 2febf5e... A test/src/test/resources/suite.xml > > > Diffs > ----- > > > common-test/src/main/java/org/apache/sqoop/common/test/kafka/KafkaRunnerFactory.java > b26ca74 > common-test/src/main/java/org/apache/sqoop/common/test/kafka/TestUtil.java > 09ddcc7 > pom.xml c608ca7 > test/pom.xml d8fbfa2 > test/src/main/java/org/apache/sqoop/test/hadoop/HadoopRunnerFactory.java > 020fa3f > test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java > c8fca68 > > test/src/main/java/org/apache/sqoop/test/testcases/KafkaConnectorTestCase.java > 9aa69ed > test/src/main/java/org/apache/sqoop/test/testcases/TomcatTestCase.java > 9416473 > > test/src/test/java/org/apache/sqoop/integration/connector/hive/FromRDBMSToKiteHiveTest.java > 37a0a7d > > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/AllTypesTest.java > 4e46637 > > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/IncrementalReadTest.java > 5bde35c > > test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromHDFSToKafkaTest.java > 88db2f2 > > test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromRDBMSToKafkaTest.java > 92a52b8 > > test/src/test/java/org/apache/sqoop/integration/connector/kite/FromRDBMSToKiteTest.java > 5c90501 > test/src/test/resources/suite.xml PRE-CREATION > > Diff: https://reviews.apache.org/r/31824/diff/ > > > Testing > ------- > > Tested locally. Also, tested against SQOOP-1781 jira and posted results > there. They seem mixed largely because it's difficult to provide changes to > testing and changes to dev support together. > > > Thanks, > > Abraham Elmahrek > >
