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