> On Nov. 30, 2015, 5:07 p.m., Jarek Cecho wrote: > > It seems that we've added the test ShowJobInOrderTest back in SQOOP-2398, > > but sadly the description of the JIRA is very sparse. I'm not clear why we > > actually added it as we don't have any guarantees in terms of ordering on > > the REST interface. > > > > Nevertheless it seems that the issue is that we're comparing persistence ID > > and we are assuming completely new database at the begging (e.g. all > > incremental columns to start from 1). That is incorrect assumption and > > comparison as a) we're droopping the ID's completely and b) in case that > > we're running the test case against real database (MySQL, PostgreSQL) it > > will never be true. > > > > So, what about fixing it properly? Rather then keeing the now-not-public > > Ids what about setting jobName() properly and compare that instead? > > Colin Ma wrote: > Thanks for the comments. I agree that we should compare the job by > jobName, and this is implemented in SQOOP-2690. > The root cause for this problem is InformalObjectNameTest, > ShowJobInOrderTest, SubmissionWithDisabledModelObjectsTest shared the same > DatabaseInfrastructureProvider. The InformalObjectNameTest is always executed > first(29 jobs will be created), and this make the ShowJobInOrderTest failed. > Even compare with the name(already included in SQOOP-2690), the > ShowJobInOrderTest will be still failed. > Clear DB for after the test can solve the problem, I think maybe > afterClass will be better than afterMethod. > > Jarek Cecho wrote: > I see your point Colin, thanks for explanation. So if I would reword that > to make sure that I understand the problem correct - it seems that the root > of the issue is that InformalObjectNameTest is not properly cleaning up > created objects (links and jobs). > > I don't mind using afterMethod() callback - my concern is that > re-creating new DatabaseHandler won't help us with any other database other > then Derby - if we would run tests against MySQL, PostgreSQL - all those > would fail. > > So perhaps rather then re-creating the database, would it make sense to > add a callback to clean up the state? Something like I'm doing here: > > > https://github.com/apache/sqoop/blob/sqoop2/test/src/test/java/org/apache/sqoop/integration/server/rest/RestTest.java#L134
The patch is updated according to your comments. Currently, this problem is only found in the package org.apache.sqoop.integration.server, so I only clear the data for these test cases. For the other test cases, we can fix it if necessary in the future. - Colin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40735/#review108325 ----------------------------------------------------------- On Dec. 3, 2015, 1:35 a.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40735/ > ----------------------------------------------------------- > > (Updated Dec. 3, 2015, 1:35 a.m.) > > > Review request for Sqoop. > > > Repository: sqoop-sqoop2 > > > Description > ------- > > ShowJobInOrderTest always thrown the exception during the test, because the > DatabaseInfrastructureProvider is shared with all tests case in one suite. > > > Diffs > ----- > > test/src/main/java/org/apache/sqoop/test/infrastructure/SqoopTestCase.java > 4c5d3a8 > > test/src/test/java/org/apache/sqoop/integration/server/InformalObjectNameTest.java > 920679f > > test/src/test/java/org/apache/sqoop/integration/server/ShowJobInOrderTest.java > cbf1e90 > > test/src/test/java/org/apache/sqoop/integration/server/SubmissionWithDisabledModelObjectsTest.java > 9e682bc > test/src/test/java/org/apache/sqoop/integration/server/rest/RestTest.java > 4ac564c > > Diff: https://reviews.apache.org/r/40735/diff/ > > > Testing > ------- > > > Thanks, > > Colin Ma > >
