Menu
▾
▴
#927 Bug in IChemModel change event propagation
when a IMoleculeSet gets removed from a IChemModel, then a change in
that IMoleculeSet should not trigger a changeevent in the
IChemModel...
Right?
However, that is not what currently is implemented:
- @Test public void testStateChanged_ButNotAfterRemoval_MoleculeSet() {
- ChemObjectListenerImpl listener = new ChemObjectListenerImpl();
- IChemModel chemObject = (IChemModel)newChemObject();
- chemObject.addListener(listener);
+ - 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);
- // changing the set must not trigger a change event in the IChemModel
- molSet.addAtomContainer(chemObject.getBuilder().newMolecule());
- Assert.assertFalse(listener.changed);
- }
Attached patches are against cdk-1.2.x...
Is my assumption correct? Are these actual bugs?
Well, the cause of this is line 100 in ChemModel in the setMoleculeSet method:
this.setOfMolecules.addListener(this);
which means that the reference you have to the molecule set (molSet) still shares a reference to the listener.
I don't think that this behaviour makes sense, so your test is valid, yes.
I think your partches are correct and fix the bug
Argh... I am sleeping... these are the patches for the unit tests, and have both already been applied... I thought I had uploaded the fix too...
Sorry!!!
Will dig those up now...
The unit test was actually wrong... See commit message.
the patch
0001-Fixed-unit-tests-the-IChemModel.setFoo-null-should.patch
fixes the unit test, and can be applied on top of the other two which were applied earlier to cdk-1.2.x already.
Patch still fails
Applying: Fixed unit tests: the IChemModel.setFoo(null) should actually give a change event on the listener of the IChemModel
error: patch failed: src/test/org/openscience/cdk/interfaces/AbstractChemModelTest.java:228
error: src/test/org/openscience/cdk/interfaces/AbstractChemModelTest.java: patch does not apply
Patch failed at 0001.
Latest patches applied and pushed