I am sure the API suggests that those should be 'comparable'... the underlying problem is the use of == ...
Good spot
Should these go in AbstractIsotopeTest instead of just silent.IsotopeTest. It looks like the bug is in both.
Tried that, but compare() is not part of the IIsotope interface... and indeed, I think we need to copy these tests also into cdk.IsotopeTest ...
Okay no problem - I saw the method in all the implementations so presumed it was also in the interface.
You should also test larger masses:
Integer.valueOf(12) == Integer.valueOf(12) // true Integer.valueOf(260) == Integer.valueOf(260) // false
There's a missing dependency for silent:
[javac] /Users/johnmay/workspace/github/johnmay/cdk/build/src/test-silent/org/openscience/cdk/silent/IsotopeTest.java:121: error: cannot find symbol [javac] Isotope iso2 = new Isotope(Elements.CARBON);
Got it.
I think it just needs cdk-extra in test-silent.cdkdeps
On 14 Nov 2012, at 12:21, Egon Willighagen egonw@users.sf.net wrote:
Got it. patches:578 Unit tests for cdk.silent.Isotope.compare() Status: open Labels: silent Created: Sat Nov 10, 2012 09:30 PM UTC by Egon Willighagen Last Updated: Wed Nov 14, 2012 12:10 PM UTC Owner: nobody I am sure the API suggests that those should be 'comparable'... the underlying problem is the use of == ... Sent from sourceforge.net because you indicated interest in https://sourceforge.net/p/cdk/patches/578/ To unsubscribe from further messages, please visit https://sourceforge.net/auth/prefs/
patches:578 Unit tests for cdk.silent.Isotope.compare()
Status: open Labels: silent Created: Sat Nov 10, 2012 09:30 PM UTC by Egon Willighagen Last Updated: Wed Nov 14, 2012 12:10 PM UTC Owner: nobody
Sent from sourceforge.net because you indicated interest in https://sourceforge.net/p/cdk/patches/578/
To unsubscribe from further messages, please visit https://sourceforge.net/auth/prefs/
OK, here's the update patch:
Applied and pushed.
Log in to post a comment.
Good spot
Should these go in AbstractIsotopeTest instead of just silent.IsotopeTest. It looks like the bug is in both.
Tried that, but compare() is not part of the IIsotope interface... and indeed, I think we need to copy these tests also into cdk.IsotopeTest ...
Okay no problem - I saw the method in all the implementations so presumed it was also in the interface.
You should also test larger masses:
There's a missing dependency for silent:
Got it.
I think it just needs cdk-extra in test-silent.cdkdeps
On 14 Nov 2012, at 12:21, Egon Willighagen egonw@users.sf.net wrote:
OK, here's the update patch:
Applied and pushed.