From: SourceForge.net <no...@so...> - 2012-04-23 07:33:03
|
Patches item #3519885, was opened at 2012-04-20 09:33 Message generated for change (Comment added) made by danielszisz You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3519885&group_id=20024 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: master Group: Needs Review Status: Open Resolution: None Priority: 5 Private: No Submitted By: Daniel Szisz (danielszisz) Assigned to: Daniel Szisz (danielszisz) Summary: New AtomPlacer3D test class Initial Comment: Hi ! I made some new tests that tests the not-yet-tested functionalities of AtomPlacer3D. It is intended to go into master. I also post a modification patch to the first patch. Hopefully these tests will reveal some bugs in ModelBuilder3D. ---------------------------------------------------------------------- >Comment By: Daniel Szisz (danielszisz) Date: 2012-04-23 00:33 Message: Corrected the test file based on the suggestions of Egon. ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2012-04-22 01:59 Message: (Otherwise, great work you are doing!) ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2012-04-22 01:57 Message: Daniel, some comments: 1. can you add the copyright header? 2. in code like: + @Test + public void testAllHeavyAtomsPlaced_IAtomContainer() { + AtomPlacer3D atmplacer=new AtomPlacer3D(); + List<IAtomContainer> molecules = new ArrayList<IAtomContainer>(); + molecules.add(MoleculeFactory.makeAlkane(5)); + molecules.add(MoleculeFactory.makeBenzene()); + molecules.add(MoleculeFactory.makePhenylAmine()); + molecules.add(MoleculeFactory.makeCyclobutadiene()); are these four different situations, with different code run? That is, can they fail because of different reasons? If so, I suggest to split them up into separate unit tests... 3. I prefer longer variable names that I can give easily meaning too... s1 can be anything. For unit tests I find this less important, but just want to mention it. Formally, var names have to be at least three characters... 4. Assert.assertTrue(anglev==109.608); Use: assertEquals*109.608, anglev, 0.001); ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3519885&group_id=20024 |