[ 
https://issues.apache.org/jira/browse/QPID-3751?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13189823#comment-13189823
 ] 

[email protected] commented on QPID-3751:
-----------------------------------------------------



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

Agreed will fix.


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

Agreed will fix.


bq.  On 2012-01-20 14:18:52, Keith Wall wrote:
bq.  > 
/trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java,
 line 75
bq.  > <https://reviews.apache.org/r/3540/diff/2/?file=69867#file69867line75>
bq.  >
bq.  >     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.
bq.  >     
bq.  >     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.
bq.  >     
bq.  >     
bq.  >     
bq.  >

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.


bq.  On 2012-01-20 14:18:52, Keith Wall wrote:
bq.  > 
/trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java,
 line 46
bq.  > <https://reviews.apache.org/r/3540/diff/2/?file=69867#file69867line46>
bq.  >
bq.  >     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:
bq.  >     
bq.  >     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:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/3540/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2012-01-19 00:32:22)
bq.  
bq.  
bq.  Review request for qpid, Gordon Sim, Robbie Gemmell, rajith attapattu, and 
Keith Wall.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  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.
bq.  
bq.  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. 
bq.  
bq.  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. 
bq.  
bq.  To get things started I have added two tests classes:
bq.  
bq.  
qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java
bq.  
bq.  This is a system test class with two tests that test the following JIRA's
bq.  
bq.  https://issues.apache.org/jira/browse/QPID-3700 -- check for 
javax.jms.IllegalStateException for Session.commit() when a connection has been 
closed
bq.  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().
bq.  
bq.  Results:
bq.  [junit] Running org.apache.qpid.ra.QpidRAConnectionTest
bq.  [junit] Tests run: 2, Failures: 0, Errors: 0, Time elapsed: 2.75 sec
bq.  
bq.  
bq.  qpid/java/jca/src/test/java/org/apache/qpid/ra/QpidResourceAdapterTest.java
bq.  
bq.  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.
bq.  
bq.  Results:
bq.   [junit] Running org.apache.qpid.ra.QpidResourceAdapterTest
bq.   [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 0.129 sec
bq.   
bq.  
bq.  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.
bq.  
bq.  
bq.  This addresses bug QPID-3751.
bq.      https://issues.apache.org/jira/browse/QPID-3751
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    /trunk/qpid/java/build.deps 1233097 
bq.    /trunk/qpid/java/build.xml 1233097 
bq.    /trunk/qpid/java/jca/example/.gitignore 1233097 
bq.    
/trunk/qpid/java/jca/src/test/java/org/apache/qpid/ra/QpidResourceAdapterTest.java
 PRE-CREATION 
bq.    /trunk/qpid/java/systests/build.xml 1233097 
bq.    
/trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAConnectionTest.java
 PRE-CREATION 
bq.    /trunk/qpid/java/.gitignore PRE-CREATION 
bq.  
bq.  Diff: https://reviews.apache.org/r/3540/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  Java build and test-suite execution.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Weston
bq.  
bq.


                
> Add Unit/System Testing to JCA Component
> ----------------------------------------
>
>                 Key: QPID-3751
>                 URL: https://issues.apache.org/jira/browse/QPID-3751
>             Project: Qpid
>          Issue Type: Improvement
>          Components: JCA
>         Environment: All OS platforms, all JEE supported platforms.
>            Reporter: Weston M. Price
>            Assignee: Weston M. Price
>             Fix For: 0.15, Future
>
>         Attachments: jca-testing.patch
>
>
> Currently the JCA component/code lacks any unit testing as well as any 
> integration testing. Further, the necessary configuration isn't in place to 
> allow for this to work as it stand today. Originally we were using our 
> internal TCK in conjunction with the JCA examples to test the adapter. It was 
> initially thought that the requirement of a running application server was a 
> 'must have' and introducing this requirement into the test framework would be 
> too disruptive. 
> I have had time (finally) to reconsider this. Being that we provide a 
> non-managed (i.e. non JEE) javax.resource.spi.ConnectionManager there is a 
> good amount of unit testing we can do, as we as being able to provide 
> contributions to the system tests. Further, for certain tests that require 
> app server functionality, simple mock objects can be useful. While we only 
> officially support the C++ broker for JCA, this shouldn't impede unit or 
> system testing as the Java broker is sufficient to at least cover a majority 
> of our test cases. For those future tests that actually require the C++ 
> broker we can use the excludes mechanism and address this need at that time.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:[email protected]

Reply via email to