Hardy Pottinger and I were looking at mysterious test failures and came across test code that I think can be improved. We'll file a ticket on the test, but the nature of the problem suggests that it may exist elsewhere as well.
The problem we found is that org.dspace.content.ItemTest#testAddMetadata_7args_2_noauthority is not completely controlling the environment presented to Item. It relies on the presence and absence of certain properties in config/dspace.cfg. This became evident when tests were run on a local version of DSpace which included the local configuration, not the stock configuration that ships with DSpace. This is not an unreasonable thing to do, I think, as it keeps the local configuration under version control and closely associated with the local code. But in this case it happened that the local configuration enables authority control on a field which the test requires to be uncontrolled. What I think the test class *should* do is to engage org.dspace.core.MockConfigurationManager and provide the exact values that the tests expect for all relevant properties. Then, no changes to dspace.cfg can have any effect on the outcome of a test. What made this interesting to find is that the configuration affects, not Item, but MetadataAuthorityManager. There is a note in the test which says that MetadataAuthorityManager cannot be properly mocked. Still, we can better control the environment in which that class operates when Item calls its methods. I'm rattling on about all this because it shows that we need to be very careful about setting up the environment of a unit test, and because this looks like a thing which is likely to be found elsewhere in the test suite and indeed in other test classes as well. When writing test code, we need to follow the call chains all the way down and ferret out any external dependencies we may find, so they can be controlled. (*sigh*, I see now that the time I spent implementing dspace.cfg.more would have been better spent removing the need for external configuration data. Only tests of ConfigurationManager and ConfigurationService should depend on external files for configuration properties.) -- Mark H. Wood Lead Technology Analyst University Library Indiana University - Purdue University Indianapolis 755 W. Michigan Street Indianapolis, IN 46202 317-274-0749 www.ulib.iupui.edu
signature.asc
Description: Digital signature
------------------------------------------------------------------------------ Dive into the World of Parallel Programming. The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________ Dspace-devel mailing list Dspace-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dspace-devel
