You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Weston Price <wp...@redhat.com> on 2012/01/19 01:28:11 UTC

Review Request: Add Unit/System Testing to Qpid JCA Project

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

Review request for qpid, Gordon Sim, Robbie Gemmell, rajith attapattu, and Keith Wall.


Summary
-------

The following patch adds the necessary changes to add system tests as well as unit tests to the Qpid JCA subproject. Although we use the TCK (internal) to test the JCA adapter, the lack of an application server should not prohibit testing in the JCA project. While full integration testing might not be possible, we do provide a non-managed javax.resource.spi.ConnectionManager that allows us to acquire a javax.jms.Connection/javax.jms.Session etc (non-pooled, no auto XA enlistment) which in turn allows for system testing. Unit testing can also be achieved similar to the other Qpid Java sub projects. Also, this will allow us to add mock objects where appropriate without having to bring in an entire JEE environment.

Note, the JCA examples also provide a 'smoke test' like framework, however, more testing really needs to be added and maintained as was initially identified in the Qpid JCA review prior to the adapter being included in the Qpid project. 

While the Qpid JCA adapter only officially supports the C++ broker, a majority of the tests can be used with the Java Broker other than XA specific tests which can be excluded using the normal exclusion mechanism. 

To get things started I have added two tests classes:

qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java

This is a system test class with two tests that test the following JIRA's

https://issues.apache.org/jira/browse/QPID-3700 -- check for javax.jms.IllegalStateException for Session.commit() when a connection has been closed
https://issues.apache.org/jira/browse/QPID-3731 -- QpidRAMessage should call session.getSessionInternal() to determine if the underlying session has been closed prior to calling message.acknowledge().

Results:
[junit] Running org.apache.qpid.ra.QpidRAConnectionTest
[junit] Tests run: 2, Failures: 0, Errors: 0, Time elapsed: 2.75 sec


qpid/java/jca/src/test/java/org/apache/qpid/ra/QpidResourceAdapterTest.java

This is a unit test class that tests for the correct return value in the QpidResourceAdapter.getXAResources() call. Note, there is no corresponding JIRA for this test as it was implemented prior to the JCA being committed to the main repo.

Results:
 [junit] Running org.apache.qpid.ra.QpidResourceAdapterTest
 [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 0.129 sec
 

Note, while it might seem to be overkill to review a patch that simply adds testing to a sub-project, I had to touch a few files outside of the JCA tree and I thought a review was in order.


This addresses bug QPID-3751.
    https://issues.apache.org/jira/browse/QPID-3751


Diffs
-----

  /trunk/qpid/java/jca/example/.gitignore 1233097 
  /trunk/qpid/java/systests/build.xml 1233097 
  /trunk/qpid/java/build.deps 1233097 
  /trunk/qpid/java/build.xml 1233097 

Diff: https://reviews.apache.org/r/3540/diff


Testing
-------

Java build and test-suite execution.


Thanks,

Weston


Re: Review Request: Add Unit/System Testing to Qpid JCA Project

Posted by Weston Price <wp...@redhat.com>.

> On 2012-01-20 14:18:52, Keith Wall wrote:
> > /trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java, line 36
> > <https://reviews.apache.org/r/3540/diff/2/?file=69867#file69867line36>
> >
> >     I'd suggest using the QpidBrokerTestCase.DEFAULT_PORT as the port number rather than hardcoding 15672.
> >     
> >

Agreed will fix.


> On 2012-01-20 14:18:52, Keith Wall wrote:
> > /trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java, line 38
> > <https://reviews.apache.org/r/3540/diff/2/?file=69867#file69867line38>
> >
> >     I'd suggest a better name would be something like "testSessionCommitOnClosedConnectionThrowsException"

Agreed will fix.


> On 2012-01-20 14:18:52, Keith Wall wrote:
> > /trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java, line 75
> > <https://reviews.apache.org/r/3540/diff/2/?file=69867#file69867line75>
> >
> >     You'd normally only #acknowledge() on a message that has been received by a Consumer within a CLIENT_ACK Session.  I don't understand the meaning of acknowledging a newly created message.
> >     
> >     I can see it serves your purpose (testing your change to call sessionInternal), but I don't think this represents useful end to end test in this form.
> >     
> >     
> >     
> >

Agree in principle, however setting up a consumer using JCA without an app server is problematic. The code was added simply to test the patch without having to write a significant amount of mock code for this one condition.


> On 2012-01-20 14:18:52, Keith Wall wrote:
> > /trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java, line 46
> > <https://reviews.apache.org/r/3540/diff/2/?file=69867#file69867line46>
> >
> >     Whilst it is true that Session#createSession ignores the second argument (acknowledgeMode) if the first argument (transacted) is true, for me the code is more obvious if you write:
> >     
> >     c.createSession(true, Session.SESSION_TRANSACTED);

Agreed will change.


- Weston


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


On 2012-01-19 00:32:22, Weston Price wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3540/
> -----------------------------------------------------------
> 
> (Updated 2012-01-19 00:32:22)
> 
> 
> Review request for qpid, Gordon Sim, Robbie Gemmell, rajith attapattu, and Keith Wall.
> 
> 
> Summary
> -------
> 
> The following patch adds the necessary changes to add system tests as well as unit tests to the Qpid JCA subproject. Although we use the TCK (internal) to test the JCA adapter, the lack of an application server should not prohibit testing in the JCA project. While full integration testing might not be possible, we do provide a non-managed javax.resource.spi.ConnectionManager that allows us to acquire a javax.jms.Connection/javax.jms.Session etc (non-pooled, no auto XA enlistment) which in turn allows for system testing. Unit testing can also be achieved similar to the other Qpid Java sub projects. Also, this will allow us to add mock objects where appropriate without having to bring in an entire JEE environment.
> 
> Note, the JCA examples also provide a 'smoke test' like framework, however, more testing really needs to be added and maintained as was initially identified in the Qpid JCA review prior to the adapter being included in the Qpid project. 
> 
> While the Qpid JCA adapter only officially supports the C++ broker, a majority of the tests can be used with the Java Broker other than XA specific tests which can be excluded using the normal exclusion mechanism. 
> 
> To get things started I have added two tests classes:
> 
> qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java
> 
> This is a system test class with two tests that test the following JIRA's
> 
> https://issues.apache.org/jira/browse/QPID-3700 -- check for javax.jms.IllegalStateException for Session.commit() when a connection has been closed
> https://issues.apache.org/jira/browse/QPID-3731 -- QpidRAMessage should call session.getSessionInternal() to determine if the underlying session has been closed prior to calling message.acknowledge().
> 
> Results:
> [junit] Running org.apache.qpid.ra.QpidRAConnectionTest
> [junit] Tests run: 2, Failures: 0, Errors: 0, Time elapsed: 2.75 sec
> 
> 
> qpid/java/jca/src/test/java/org/apache/qpid/ra/QpidResourceAdapterTest.java
> 
> This is a unit test class that tests for the correct return value in the QpidResourceAdapter.getXAResources() call. Note, there is no corresponding JIRA for this test as it was implemented prior to the JCA being committed to the main repo.
> 
> Results:
>  [junit] Running org.apache.qpid.ra.QpidResourceAdapterTest
>  [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 0.129 sec
>  
> 
> Note, while it might seem to be overkill to review a patch that simply adds testing to a sub-project, I had to touch a few files outside of the JCA tree and I thought a review was in order.
> 
> 
> This addresses bug QPID-3751.
>     https://issues.apache.org/jira/browse/QPID-3751
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/java/build.deps 1233097 
>   /trunk/qpid/java/build.xml 1233097 
>   /trunk/qpid/java/jca/example/.gitignore 1233097 
>   /trunk/qpid/java/jca/src/test/java/org/apache/qpid/ra/QpidResourceAdapterTest.java PRE-CREATION 
>   /trunk/qpid/java/systests/build.xml 1233097 
>   /trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java PRE-CREATION 
>   /trunk/qpid/java/.gitignore PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/3540/diff
> 
> 
> Testing
> -------
> 
> Java build and test-suite execution.
> 
> 
> Thanks,
> 
> Weston
> 
>


Re: Review Request: Add Unit/System Testing to Qpid JCA Project

Posted by Keith Wall <ke...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3540/#review4487
-----------------------------------------------------------



/trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java
<https://reviews.apache.org/r/3540/#comment10072>

    I'd suggest using the QpidBrokerTestCase.DEFAULT_PORT as the port number rather than hardcoding 15672.
    
    



/trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java
<https://reviews.apache.org/r/3540/#comment10076>

    I'd suggest a better name would be something like "testSessionCommitOnClosedConnectionThrowsException"



/trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java
<https://reviews.apache.org/r/3540/#comment10073>

    Whilst it is true that Session#createSession ignores the second argument (acknowledgeMode) if the first argument (transacted) is true, for me the code is more obvious if you write:
    
    c.createSession(true, Session.SESSION_TRANSACTED);



/trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java
<https://reviews.apache.org/r/3540/#comment10075>

    You'd normally only #acknowledge() on a message that has been received by a Consumer within a CLIENT_ACK Session.  I don't understand the meaning of acknowledging a newly created message.
    
    I can see it serves your purpose (testing your change to call sessionInternal), but I don't think this represents useful end to end test in this form.
    
    
    
    


- Keith


On 2012-01-19 00:32:22, Weston Price wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3540/
> -----------------------------------------------------------
> 
> (Updated 2012-01-19 00:32:22)
> 
> 
> Review request for qpid, Gordon Sim, Robbie Gemmell, rajith attapattu, and Keith Wall.
> 
> 
> Summary
> -------
> 
> The following patch adds the necessary changes to add system tests as well as unit tests to the Qpid JCA subproject. Although we use the TCK (internal) to test the JCA adapter, the lack of an application server should not prohibit testing in the JCA project. While full integration testing might not be possible, we do provide a non-managed javax.resource.spi.ConnectionManager that allows us to acquire a javax.jms.Connection/javax.jms.Session etc (non-pooled, no auto XA enlistment) which in turn allows for system testing. Unit testing can also be achieved similar to the other Qpid Java sub projects. Also, this will allow us to add mock objects where appropriate without having to bring in an entire JEE environment.
> 
> Note, the JCA examples also provide a 'smoke test' like framework, however, more testing really needs to be added and maintained as was initially identified in the Qpid JCA review prior to the adapter being included in the Qpid project. 
> 
> While the Qpid JCA adapter only officially supports the C++ broker, a majority of the tests can be used with the Java Broker other than XA specific tests which can be excluded using the normal exclusion mechanism. 
> 
> To get things started I have added two tests classes:
> 
> qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java
> 
> This is a system test class with two tests that test the following JIRA's
> 
> https://issues.apache.org/jira/browse/QPID-3700 -- check for javax.jms.IllegalStateException for Session.commit() when a connection has been closed
> https://issues.apache.org/jira/browse/QPID-3731 -- QpidRAMessage should call session.getSessionInternal() to determine if the underlying session has been closed prior to calling message.acknowledge().
> 
> Results:
> [junit] Running org.apache.qpid.ra.QpidRAConnectionTest
> [junit] Tests run: 2, Failures: 0, Errors: 0, Time elapsed: 2.75 sec
> 
> 
> qpid/java/jca/src/test/java/org/apache/qpid/ra/QpidResourceAdapterTest.java
> 
> This is a unit test class that tests for the correct return value in the QpidResourceAdapter.getXAResources() call. Note, there is no corresponding JIRA for this test as it was implemented prior to the JCA being committed to the main repo.
> 
> Results:
>  [junit] Running org.apache.qpid.ra.QpidResourceAdapterTest
>  [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 0.129 sec
>  
> 
> Note, while it might seem to be overkill to review a patch that simply adds testing to a sub-project, I had to touch a few files outside of the JCA tree and I thought a review was in order.
> 
> 
> This addresses bug QPID-3751.
>     https://issues.apache.org/jira/browse/QPID-3751
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/java/build.deps 1233097 
>   /trunk/qpid/java/build.xml 1233097 
>   /trunk/qpid/java/jca/example/.gitignore 1233097 
>   /trunk/qpid/java/jca/src/test/java/org/apache/qpid/ra/QpidResourceAdapterTest.java PRE-CREATION 
>   /trunk/qpid/java/systests/build.xml 1233097 
>   /trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java PRE-CREATION 
>   /trunk/qpid/java/.gitignore PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/3540/diff
> 
> 
> Testing
> -------
> 
> Java build and test-suite execution.
> 
> 
> Thanks,
> 
> Weston
> 
>


Re: Review Request: Add Unit/System Testing to Qpid JCA Project

Posted by Weston Price <wp...@redhat.com>.

> On 2012-01-19 15:10:21, Andrew Stitcher wrote:
> > /trunk/qpid/java/build.xml, line 1
> > <https://reviews.apache.org/r/3540/diff/2/?file=69863#file69863line1>
> >
> >     There seems to be no actual change to this file, why check it in.

I automatically remove trailing whitespaces when saving a file, this appears to be what that is.


> On 2012-01-19 15:10:21, Andrew Stitcher wrote:
> > /trunk/qpid/java/jca/example/.gitignore, line 3
> > <https://reviews.apache.org/r/3540/diff/2/?file=69864#file69864line3>
> >
> >     I don't know what file type .swp is, but you've added to to several .gitignore files - why not add it to a .gitignore higher in the file hierarchy.

*.swp is a temp VIM file that gets generated when VIM has open files. I will add it higher in the hierarchy.


> On 2012-01-19 15:10:21, Andrew Stitcher wrote:
> > /trunk/qpid/java/.gitignore, line 1
> > <https://reviews.apache.org/r/3540/diff/2/?file=69861#file69861line1>
> >
> >     Not sure if this files needs to be checked in.

I can try using the .gitignore file already in use in the tree.


- Weston


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


On 2012-01-19 00:32:22, Weston Price wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3540/
> -----------------------------------------------------------
> 
> (Updated 2012-01-19 00:32:22)
> 
> 
> Review request for qpid, Gordon Sim, Robbie Gemmell, rajith attapattu, and Keith Wall.
> 
> 
> Summary
> -------
> 
> The following patch adds the necessary changes to add system tests as well as unit tests to the Qpid JCA subproject. Although we use the TCK (internal) to test the JCA adapter, the lack of an application server should not prohibit testing in the JCA project. While full integration testing might not be possible, we do provide a non-managed javax.resource.spi.ConnectionManager that allows us to acquire a javax.jms.Connection/javax.jms.Session etc (non-pooled, no auto XA enlistment) which in turn allows for system testing. Unit testing can also be achieved similar to the other Qpid Java sub projects. Also, this will allow us to add mock objects where appropriate without having to bring in an entire JEE environment.
> 
> Note, the JCA examples also provide a 'smoke test' like framework, however, more testing really needs to be added and maintained as was initially identified in the Qpid JCA review prior to the adapter being included in the Qpid project. 
> 
> While the Qpid JCA adapter only officially supports the C++ broker, a majority of the tests can be used with the Java Broker other than XA specific tests which can be excluded using the normal exclusion mechanism. 
> 
> To get things started I have added two tests classes:
> 
> qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java
> 
> This is a system test class with two tests that test the following JIRA's
> 
> https://issues.apache.org/jira/browse/QPID-3700 -- check for javax.jms.IllegalStateException for Session.commit() when a connection has been closed
> https://issues.apache.org/jira/browse/QPID-3731 -- QpidRAMessage should call session.getSessionInternal() to determine if the underlying session has been closed prior to calling message.acknowledge().
> 
> Results:
> [junit] Running org.apache.qpid.ra.QpidRAConnectionTest
> [junit] Tests run: 2, Failures: 0, Errors: 0, Time elapsed: 2.75 sec
> 
> 
> qpid/java/jca/src/test/java/org/apache/qpid/ra/QpidResourceAdapterTest.java
> 
> This is a unit test class that tests for the correct return value in the QpidResourceAdapter.getXAResources() call. Note, there is no corresponding JIRA for this test as it was implemented prior to the JCA being committed to the main repo.
> 
> Results:
>  [junit] Running org.apache.qpid.ra.QpidResourceAdapterTest
>  [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 0.129 sec
>  
> 
> Note, while it might seem to be overkill to review a patch that simply adds testing to a sub-project, I had to touch a few files outside of the JCA tree and I thought a review was in order.
> 
> 
> This addresses bug QPID-3751.
>     https://issues.apache.org/jira/browse/QPID-3751
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/java/build.deps 1233097 
>   /trunk/qpid/java/build.xml 1233097 
>   /trunk/qpid/java/jca/example/.gitignore 1233097 
>   /trunk/qpid/java/jca/src/test/java/org/apache/qpid/ra/QpidResourceAdapterTest.java PRE-CREATION 
>   /trunk/qpid/java/systests/build.xml 1233097 
>   /trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java PRE-CREATION 
>   /trunk/qpid/java/.gitignore PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/3540/diff
> 
> 
> Testing
> -------
> 
> Java build and test-suite execution.
> 
> 
> Thanks,
> 
> Weston
> 
>


Re: Review Request: Add Unit/System Testing to Qpid JCA Project

Posted by Andrew Stitcher <as...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3540/#review4471
-----------------------------------------------------------


I've found a few nits:


/trunk/qpid/java/.gitignore
<https://reviews.apache.org/r/3540/#comment10025>

    Not sure if this files needs to be checked in.



/trunk/qpid/java/build.xml
<https://reviews.apache.org/r/3540/#comment10024>

    There seems to be no actual change to this file, why check it in.



/trunk/qpid/java/jca/example/.gitignore
<https://reviews.apache.org/r/3540/#comment10026>

    I don't know what file type .swp is, but you've added to to several .gitignore files - why not add it to a .gitignore higher in the file hierarchy.


- Andrew


On 2012-01-19 00:32:22, Weston Price wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3540/
> -----------------------------------------------------------
> 
> (Updated 2012-01-19 00:32:22)
> 
> 
> Review request for qpid, Gordon Sim, Robbie Gemmell, rajith attapattu, and Keith Wall.
> 
> 
> Summary
> -------
> 
> The following patch adds the necessary changes to add system tests as well as unit tests to the Qpid JCA subproject. Although we use the TCK (internal) to test the JCA adapter, the lack of an application server should not prohibit testing in the JCA project. While full integration testing might not be possible, we do provide a non-managed javax.resource.spi.ConnectionManager that allows us to acquire a javax.jms.Connection/javax.jms.Session etc (non-pooled, no auto XA enlistment) which in turn allows for system testing. Unit testing can also be achieved similar to the other Qpid Java sub projects. Also, this will allow us to add mock objects where appropriate without having to bring in an entire JEE environment.
> 
> Note, the JCA examples also provide a 'smoke test' like framework, however, more testing really needs to be added and maintained as was initially identified in the Qpid JCA review prior to the adapter being included in the Qpid project. 
> 
> While the Qpid JCA adapter only officially supports the C++ broker, a majority of the tests can be used with the Java Broker other than XA specific tests which can be excluded using the normal exclusion mechanism. 
> 
> To get things started I have added two tests classes:
> 
> qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java
> 
> This is a system test class with two tests that test the following JIRA's
> 
> https://issues.apache.org/jira/browse/QPID-3700 -- check for javax.jms.IllegalStateException for Session.commit() when a connection has been closed
> https://issues.apache.org/jira/browse/QPID-3731 -- QpidRAMessage should call session.getSessionInternal() to determine if the underlying session has been closed prior to calling message.acknowledge().
> 
> Results:
> [junit] Running org.apache.qpid.ra.QpidRAConnectionTest
> [junit] Tests run: 2, Failures: 0, Errors: 0, Time elapsed: 2.75 sec
> 
> 
> qpid/java/jca/src/test/java/org/apache/qpid/ra/QpidResourceAdapterTest.java
> 
> This is a unit test class that tests for the correct return value in the QpidResourceAdapter.getXAResources() call. Note, there is no corresponding JIRA for this test as it was implemented prior to the JCA being committed to the main repo.
> 
> Results:
>  [junit] Running org.apache.qpid.ra.QpidResourceAdapterTest
>  [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 0.129 sec
>  
> 
> Note, while it might seem to be overkill to review a patch that simply adds testing to a sub-project, I had to touch a few files outside of the JCA tree and I thought a review was in order.
> 
> 
> This addresses bug QPID-3751.
>     https://issues.apache.org/jira/browse/QPID-3751
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/java/build.deps 1233097 
>   /trunk/qpid/java/build.xml 1233097 
>   /trunk/qpid/java/jca/example/.gitignore 1233097 
>   /trunk/qpid/java/jca/src/test/java/org/apache/qpid/ra/QpidResourceAdapterTest.java PRE-CREATION 
>   /trunk/qpid/java/systests/build.xml 1233097 
>   /trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java PRE-CREATION 
>   /trunk/qpid/java/.gitignore PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/3540/diff
> 
> 
> Testing
> -------
> 
> Java build and test-suite execution.
> 
> 
> Thanks,
> 
> Weston
> 
>


Re: Review Request: Add Unit/System Testing to Qpid JCA Project

Posted by Weston Price <wp...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3540/
-----------------------------------------------------------

(Updated 2012-01-19 00:32:22.696911)


Review request for qpid, Gordon Sim, Robbie Gemmell, rajith attapattu, and Keith Wall.


Changes
-------

Whoops. Wrong diff originally uploaded. JCA test classes have now been included.


Summary
-------

The following patch adds the necessary changes to add system tests as well as unit tests to the Qpid JCA subproject. Although we use the TCK (internal) to test the JCA adapter, the lack of an application server should not prohibit testing in the JCA project. While full integration testing might not be possible, we do provide a non-managed javax.resource.spi.ConnectionManager that allows us to acquire a javax.jms.Connection/javax.jms.Session etc (non-pooled, no auto XA enlistment) which in turn allows for system testing. Unit testing can also be achieved similar to the other Qpid Java sub projects. Also, this will allow us to add mock objects where appropriate without having to bring in an entire JEE environment.

Note, the JCA examples also provide a 'smoke test' like framework, however, more testing really needs to be added and maintained as was initially identified in the Qpid JCA review prior to the adapter being included in the Qpid project. 

While the Qpid JCA adapter only officially supports the C++ broker, a majority of the tests can be used with the Java Broker other than XA specific tests which can be excluded using the normal exclusion mechanism. 

To get things started I have added two tests classes:

qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java

This is a system test class with two tests that test the following JIRA's

https://issues.apache.org/jira/browse/QPID-3700 -- check for javax.jms.IllegalStateException for Session.commit() when a connection has been closed
https://issues.apache.org/jira/browse/QPID-3731 -- QpidRAMessage should call session.getSessionInternal() to determine if the underlying session has been closed prior to calling message.acknowledge().

Results:
[junit] Running org.apache.qpid.ra.QpidRAConnectionTest
[junit] Tests run: 2, Failures: 0, Errors: 0, Time elapsed: 2.75 sec


qpid/java/jca/src/test/java/org/apache/qpid/ra/QpidResourceAdapterTest.java

This is a unit test class that tests for the correct return value in the QpidResourceAdapter.getXAResources() call. Note, there is no corresponding JIRA for this test as it was implemented prior to the JCA being committed to the main repo.

Results:
 [junit] Running org.apache.qpid.ra.QpidResourceAdapterTest
 [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 0.129 sec
 

Note, while it might seem to be overkill to review a patch that simply adds testing to a sub-project, I had to touch a few files outside of the JCA tree and I thought a review was in order.


This addresses bug QPID-3751.
    https://issues.apache.org/jira/browse/QPID-3751


Diffs (updated)
-----

  /trunk/qpid/java/build.deps 1233097 
  /trunk/qpid/java/build.xml 1233097 
  /trunk/qpid/java/jca/example/.gitignore 1233097 
  /trunk/qpid/java/jca/src/test/java/org/apache/qpid/ra/QpidResourceAdapterTest.java PRE-CREATION 
  /trunk/qpid/java/systests/build.xml 1233097 
  /trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java PRE-CREATION 
  /trunk/qpid/java/.gitignore PRE-CREATION 

Diff: https://reviews.apache.org/r/3540/diff


Testing
-------

Java build and test-suite execution.


Thanks,

Weston