Patches for fix change event propagation
|
View:
New views
14 Messages
—
Rating Filter:
Alert me
|
 |
Patches for fix change event propagation
by Egon Willighagen-5
::
Rate this Message:
Rajarshi,
could you please signoff and apply the patches for this bug report (they are reviewed and approved)? https://sourceforge.net/tracker/?func=detail&aid=2803595&group_id=20024&atid=120024 I believe this will get the number of unit Fails below that of the 1.2.3 release... It's a big step forward towards the 1.2.4 release. Thanx, Egon -- Post-doc @ Uppsala University Homepage: http://egonw.github.com/ Blog: http://chem-bla-ics.blogspot.com/ PubList: http://www.citeulike.org/user/egonw/tag/papers ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ Cdk-devel mailing list Cdk-devel@... https://lists.sourceforge.net/lists/listinfo/cdk-devel |
|
|
Re: Patches for fix change event propagation
by Rajarshi Guha-4
::
Rate this Message:
Can you update the patches against the latest 1.2.x - right now the
first patch fails against 1.2.x On Nov 1, 2009, at 8:40 AM, Egon Willighagen wrote: > Rajarshi, > > could you please signoff and apply the patches for this bug report > (they are reviewed and approved)? > > https://sourceforge.net/tracker/?func=detail&aid=2803595&group_id=20024&atid=120024 > > I believe this will get the number of unit Fails below that of the > 1.2.3 release... It's a big step forward towards the 1.2.4 release. > > Thanx, > > Egon > > -- > Post-doc @ Uppsala University > Homepage: http://egonw.github.com/ > Blog: http://chem-bla-ics.blogspot.com/ > PubList: http://www.citeulike.org/user/egonw/tag/papers > > ------------------------------------------------------------------------------ > Come build with us! The BlackBerry(R) Developer Conference in SF, CA > is the only developer event you need to attend this year. Jumpstart > your > developing skills, take BlackBerry mobile applications to market and > stay > ahead of the curve. Join us from November 9 - 12, 2009. Register now! > http://p.sf.net/sfu/devconference > _______________________________________________ > Cdk-devel mailing list > Cdk-devel@... > https://lists.sourceforge.net/lists/listinfo/cdk-devel ---------------------------------------------------- Rajarshi Guha | NIH Chemical Genomics Center http://www.rguha.net | http://ncgc.nih.gov ---------------------------------------------------- Mathematics is a game played according to certain simple rules with meaningless marks on paper. -- David Hilbert ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ Cdk-devel mailing list Cdk-devel@... https://lists.sourceforge.net/lists/listinfo/cdk-devel |
|
|
Re: Patches for fix change event propagation
by Egon Willighagen-5
::
Rate this Message:
On Sun, Nov 1, 2009 at 3:16 PM, Rajarshi Guha <rajarshi.guha@...> wrote:
> Can you update the patches against the latest 1.2.x - right now the first > patch fails against 1.2.x Sorry! These patches are just the unit tests, and already applied... I thought I had attached the fixes too... :( Egon -- Post-doc @ Uppsala University Homepage: http://egonw.github.com/ Blog: http://chem-bla-ics.blogspot.com/ PubList: http://www.citeulike.org/user/egonw/tag/papers ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ Cdk-devel mailing list Cdk-devel@... https://lists.sourceforge.net/lists/listinfo/cdk-devel |
|
|
Re: Patches for fix change event propagation
by Egon Willighagen-5
::
Rate this Message:
On Sun, Nov 1, 2009 at 3:30 PM, Egon Willighagen
<egon.willighagen@...> wrote: > On Sun, Nov 1, 2009 at 3:16 PM, Rajarshi Guha <rajarshi.guha@...> wrote: >> Can you update the patches against the latest 1.2.x - right now the first >> patch fails against 1.2.x > > Sorry! > > These patches are just the unit tests, and already applied... I > thought I had attached the fixes too... :( And while the analysis was OK, the unit tests were actually wrong... Please see the attached patch. Egon -- Post-doc @ Uppsala University Homepage: http://egonw.github.com/ Blog: http://chem-bla-ics.blogspot.com/ PubList: http://www.citeulike.org/user/egonw/tag/papers [0001-Fixed-unit-tests-the-IChemModel.setFoo-null-should.patch] From 888b152d04aeec5fe5a447f9e479a9b3275d098a Mon Sep 17 00:00:00 2001 From: Egon Willighagen <egonw@...> Date: Sun, 1 Nov 2009 16:05:53 +0100 Subject: [PATCH] Fixed unit tests: the IChemModel.setFoo(null) should actually give a change event on the listener of the IChemModel --- .../cdk/interfaces/AbstractChemModelTest.java | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/org/openscience/cdk/interfaces/AbstractChemModelTest.java b/src/test/org/openscience/cdk/interfaces/AbstractChemModelTest.java index 4a6403b..9ea7a59 100644 --- a/src/test/org/openscience/cdk/interfaces/AbstractChemModelTest.java +++ b/src/test/org/openscience/cdk/interfaces/AbstractChemModelTest.java @@ -211,10 +211,10 @@ public abstract class AbstractChemModelTest extends AbstractChemObjectTest { IMoleculeSet molSet = chemObject.getBuilder().newMoleculeSet(); chemObject.setMoleculeSet(molSet); Assert.assertTrue(listener.changed); - // reset the listener - listener.reset(); Assert.assertFalse(listener.changed); // remove the set from the IChemModel chemObject.setMoleculeSet(null); + // reset the listener + listener.reset(); Assert.assertFalse(listener.changed); // changing the set must *not* trigger a change event in the IChemModel molSet.addAtomContainer(chemObject.getBuilder().newMolecule()); Assert.assertFalse(listener.changed); @@ -228,10 +228,10 @@ public abstract class AbstractChemModelTest extends AbstractChemObjectTest { IReactionSet reactionSet = chemObject.getBuilder().newReactionSet(); chemObject.setReactionSet(reactionSet); Assert.assertTrue(listener.changed); - // reset the listener - listener.reset(); Assert.assertFalse(listener.changed); // remove the set from the IChemModel chemObject.setReactionSet(null); + // reset the listener + listener.reset(); Assert.assertFalse(listener.changed); // changing the set must *not* trigger a change event in the IChemModel reactionSet.addReaction(chemObject.getBuilder().newReaction()); Assert.assertTrue(listener.changed); @@ -245,10 +245,10 @@ public abstract class AbstractChemModelTest extends AbstractChemObjectTest { IRingSet ringSet = chemObject.getBuilder().newRingSet(); chemObject.setRingSet(ringSet); Assert.assertTrue(listener.changed); - // reset the listener - listener.reset(); Assert.assertFalse(listener.changed); // remove the set from the IChemModel chemObject.setRingSet(null); + // reset the listener + listener.reset(); Assert.assertFalse(listener.changed); // changing the set must *not* trigger a change event in the IChemModel ringSet.addAtomContainer(chemObject.getBuilder().newRing()); Assert.assertTrue(listener.changed); -- 1.6.0.4 ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ Cdk-devel mailing list Cdk-devel@... https://lists.sourceforge.net/lists/listinfo/cdk-devel |
|
|
Re: Patches for fix change event propagation
by Rajarshi Guha-4
::
Rate this Message:
Patch still fails on latest cdk-1.2.x
On Nov 1, 2009, at 10:07 AM, Egon Willighagen wrote: > On Sun, Nov 1, 2009 at 3:30 PM, Egon Willighagen > <egon.willighagen@...> wrote: >> On Sun, Nov 1, 2009 at 3:16 PM, Rajarshi Guha <rajarshi.guha@... >> > wrote: >>> Can you update the patches against the latest 1.2.x - right now >>> the first >>> patch fails against 1.2.x >> >> Sorry! >> >> These patches are just the unit tests, and already applied... I >> thought I had attached the fixes too... :( > > And while the analysis was OK, the unit tests were actually wrong... > > Please see the attached patch. > > Egon > > > -- > Post-doc @ Uppsala University > Homepage: http://egonw.github.com/ > Blog: http://chem-bla-ics.blogspot.com/ > PubList: http://www.citeulike.org/user/egonw/tag/papers > <0001-Fixed-unit-tests-the-IChemModel.setFoo-null-should.patch> ---------------------------------------------------- Rajarshi Guha | NIH Chemical Genomics Center http://www.rguha.net | http://ncgc.nih.gov ---------------------------------------------------- All the evidence concerning the universe has not yet been collected, so there's still hope. ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ Cdk-devel mailing list Cdk-devel@... https://lists.sourceforge.net/lists/listinfo/cdk-devel |
|
|
Re: Patches for fix change event propagation
by Egon Willighagen-5
::
Rate this Message:
On Sun, Nov 1, 2009 at 4:17 PM, Rajarshi Guha <rajarshi.guha@...> wrote:
> Patch still fails on latest cdk-1.2.x OK, working on it... spotted another problem in the unit test... so much for reviewing, and egonw--, egonw-- :( Anyways... closing in... and actually found that there was another instance of the same problem in setCrystal() for which there was not even a unit test... and will write one for that method too... BTW, thanx for looking at all those other patches! Very much appreciated... Egon -- Post-doc @ Uppsala University Homepage: http://egonw.github.com/ Blog: http://chem-bla-ics.blogspot.com/ PubList: http://www.citeulike.org/user/egonw/tag/papers ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ Cdk-devel mailing list Cdk-devel@... https://lists.sourceforge.net/lists/listinfo/cdk-devel |
|
|
Re: Patches for fix change event propagation
by Rajarshi Guha-4
::
Rate this Message:
On Nov 1, 2009, at 10:22 AM, Egon Willighagen wrote: > On Sun, Nov 1, 2009 at 4:17 PM, Rajarshi Guha > <rajarshi.guha@...> wrote: >> Patch still fails on latest cdk-1.2.x > > OK, working on it... spotted another problem in the unit test... so > much for reviewing, and egonw--, egonw-- :( > > Anyways... closing in... and actually found that there was another > instance of the same problem in setCrystal() for which there was not > even a unit test... and will write one for that method too... OK > BTW, thanx for looking at all those other patches! Very much > appreciated... No problem - hopefully, one day the periodic table patches will get committed :) ---------------------------------------------------- Rajarshi Guha | NIH Chemical Genomics Center http://www.rguha.net | http://ncgc.nih.gov ---------------------------------------------------- Q: What do you get when you cross a mosquito with a mountain climber? A: Nothing. You can't cross a vector with a scaler. ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ Cdk-devel mailing list Cdk-devel@... https://lists.sourceforge.net/lists/listinfo/cdk-devel |
|
|
Re: Patches for fix change event propagation
by Egon Willighagen-5
::
Rate this Message:
On Sun, Nov 1, 2009 at 4:23 PM, Rajarshi Guha <rajarshi.guha@...> wrote:
> No problem - hopefully, one day the periodic table patches will get committed :) It hasn't?? Oh... I think we need to think about a better way to keep track of patches... the current system does seem to work so well, and relies on manually pinging... then again, that's the job of a journal editor really too... making sure that reviewers reply... but we do not have someone in that position (yet)... After I finalized the fixes for the change event stuff, I'll try to have a look at the PT patches... What was the tracker number again? Egon -- Post-doc @ Uppsala University Homepage: http://egonw.github.com/ Blog: http://chem-bla-ics.blogspot.com/ PubList: http://www.citeulike.org/user/egonw/tag/papers ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ Cdk-devel mailing list Cdk-devel@... https://lists.sourceforge.net/lists/listinfo/cdk-devel |
|
|
Re: Patches for fix change event propagation
by Egon Willighagen-5
::
Rate this Message:
On Sun, Nov 1, 2009 at 4:22 PM, Egon Willighagen
<egon.willighagen@...> wrote: > On Sun, Nov 1, 2009 at 4:17 PM, Rajarshi Guha <rajarshi.guha@...> wrote: > Anyways... closing in... and actually found that there was another > instance of the same problem in setCrystal() for which there was not > even a unit test... and will write one for that method too... Rajarshi, please find four patches against the current CDK 1.2.x... 0001-Fixed-unit-tests-the-IChemModel.setFoo-null-should.patch Fixes the unit tests to make it test the change event are the correct time. 0002-Fixed-propagation-of-change-events-to-IChemModel-whe.patch Actual fix for the bug: proper (de)registration of listeners. 0003-Added-missing-unit-tests-for-IChemModel-event-propag.patch Test for event propagation for the IChemModel.ICrystal field... 0004-Overwrite-unit-tests-because-there-are-no-change-ev.patch Fixed for NNChemModelTest, to overwrite the event propagation testing in the AbstractChemModelTest, as the NoNotification implementation does not notify at all... Egon -- Post-doc @ Uppsala University Homepage: http://egonw.github.com/ Blog: http://chem-bla-ics.blogspot.com/ PubList: http://www.citeulike.org/user/egonw/tag/papers ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ Cdk-devel mailing list Cdk-devel@... https://lists.sourceforge.net/lists/listinfo/cdk-devel |
|
|
Re: Patches for fix change event propagation
by Rajarshi Guha-4
::
Rate this Message:
On Nov 1, 2009, at 10:26 AM, Egon Willighagen wrote: > On Sun, Nov 1, 2009 at 4:23 PM, Rajarshi Guha > <rajarshi.guha@...> wrote: >> No problem - hopefully, one day the periodic table patches will get >> committed :) > > It hasn't?? Oh... > > I think we need to think about a better way to keep track of > patches... the current system does seem to work so well, and relies on > manually pinging... then again, that's the job of a journal editor > really too... making sure that reviewers reply... but we do not have > someone in that position (yet)... Hmm, I'm not sure how much more a different tracker would help - the bottleneck generally seems to be time and number of reviewers. > After I finalized the fixes for the change event stuff, I'll try to > have a look at the PT patches... What was the tracker number again? 2688082 ---------------------------------------------------- Rajarshi Guha | NIH Chemical Genomics Center http://www.rguha.net | http://ncgc.nih.gov ---------------------------------------------------- 667: The neighbor of the beast. ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ Cdk-devel mailing list Cdk-devel@... https://lists.sourceforge.net/lists/listinfo/cdk-devel |
|
|
Re: Patches for fix change event propagation
by Egon Willighagen-5
::
Rate this Message:
On Sun, Nov 1, 2009 at 4:52 PM, Rajarshi Guha <rajarshi.guha@...> wrote:
> On Nov 1, 2009, at 10:26 AM, Egon Willighagen wrote: >> I think we need to think about a better way to keep track of >> patches... the current system does seem to work so well, and relies on >> manually pinging... then again, that's the job of a journal editor >> really too... making sure that reviewers reply... but we do not have >> someone in that position (yet)... > > Hmm, I'm not sure how much more a different tracker would help - the > bottleneck generally seems to be time and number of reviewers. Agreed. 'a better way' was not so much about a different tracker, but actually finding people to track the patches... :) >> After I finalized the fixes for the change event stuff, I'll try to >> have a look at the PT patches... What was the tracker number again? > > 2688082 OK, with the patches for the event passing sent, I'll look at it now... Egon -- Post-doc @ Uppsala University Homepage: http://egonw.github.com/ Blog: http://chem-bla-ics.blogspot.com/ PubList: http://www.citeulike.org/user/egonw/tag/papers ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ Cdk-devel mailing list Cdk-devel@... https://lists.sourceforge.net/lists/listinfo/cdk-devel |
|
|
Re: Patches for fix change event propagation
by Rajarshi Guha-4
::
Rate this Message:
No patches attached?
On Nov 1, 2009, at 10:48 AM, Egon Willighagen wrote: > On Sun, Nov 1, 2009 at 4:22 PM, Egon Willighagen > <egon.willighagen@...> wrote: >> On Sun, Nov 1, 2009 at 4:17 PM, Rajarshi Guha <rajarshi.guha@... >> > wrote: >> Anyways... closing in... and actually found that there was another >> instance of the same problem in setCrystal() for which there was not >> even a unit test... and will write one for that method too... > > Rajarshi, > > please find four patches against the current CDK 1.2.x... > > 0001-Fixed-unit-tests-the-IChemModel.setFoo-null-should.patch > > Fixes the unit tests to make it test the change event are the > correct time. > > 0002-Fixed-propagation-of-change-events-to-IChemModel-whe.patch > > Actual fix for the bug: proper (de)registration of listeners. > > 0003-Added-missing-unit-tests-for-IChemModel-event-propag.patch > > Test for event propagation for the IChemModel.ICrystal field... > > 0004-Overwrite-unit-tests-because-there-are-no-change-ev.patch > > Fixed for NNChemModelTest, to overwrite the event propagation testing > in the AbstractChemModelTest, as the NoNotification implementation > does not notify at all... > > Egon > > -- > Post-doc @ Uppsala University > Homepage: http://egonw.github.com/ > Blog: http://chem-bla-ics.blogspot.com/ > PubList: http://www.citeulike.org/user/egonw/tag/papers ---------------------------------------------------- Rajarshi Guha | NIH Chemical Genomics Center http://www.rguha.net | http://ncgc.nih.gov ---------------------------------------------------- A computer lets you make more mistakes faster than any other invention, with the possible exceptions of handguns and Tequilla. -- Mitch Ratcliffe ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ Cdk-devel mailing list Cdk-devel@... https://lists.sourceforge.net/lists/listinfo/cdk-devel |
|
|
Re: Patches for fix change event propagation
by Egon Willighagen-5
::
Rate this Message:
On Sun, Nov 1, 2009 at 5:10 PM, Rajarshi Guha <rajarshi.guha@...> wrote:
> No patches attached? Mmmm.... Egon -- Post-doc @ Uppsala University Homepage: http://egonw.github.com/ Blog: http://chem-bla-ics.blogspot.com/ PubList: http://www.citeulike.org/user/egonw/tag/papers [0001-Fixed-unit-tests-the-IChemModel.setFoo-null-should.patch] From 219986895e8002e804fb081acdbf7ea90b504061 Mon Sep 17 00:00:00 2001 From: Egon Willighagen <egonw@...> Date: Sun, 1 Nov 2009 16:05:53 +0100 Subject: [PATCH] Fixed unit tests: the IChemModel.setFoo(null) should actually give a change event on the listener of the IChemModel, and not after unregistering of the Foo object. --- .../cdk/interfaces/AbstractChemModelTest.java | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/org/openscience/cdk/interfaces/AbstractChemModelTest.java b/src/test/org/openscience/cdk/interfaces/AbstractChemModelTest.java index 74df251..2595b88 100644 --- a/src/test/org/openscience/cdk/interfaces/AbstractChemModelTest.java +++ b/src/test/org/openscience/cdk/interfaces/AbstractChemModelTest.java @@ -211,10 +211,10 @@ public abstract class AbstractChemModelTest extends AbstractChemObjectTest { IMoleculeSet molSet = chemObject.getBuilder().newMoleculeSet(); chemObject.setMoleculeSet(molSet); Assert.assertTrue(listener.changed); - // reset the listener - listener.reset(); Assert.assertFalse(listener.changed); // remove the set from the IChemModel chemObject.setMoleculeSet(null); + // reset the listener + listener.reset(); Assert.assertFalse(listener.changed); // changing the set must *not* trigger a change event in the IChemModel molSet.addAtomContainer(chemObject.getBuilder().newMolecule()); Assert.assertFalse(listener.changed); @@ -228,10 +228,10 @@ public abstract class AbstractChemModelTest extends AbstractChemObjectTest { IReactionSet reactionSet = chemObject.getBuilder().newReactionSet(); chemObject.setReactionSet(reactionSet); Assert.assertTrue(listener.changed); - // reset the listener - listener.reset(); Assert.assertFalse(listener.changed); // remove the set from the IChemModel chemObject.setReactionSet(null); + // reset the listener + listener.reset(); Assert.assertFalse(listener.changed); // changing the set must *not* trigger a change event in the IChemModel reactionSet.addReaction(chemObject.getBuilder().newReaction()); Assert.assertFalse(listener.changed); @@ -245,10 +245,10 @@ public abstract class AbstractChemModelTest extends AbstractChemObjectTest { IRingSet ringSet = chemObject.getBuilder().newRingSet(); chemObject.setRingSet(ringSet); Assert.assertTrue(listener.changed); - // reset the listener - listener.reset(); Assert.assertFalse(listener.changed); // remove the set from the IChemModel chemObject.setRingSet(null); + // reset the listener + listener.reset(); Assert.assertFalse(listener.changed); // changing the set must *not* trigger a change event in the IChemModel ringSet.addAtomContainer(chemObject.getBuilder().newRing()); Assert.assertFalse(listener.changed); -- 1.6.0.4 [0002-Fixed-propagation-of-change-events-to-IChemModel-whe.patch] From 1e659d8aa3e302cd05a43e2eaf84c1495e3bfc41 Mon Sep 17 00:00:00 2001 From: Egon Willighagen <egonw@...> Date: Sun, 1 Nov 2009 15:43:37 +0100 Subject: [PATCH] Fixed propagation of change events to IChemModel when modifications are made in child IChemObjects --- src/main/org/openscience/cdk/ChemModel.java | 23 ++++++++++++++++++----- 1 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/main/org/openscience/cdk/ChemModel.java b/src/main/org/openscience/cdk/ChemModel.java index a3596cf..50b38ad 100644 --- a/src/main/org/openscience/cdk/ChemModel.java +++ b/src/main/org/openscience/cdk/ChemModel.java @@ -96,8 +96,11 @@ public class ChemModel extends ChemObject implements Serializable, IChemModel, I */ public void setMoleculeSet(IMoleculeSet setOfMolecules) { + if (this.setOfMolecules != null) + this.setOfMolecules.removeListener(this); this.setOfMolecules = setOfMolecules; - this.setOfMolecules.addListener(this); + if (this.setOfMolecules != null) + this.setOfMolecules.addListener(this); notifyChanged(); } @@ -124,7 +127,11 @@ public class ChemModel extends ChemObject implements Serializable, IChemModel, I */ public void setRingSet(IRingSet ringSet) { + if (this.ringSet != null) + this.ringSet.removeListener(this); this.ringSet = ringSet; + if (this.ringSet != null) + this.ringSet.addListener(this); notifyChanged(); } @@ -147,9 +154,12 @@ public class ChemModel extends ChemObject implements Serializable, IChemModel, I * @see #getCrystal */ public void setCrystal(ICrystal crystal) { + if (this.crystal != null) + this.crystal.removeListener(this); this.crystal = crystal; - this.crystal.addListener(this); - notifyChanged(); + if (this.crystal != null) + this.crystal.addListener(this); + notifyChanged(); } /** @@ -171,9 +181,12 @@ public class ChemModel extends ChemObject implements Serializable, IChemModel, I * @see #getReactionSet */ public void setReactionSet(IReactionSet sor) { + if (this.setOfReactions != null) + this.setOfReactions.removeListener(this); this.setOfReactions = sor; - this.setOfReactions.addListener(this); - notifyChanged(); + if (this.setOfReactions != null) + this.setOfReactions.addListener(this); + notifyChanged(); } /** -- 1.6.0.4 [0003-Added-missing-unit-tests-for-IChemModel-event-propag.patch] From 6546ab38de13e703991f86f701d98c9ebc79bd81 Mon Sep 17 00:00:00 2001 From: Egon Willighagen <egonw@...> Date: Sun, 1 Nov 2009 16:38:55 +0100 Subject: [PATCH] Added missing unit tests for IChemModel event propagation for the ICrystal field --- .../cdk/interfaces/AbstractChemModelTest.java | 32 ++++++++++++++++++++ 1 files changed, 32 insertions(+), 0 deletions(-) diff --git a/src/test/org/openscience/cdk/interfaces/AbstractChemModelTest.java b/src/test/org/openscience/cdk/interfaces/AbstractChemModelTest.java index 2595b88..6f764bf 100644 --- a/src/test/org/openscience/cdk/interfaces/AbstractChemModelTest.java +++ b/src/test/org/openscience/cdk/interfaces/AbstractChemModelTest.java @@ -158,6 +158,21 @@ public abstract class AbstractChemModelTest extends AbstractChemObjectTest { Assert.assertTrue(listener.changed); } + @Test public void testStateChanged_EventPropagation_Crystal() { + ChemObjectListenerImpl listener = new ChemObjectListenerImpl(); + IChemModel chemObject = (IChemModel)newChemObject(); + chemObject.addListener(listener); + + ICrystal crystal = chemObject.getBuilder().newCrystal(); + chemObject.setCrystal(crystal); + Assert.assertTrue(listener.changed); + // reset the listener + listener.reset(); Assert.assertFalse(listener.changed); + // changing the set should trigger a change event in the IChemModel + crystal.add(chemObject.getBuilder().newMolecule()); + Assert.assertTrue(listener.changed); + } + @Test public void testStateChanged_EventPropagation_MoleculeSet() { ChemObjectListenerImpl listener = new ChemObjectListenerImpl(); IChemModel chemObject = (IChemModel)newChemObject(); @@ -203,6 +218,23 @@ public abstract class AbstractChemModelTest extends AbstractChemObjectTest { Assert.assertTrue(listener.changed); } + @Test public void testStateChanged_ButNotAfterRemoval_Crystal() { + ChemObjectListenerImpl listener = new ChemObjectListenerImpl(); + IChemModel chemObject = (IChemModel)newChemObject(); + chemObject.addListener(listener); + + ICrystal crystal = chemObject.getBuilder().newCrystal(); + chemObject.setCrystal(crystal); + Assert.assertTrue(listener.changed); + // remove the set from the IChemModel + chemObject.setCrystal(null); + // reset the listener + listener.reset(); Assert.assertFalse(listener.changed); + // changing the set must *not* trigger a change event in the IChemModel + crystal.add(chemObject.getBuilder().newMolecule()); + Assert.assertFalse(listener.changed); + } + @Test public void testStateChanged_ButNotAfterRemoval_MoleculeSet() { ChemObjectListenerImpl listener = new ChemObjectListenerImpl(); IChemModel chemObject = (IChemModel)newChemObject(); -- 1.6.0.4 [0004-Overwrite-unit-tests-because-there-are-no-change-ev.patch] From ca15fd268ecb217f7042c73891891ef581a539b5 Mon Sep 17 00:00:00 2001 From: Egon Willighagen <egonw@...> Date: Sun, 1 Nov 2009 16:44:20 +0100 Subject: [PATCH] Overwrite unit tests, because there are no change events passed around at all for the NoNotification interface implementations --- .../openscience/cdk/nonotify/NNChemModelTest.java | 132 ++++++++++++++++++++ 1 files changed, 132 insertions(+), 0 deletions(-) diff --git a/src/test/org/openscience/cdk/nonotify/NNChemModelTest.java b/src/test/org/openscience/cdk/nonotify/NNChemModelTest.java index 4a94921..37f57dd 100644 --- a/src/test/org/openscience/cdk/nonotify/NNChemModelTest.java +++ b/src/test/org/openscience/cdk/nonotify/NNChemModelTest.java @@ -30,6 +30,10 @@ import org.junit.Test; import org.openscience.cdk.interfaces.AbstractChemModelTest; import org.openscience.cdk.interfaces.IChemModel; import org.openscience.cdk.interfaces.IChemObject; +import org.openscience.cdk.interfaces.ICrystal; +import org.openscience.cdk.interfaces.IMoleculeSet; +import org.openscience.cdk.interfaces.IReactionSet; +import org.openscience.cdk.interfaces.IRingSet; import org.openscience.cdk.interfaces.ITestObjectBuilder; /** @@ -78,4 +82,132 @@ public class NNChemModelTest extends AbstractChemModelTest { @Test public void testSetNotification_true() { NNChemObjectTestHelper.testSetNotification_true(newChemObject()); } + + @Test public void testStateChanged_EventPropagation_Crystal() { + NNChemObjectListener listener = new NNChemObjectListener(); + IChemModel chemObject = (IChemModel)newChemObject(); + chemObject.addListener(listener); + + ICrystal crystal = chemObject.getBuilder().newCrystal(); + chemObject.setCrystal(crystal); + Assert.assertFalse(listener.getChanged()); + // reset the listener + listener.reset(); Assert.assertFalse(listener.getChanged()); + // changing the set should trigger a change event in the IChemModel + crystal.add(chemObject.getBuilder().newMolecule()); + Assert.assertFalse(listener.getChanged()); + } + + @Test public void testStateChanged_EventPropagation_MoleculeSet() { + NNChemObjectListener listener = new NNChemObjectListener(); + IChemModel chemObject = (IChemModel)newChemObject(); + chemObject.addListener(listener); + + IMoleculeSet molSet = chemObject.getBuilder().newMoleculeSet(); + chemObject.setMoleculeSet(molSet); + Assert.assertFalse(listener.getChanged()); + // reset the listener + listener.reset(); Assert.assertFalse(listener.getChanged()); + // changing the set should trigger a change event in the IChemModel + molSet.addAtomContainer(chemObject.getBuilder().newMolecule()); + Assert.assertFalse(listener.getChanged()); + } + + @Test public void testStateChanged_EventPropagation_ReactionSet() { + NNChemObjectListener listener = new NNChemObjectListener(); + IChemModel chemObject = (IChemModel)newChemObject(); + chemObject.addListener(listener); + + IReactionSet reactionSet = chemObject.getBuilder().newReactionSet(); + chemObject.setReactionSet(reactionSet); + Assert.assertFalse(listener.getChanged()); + // reset the listener + listener.reset(); Assert.assertFalse(listener.getChanged()); + // changing the set should trigger a change event in the IChemModel + reactionSet.addReaction(chemObject.getBuilder().newReaction()); + Assert.assertFalse(listener.getChanged()); + } + + @Test public void testStateChanged_EventPropagation_RingSet() { + NNChemObjectListener listener = new NNChemObjectListener(); + IChemModel chemObject = (IChemModel)newChemObject(); + chemObject.addListener(listener); + + IRingSet ringSet = chemObject.getBuilder().newRingSet(); + chemObject.setRingSet(ringSet); + Assert.assertFalse(listener.getChanged()); + // reset the listener + listener.reset(); Assert.assertFalse(listener.getChanged()); + // changing the set should trigger a change event in the IChemModel + ringSet.addAtomContainer(chemObject.getBuilder().newRing()); + Assert.assertFalse(listener.getChanged()); + } + + @Test public void testStateChanged_ButNotAfterRemoval_Crystal() { + NNChemObjectListener listener = new NNChemObjectListener(); + IChemModel chemObject = (IChemModel)newChemObject(); + chemObject.addListener(listener); + + ICrystal crystal = chemObject.getBuilder().newCrystal(); + chemObject.setCrystal(crystal); + Assert.assertFalse(listener.getChanged()); + // remove the set from the IChemModel + chemObject.setCrystal(null); + // reset the listener + listener.reset(); Assert.assertFalse(listener.getChanged()); + // changing the set must *not* trigger a change event in the IChemModel + crystal.add(chemObject.getBuilder().newMolecule()); + Assert.assertFalse(listener.getChanged()); + } + + @Test public void testStateChanged_ButNotAfterRemoval_MoleculeSet() { + NNChemObjectListener listener = new NNChemObjectListener(); + IChemModel chemObject = (IChemModel)newChemObject(); + chemObject.addListener(listener); + + IMoleculeSet molSet = chemObject.getBuilder().newMoleculeSet(); + chemObject.setMoleculeSet(molSet); + Assert.assertFalse(listener.getChanged()); + // remove the set from the IChemModel + chemObject.setMoleculeSet(null); + // reset the listener + listener.reset(); Assert.assertFalse(listener.getChanged()); + // changing the set must *not* trigger a change event in the IChemModel + molSet.addAtomContainer(chemObject.getBuilder().newMolecule()); + Assert.assertFalse(listener.getChanged()); + } + + @Test public void testStateChanged_ButNotAfterRemoval_ReactionSet() { + NNChemObjectListener listener = new NNChemObjectListener(); + IChemModel chemObject = (IChemModel)newChemObject(); + chemObject.addListener(listener); + + IReactionSet reactionSet = chemObject.getBuilder().newReactionSet(); + chemObject.setReactionSet(reactionSet); + Assert.assertFalse(listener.getChanged()); + // remove the set from the IChemModel + chemObject.setReactionSet(null); + // reset the listener + listener.reset(); Assert.assertFalse(listener.getChanged()); + // changing the set must *not* trigger a change event in the IChemModel + reactionSet.addReaction(chemObject.getBuilder().newReaction()); + Assert.assertFalse(listener.getChanged()); + } + + @Test public void testStateChanged_ButNotAfterRemoval_RingSet() { + NNChemObjectListener listener = new NNChemObjectListener(); + IChemModel chemObject = (IChemModel)newChemObject(); + chemObject.addListener(listener); + + IRingSet ringSet = chemObject.getBuilder().newRingSet(); + chemObject.setRingSet(ringSet); + Assert.assertFalse(listener.getChanged()); + // remove the set from the IChemModel + chemObject.setRingSet(null); + // reset the listener + listener.reset(); Assert.assertFalse(listener.getChanged()); + // changing the set must *not* trigger a change event in the IChemModel + ringSet.addAtomContainer(chemObject.getBuilder().newRing()); + Assert.assertFalse(listener.getChanged()); + } } -- 1.6.0.4 ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ Cdk-devel mailing list Cdk-devel@... https://lists.sourceforge.net/lists/listinfo/cdk-devel |
|
|
Re: Patches for fix change event propagation
by Rajarshi Guha-4
::
Rate this Message:
OK, all applied and pushed to github. Closing the report
On Nov 1, 2009, at 11:11 AM, Egon Willighagen wrote: > On Sun, Nov 1, 2009 at 5:10 PM, Rajarshi Guha > <rajarshi.guha@...> wrote: >> No patches attached? > > Mmmm.... > > Egon > > -- > Post-doc @ Uppsala University > Homepage: http://egonw.github.com/ > Blog: http://chem-bla-ics.blogspot.com/ > PubList: http://www.citeulike.org/user/egonw/tag/papers > <0001-Fixed-unit-tests-the-IChemModel.setFoo-null-should.patch><0002- > Fixed-propagation-of-change-events-to-IChemModel-whe.patch><0003- > Added-missing-unit-tests-for-IChemModel-event-propag.patch><0004- > Overwrite-unit-tests-because-there-are-no-change-ev.patch> ---------------------------------------------------- Rajarshi Guha | NIH Chemical Genomics Center http://www.rguha.net | http://ncgc.nih.gov ---------------------------------------------------- "A fractal is by definition a set for which the Hausdorff Besicovitch dimension strictly exceeds the topological dimension." -- Mandelbrot, "The Fractal Geometry of Nature" ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ Cdk-devel mailing list Cdk-devel@... https://lists.sourceforge.net/lists/listinfo/cdk-devel |
| Free embeddable forum powered by Nabble | Forum Help |
