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