You are viewing a plain text version of this content. The canonical link for it is here.
Posted to proton@qpid.apache.org by Phil Harvey <ph...@philharveyonline.com> on 2013/05/10 16:04:49 UTC
proton-j Messenger tests failing on Jenkins (PROTON-295)
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-SVN&view=rev&rev=1480445>by
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
Re: proton-j Messenger tests failing on Jenkins (PROTON-295)
Posted by Phil Harvey <ph...@philharveyonline.com>.
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 <rh...@alum.mit.edu> wrote:
> On Thu, May 16, 2013 at 4:35 AM, Phil Harvey <phil@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)
Posted by Rafael Schloming <rh...@alum.mit.edu>.
On Thu, May 16, 2013 at 4:35 AM, Phil Harvey <ph...@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)
Posted by Phil Harvey <ph...@philharveyonline.com>.
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.
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.
Phil
On 13 May 2013 14:35, Rafael Schloming <rh...@alum.mit.edu> wrote:
> 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 <phil@philharveyonline.com
> >wrote:
>
> > 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-SVN&view=rev&rev=1480445
> >by
> > 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
> >
>
Re: proton-j Messenger tests failing on Jenkins (PROTON-295)
Posted by Rafael Schloming <rh...@alum.mit.edu>.
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 <ph...@philharveyonline.com>wrote:
> 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-SVN&view=rev&rev=1480445>by
> 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
>