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 <[email protected]> wrote: > On Thu, May 16, 2013 at 4:35 AM, Phil Harvey <[email protected] > >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 >
