> 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.

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


- Jarek


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40735/#review108325
-----------------------------------------------------------


On Nov. 26, 2015, 8:14 a.m., Colin Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40735/
> -----------------------------------------------------------
> 
> (Updated Nov. 26, 2015, 8:14 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/ShowJobInOrderTest.java
>  cbf1e90 
> 
> Diff: https://reviews.apache.org/r/40735/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colin Ma
> 
>

Reply via email to