These failed for me unexpectedly:
JmolParserTest.testFileParser()
JmolParserTest.testAlignmentLoader()
I eventually tracked it down (it took a while):
* test setup sets ADD_SS_ANN and STRUCT_FROM_PDB to TRUE, but didn't
explicitly set ADD_TEMPFACT_ANN to FALSE
* I had this set to TRUE in my properties file, so two annotations per
sequence were created
* the assertion on the first annotation on the sequence reported 'No
secondary structure assigned', but it actually failed on 'annotation.hasIcons'
* the secondary structure annotation was created fine, but was the
second annotation, and Temp Factor fails 'hasIcons'
Fix: set ADD_TEMPFACT_ANN to FALSE in setup
Moral 1: use separate assertions for different failure conditions
Moral 2: always use a test properties file for testing under controlled
conditions e.g.
Cache.loadProperties("test/jalview/io/testProps.jvprops");
It took time to debug because the preferences passing is so complicated:
* the JmolParser constructor call in testFileParser() has
addAlignmentAnnotations = false...?
* hang on, that parameter isn't used; it should be either removed or
used (as in PDBFile constructor)
* in fact, the preferences for 'annotation from structure' etc are set but
never read in testFileParser
* they are read from Cache in FormatAdapter.init(), then transferred to
a singleton with StructureImportSettings.addSettings(), then read in
StructureFile.xferSettings() (from JmolParser) or StructureFile.addSettings()
(from PBDFile constructors), then transferred in the PDBChain constructor, then
read in makeResidueList()
* but testFileParser bypasses the first two steps (goes direct to the
JmolParser constructor) so the preferences are never read
* sure enough, if I turn off secondary structure in my preferences, and
run testFileParser() on its own, it fails
* it passes when JmolParserTest is run because other tests initialise
the StructureImportSettings singleton (I think)
This all looks unnecessarily complicated, and makes debugging tedious. Is
there any reason why we can't simply read the properties from Cache when needed?
If there is an objection that it needs a test of two flags combined, and that
is application logic that shouldn't be in Cache, then let
StructureImportSettings do that, but not copy / cache / store / propagate the
actual values.
mungo
Mungo Carstairs
Jalview Computational Scientist
The Barton Group
Division of Computational Biology
School of Life Sciences
University of Dundee, Dundee, Scotland, UK.
www.jalview.org<http://www.jalview.org/>
www.compbio.dundee.ac.uk<http://www.compbio.dundee.ac.uk/>
The University of Dundee is a registered Scottish Charity, No: SC015096
_______________________________________________
Jalview-dev mailing list
[email protected]
http://www.compbio.dundee.ac.uk/mailman/listinfo/jalview-dev
