Improve ex msg when locations & classes are declared in test hierarchy

Prior to this commit, if both locations and classes were declared via
@ContextConfiguration at differing levels in a test class hierarchy,
the exception message stated that neither of the default context
loaders was able to load an ApplicationContext from the merged context
configuration, but the message didn't explain why.

This commit adds an explicit check for such scenarios and provides a
more informative exception message similar to the following:

"Neither X nor Y supports loading an ApplicationContext from
[MergedContextConfiguration ...]: declare either 'locations' or
'classes' but not both."

Issue: SPR-12060

spring-test/src/main/java/org/springframework/test/context/support/AbstractDelegatingSmartContextLoader.java

@@ -21,7 +21,6 @@ import java.util.List;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-
 import org.springframework.context.ApplicationContext;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.ContextConfigurationAttributes;
@@ -86,7 +85,6 @@ public abstract class AbstractDelegatingSmartContextLoader implements SmartConte
 	 */
 	protected abstract SmartContextLoader getAnnotationConfigLoader();
 
-
 	// --- SmartContextLoader --------------------------------------------------
 
 	private static String name(SmartContextLoader loader) {
@@ -112,10 +110,10 @@ public abstract class AbstractDelegatingSmartContextLoader implements SmartConte
 
 	private boolean supports(SmartContextLoader loader, MergedContextConfiguration mergedConfig) {
 		if (loader == getAnnotationConfigLoader()) {
-			return ObjectUtils.isEmpty(mergedConfig.getLocations()) && !ObjectUtils.isEmpty(mergedConfig.getClasses());
+			return mergedConfig.hasClasses() && !mergedConfig.hasLocations();
 		}
 		else {
-			return !ObjectUtils.isEmpty(mergedConfig.getLocations()) && ObjectUtils.isEmpty(mergedConfig.getClasses());
+			return mergedConfig.hasLocations() && !mergedConfig.hasClasses();
 		}
 	}
 
@@ -152,11 +150,10 @@ public abstract class AbstractDelegatingSmartContextLoader implements SmartConte
 	 */
 	@Override
 	public void processContextConfiguration(final ContextConfigurationAttributes configAttributes) {
-
 		Assert.notNull(configAttributes, "configAttributes must not be null");
 		Assert.isTrue(!(configAttributes.hasLocations() && configAttributes.hasClasses()), String.format(
-				"Cannot process locations AND classes for context configuration %s; configure one or the other, but not both.",
-				configAttributes));
+			"Cannot process locations AND classes for context configuration %s: "
+					+ "configure one or the other, but not both.", configAttributes));
 
 		// If the original locations or classes were not empty, there's no
 		// need to bother with default detection checks; just let the
@@ -208,15 +205,15 @@ public abstract class AbstractDelegatingSmartContextLoader implements SmartConte
 			// throw an exception.
 			if (!configAttributes.hasResources() && ObjectUtils.isEmpty(configAttributes.getInitializers())) {
 				throw new IllegalStateException(String.format(
-					"Neither %s nor %s was able to detect defaults, and no ApplicationContextInitializers " +
-							"were declared for context configuration %s", name(getXmlLoader()),
+					"Neither %s nor %s was able to detect defaults, and no ApplicationContextInitializers "
+							+ "were declared for context configuration %s", name(getXmlLoader()),
 					name(getAnnotationConfigLoader()), configAttributes));
 			}
 
 			if (configAttributes.hasLocations() && configAttributes.hasClasses()) {
 				String message = String.format(
-					"Configuration error: both default locations AND default configuration classes " +
-							"were detected for context configuration %s; configure one or the other, but not both.",
+					"Configuration error: both default locations AND default configuration classes "
+							+ "were detected for context configuration %s; configure one or the other, but not both.",
 					configAttributes);
 				logger.error(message);
 				throw new IllegalStateException(message);
@@ -249,6 +246,13 @@ public abstract class AbstractDelegatingSmartContextLoader implements SmartConte
 		Assert.notNull(mergedConfig, "mergedConfig must not be null");
 		List<SmartContextLoader> candidates = Arrays.asList(getXmlLoader(), getAnnotationConfigLoader());
 
+		if (mergedConfig.hasLocations() && mergedConfig.hasClasses()) {
+			throw new IllegalStateException(String.format(
+				"Neither %s nor %s supports loading an ApplicationContext from %s: "
+						+ "declare either 'locations' or 'classes' but not both.", name(getXmlLoader()),
+				name(getAnnotationConfigLoader()), mergedConfig));
+		}
+
 		for (SmartContextLoader loader : candidates) {
 			// Determine if each loader can load a context from the mergedConfig. If it
 			// can, let it; otherwise, keep iterating.
@@ -263,12 +267,12 @@ public abstract class AbstractDelegatingSmartContextLoader implements SmartConte
 			return delegateLoading(getAnnotationConfigLoader(), mergedConfig);
 		}
 
+		// else...
 		throw new IllegalStateException(String.format(
 			"Neither %s nor %s was able to load an ApplicationContext from %s.", name(getXmlLoader()),
 			name(getAnnotationConfigLoader()), mergedConfig));
 	}
 
-
 	// --- ContextLoader -------------------------------------------------------
 
 	/**
@@ -279,8 +283,8 @@ public abstract class AbstractDelegatingSmartContextLoader implements SmartConte
 	 */
 	@Override
 	public final String[] processLocations(Class<?> clazz, String... locations) {
-		throw new UnsupportedOperationException("DelegatingSmartContextLoaders do not support the ContextLoader SPI. " +
-				"Call processContextConfiguration(ContextConfigurationAttributes) instead.");
+		throw new UnsupportedOperationException("DelegatingSmartContextLoaders do not support the ContextLoader SPI. "
+				+ "Call processContextConfiguration(ContextConfigurationAttributes) instead.");
 	}
 
 	/**
@@ -291,8 +295,8 @@ public abstract class AbstractDelegatingSmartContextLoader implements SmartConte
 	 */
 	@Override
 	public final ApplicationContext loadContext(String... locations) throws Exception {
-		throw new UnsupportedOperationException("DelegatingSmartContextLoaders do not support the ContextLoader SPI. " +
-				"Call loadContext(MergedContextConfiguration) instead.");
+		throw new UnsupportedOperationException("DelegatingSmartContextLoaders do not support the ContextLoader SPI. "
+				+ "Call loadContext(MergedContextConfiguration) instead.");
 	}
 
 }

spring-test/src/test/java/org/springframework/test/context/support/DelegatingSmartContextLoaderTests.java

 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2014 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,12 +16,9 @@
 
 package org.springframework.test.context.support;
 
-import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.context.annotation.Bean;
@@ -31,6 +28,9 @@ import org.springframework.test.context.ContextLoader;
 import org.springframework.test.context.MergedContextConfiguration;
 import org.springframework.util.ObjectUtils;
 
+import static org.hamcrest.CoreMatchers.*;
+import static org.junit.Assert.*;
+
 /**
  * Unit tests for {@link DelegatingSmartContextLoader}.
  *
@@ -44,6 +44,9 @@ public class DelegatingSmartContextLoaderTests {
 
 	private final DelegatingSmartContextLoader loader = new DelegatingSmartContextLoader();
 
+	@Rule
+	public ExpectedException expectedException = ExpectedException.none();
+
 
 	private static void assertEmpty(Object[] array) {
 		assertTrue(ObjectUtils.isEmpty(array));
@@ -51,8 +54,12 @@ public class DelegatingSmartContextLoaderTests {
 
 	// --- SmartContextLoader - processContextConfiguration() ------------------
 
-	@Test(expected = IllegalStateException.class)
+	@Test
 	public void processContextConfigurationWithoutLocationsAndConfigurationClassesForBogusTestClass() {
+		expectedException.expect(IllegalStateException.class);
+		expectedException.expectMessage(startsWith("Neither"));
+		expectedException.expectMessage(containsString("was able to detect defaults"));
+
 		ContextConfigurationAttributes configAttributes = new ContextConfigurationAttributes(getClass(),
 			EMPTY_STRING_ARRAY, EMPTY_CLASS_ARRAY, true, null, true, ContextLoader.class);
 		loader.processContextConfiguration(configAttributes);
@@ -76,8 +83,11 @@ public class DelegatingSmartContextLoaderTests {
 		assertEmpty(configAttributes.getLocations());
 	}
 
-	@Test(expected = IllegalStateException.class)
+	@Test
 	public void processContextConfigurationWithDefaultXmlConfigAndConfigurationClassGeneration() {
+		expectedException.expect(IllegalStateException.class);
+		expectedException.expectMessage(containsString("both default locations AND default configuration classes were detected"));
+
 		ContextConfigurationAttributes configAttributes = new ContextConfigurationAttributes(
 			ImproperDuplicateDefaultXmlAndConfigClassTestCase.class, EMPTY_STRING_ARRAY, EMPTY_CLASS_ARRAY, true, null,
 			true, ContextLoader.class);
@@ -112,13 +122,31 @@ public class DelegatingSmartContextLoaderTests {
 		loader.loadContext(mergedConfig);
 	}
 
-	@Test(expected = IllegalStateException.class)
+	@Test
 	public void loadContextWithoutLocationsAndConfigurationClasses() throws Exception {
+		expectedException.expect(IllegalStateException.class);
+		expectedException.expectMessage(startsWith("Neither"));
+		expectedException.expectMessage(containsString("was able to load an ApplicationContext from"));
+
 		MergedContextConfiguration mergedConfig = new MergedContextConfiguration(getClass(), EMPTY_STRING_ARRAY,
 			EMPTY_CLASS_ARRAY, EMPTY_STRING_ARRAY, loader);
 		loader.loadContext(mergedConfig);
 	}
 
+	/**
+	 * @since 4.1
+	 */
+	@Test
+	public void loadContextWithLocationsAndConfigurationClasses() throws Exception {
+		expectedException.expect(IllegalStateException.class);
+		expectedException.expectMessage(startsWith("Neither"));
+		expectedException.expectMessage(endsWith("declare either 'locations' or 'classes' but not both."));
+
+		MergedContextConfiguration mergedConfig = new MergedContextConfiguration(getClass(),
+			new String[] { "test.xml" }, new Class[] { getClass() }, EMPTY_STRING_ARRAY, loader);
+		loader.loadContext(mergedConfig);
+	}
+
 	private void assertApplicationContextLoadsAndContainsFooString(MergedContextConfiguration mergedConfig)
 			throws Exception {
 		ApplicationContext applicationContext = loader.loadContext(mergedConfig);