Re: proton-j Messenger tests failing on Jenkins (PROTON-295)

2013-05-23 Thread Phil Harvey
I initially disabled the failing test because I didn't have time to exclude
it more selectively.  I've just committed a change under PROTON-315 to make
it skip iff we're using proton-j.

It is unfortunate that the Java Messenger implementation is falling so far
beind the C implementation.  Sadly, I don't have the bandwidth to address
this either.

Phil



On 16 May 2013 14:26, Rafael Schloming r...@alum.mit.edu wrote:

 On Thu, May 16, 2013 at 4:35 AM, Phil Harvey p...@philharveyonline.com
 wrote:

  Hi Rafi,
 
  I have Jira'd this test failure in PROTON-315 and commented out the
 failing
  test.  I have initially assigned the Jira to you but you may wish to
 canvas
  for people to assist with whatever Java Messenger changes are necessary.
  I'm not very familiar with this code personally, but would be happy to
 try
  to assist anyway.
 

 The test passes for the C impl and was added to check for a serious
 regression in the C impl, so disabling it entirely is obviously not ideal.
 I'd suggest just skipping it for the Java impl for now if you want the
 tests to pass. I will note however that it's not a test for a new feature,
 it's simply an additional test for an existing feature, and it covers what
 would likely be a common usage of that feature, so having the tests
 actually pass without that test included still isn't that great a
 situation. It's not really the same as a new feature that we have yet to
 add to the Java code, it really is a fairly basic malfunction.

 Put another way, this isn't a code change that caused existing tests to
 fail. From the Java perspective this is simply an additional test with no
 code changes, one that covers a pretty basic usage scenario.

 Overall there is a growing chunk of work that needs to be done to the Java
 Messenger impl to bring it up to parity with the C side both on features
 and bug fixes. I'm not sure what to do about it as we don't really have a
 party taking ownership of that code, and I don't currently have time to
 learn the ins and outs of it, at least not piecemeal for isolated patches.


  More generally, I believe we should not commit code to trunk that doesn't
  pass all the tests.  I just want to check that this is still our policy
 so
  please shout if you disagree.
 

 I certainly agree. I did actually check that the tests pass and I was under
 the impression they did, however upon further investigation my check was
 foiled by a number of things.

  - the config.sh script was never updated to find maven jars, because of
 this I could only run the tests via the build system rather than launching
 proton-test directly
  - the java stuff didn't build with cmake because of the bouncycastle
 dependency
  - the jni stuff *did* build (which I mistook for the non JNI tests
 passing)
  - the jni tests were falsely reported as passing, they seem to seg fault
 part way through, and cmake reports them passing

 I was only able to see the failure after downloading the bouncycastle
 dependency and updating the config.sh script to find the jars that cmake
 builds, however make test still does not appear to run a pure java profile,
 only the jni tests even when the pure java is actually successfully built.

 --Rafael



Re: proton-j Messenger tests failing on Jenkins (PROTON-295)

2013-05-16 Thread Rafael Schloming
On Thu, May 16, 2013 at 4:35 AM, Phil Harvey p...@philharveyonline.comwrote:

 Hi Rafi,

 I have Jira'd this test failure in PROTON-315 and commented out the failing
 test.  I have initially assigned the Jira to you but you may wish to canvas
 for people to assist with whatever Java Messenger changes are necessary.
 I'm not very familiar with this code personally, but would be happy to try
 to assist anyway.


The test passes for the C impl and was added to check for a serious
regression in the C impl, so disabling it entirely is obviously not ideal.
I'd suggest just skipping it for the Java impl for now if you want the
tests to pass. I will note however that it's not a test for a new feature,
it's simply an additional test for an existing feature, and it covers what
would likely be a common usage of that feature, so having the tests
actually pass without that test included still isn't that great a
situation. It's not really the same as a new feature that we have yet to
add to the Java code, it really is a fairly basic malfunction.

Put another way, this isn't a code change that caused existing tests to
fail. From the Java perspective this is simply an additional test with no
code changes, one that covers a pretty basic usage scenario.

Overall there is a growing chunk of work that needs to be done to the Java
Messenger impl to bring it up to parity with the C side both on features
and bug fixes. I'm not sure what to do about it as we don't really have a
party taking ownership of that code, and I don't currently have time to
learn the ins and outs of it, at least not piecemeal for isolated patches.


 More generally, I believe we should not commit code to trunk that doesn't
 pass all the tests.  I just want to check that this is still our policy so
 please shout if you disagree.


I certainly agree. I did actually check that the tests pass and I was under
the impression they did, however upon further investigation my check was
foiled by a number of things.

 - the config.sh script was never updated to find maven jars, because of
this I could only run the tests via the build system rather than launching
proton-test directly
 - the java stuff didn't build with cmake because of the bouncycastle
dependency
 - the jni stuff *did* build (which I mistook for the non JNI tests passing)
 - the jni tests were falsely reported as passing, they seem to seg fault
part way through, and cmake reports them passing

I was only able to see the failure after downloading the bouncycastle
dependency and updating the config.sh script to find the jars that cmake
builds, however make test still does not appear to run a pure java profile,
only the jni tests even when the pure java is actually successfully built.

--Rafael


Re: proton-j Messenger tests failing on Jenkins (PROTON-295)

2013-05-13 Thread Rafael Schloming
I took a look at the failure. The java Messenger impl seems to be hanging
on the new test before it even gets to the asserts that have anything to do
with the bug. I fixed the test harness so that it no longer hangs, but it
is still timing out and I'm not really familiar enough with the java
Messenger code to figure out why.

--Rafael



On Fri, May 10, 2013 at 10:04 AM, Phil Harvey p...@philharveyonline.comwrote:

 Hi,

 The following commit made on Wednesday is causing the proton-j Jenkins job
 to fail:

 *Revision 1480445
 http://svn.apache.org/viewcvs.cgi/?root=Apache-SVNview=revrev=1480445by
 rhs https://builds.apache.org/user/rhs/: *
 PROTON-295 https://issues.apache.org/jira/browse/PROTON-295: decoupled
 tracking of store entries from put/get of store entries, fixed tracking of
 incoming entries to start when they are returned via get rather than when
 they are read off of the wire

 See: https://builds.apache.org/view/M-R/view/Qpid/job/Qpid-proton-j/334/

 Is anyone who is familiar with this change available to fix it please?

 I recommend that folks subscribe to Jenkins build notifications emails so
 they are aware when their commits break the build.

 Thanks
 Phil



proton-j Messenger tests failing on Jenkins (PROTON-295)

2013-05-10 Thread Phil Harvey
Hi,

The following commit made on Wednesday is causing the proton-j Jenkins job
to fail:

*Revision 
1480445http://svn.apache.org/viewcvs.cgi/?root=Apache-SVNview=revrev=1480445by
rhs https://builds.apache.org/user/rhs/: *
PROTON-295 https://issues.apache.org/jira/browse/PROTON-295: decoupled
tracking of store entries from put/get of store entries, fixed tracking of
incoming entries to start when they are returned via get rather than when
they are read off of the wire

See: https://builds.apache.org/view/M-R/view/Qpid/job/Qpid-proton-j/334/

Is anyone who is familiar with this change available to fix it please?

I recommend that folks subscribe to Jenkins build notifications emails so
they are aware when their commits break the build.

Thanks
Phil