Updated version, all tests pass. I took a quick look at what happens if you define two cores with the same name old-style, and errors are thrown in addition to the warning issued. So leaving it as-is seems fine.
But, while looking at that code I think it throws a bogus warning, it just tests for the dataDir defined in the <core> tag. So defining two cores with dataDir=./data would issue the warning even though they were in different instance dirs.
String absData = new File(instDir, dataDir).getCanonicalPath();
a reliable way to get the full path? If so, the test in this patch will work in old-style. It worked on a single test anyway.
2> One could test for duplicate dirs in new-style as well. Is it worth it? Two variants come to mind:
2a> Pass a map a-la coreDescriptorMap and do simliar stuff.
2b> in SolrCoreDiscoverer.discover, just analyze the map upon return and issue a warning.
Either way it would just be some bookeeping local to SolrCoreDiscoverer and not intrusive. But is either one worth the effort?
[~markrmiller] Andy Fowler any opinions?