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

Reply via email to