Nicer exception handling and testing

Author: lptr

spaghetti-core/src/main/groovy/com/prezi/spaghetti/ast/parser/AstParserException.java

@@ -8,6 +8,6 @@ public AstParserException(ModuleDefinitionSource source, String message) {
 	}
 
 	public AstParserException(ModuleDefinitionSource source, String message, Throwable cause) {
-		super("Parse error in " + source.getLocation() + " " + message, cause);
+		super("Parse error in " + source.getLocation() + message, cause);
 	}
 }

spaghetti-core/src/main/groovy/com/prezi/spaghetti/ast/parser/InternalAstParserException.java

@@ -15,7 +15,7 @@ private InternalAstParserException(Token start, Token stop, String message, Thro
 
 	private static String createMessage(Token start, Token stop, String message)
 	{
-		return "at line " + start.getLine() + ":" + start.getCharPositionInLine() + ": " + message;
+		return " at line " + start.getLine() + ":" + start.getCharPositionInLine() + ": " + message;
 	}
 
 	public InternalAstParserException(Token token, String message, Throwable cause)

spaghetti-core/src/main/groovy/com/prezi/spaghetti/internal/DependencyTreeResolver.groovy

@@ -14,6 +14,15 @@ class DependencyTreeResolver {
 			[ module, dependencies.toSet() ]
 		}
 
+		// Check for non-existent modules
+		remainingModules.each { module, dependencies ->
+			dependencies.each {
+				if (!remainingModules.containsKey(it)) {
+					throw new IllegalArgumentException("Module found: ${it} (dependency of module ${module})")
+				}
+			}
+		}
+
 		Map<M, I> moduleInstances = [:]
 
 		while (remainingModules) {

spaghetti-core/src/test/groovy/com/prezi/spaghetti/ast/FQNameTest.groovy

@@ -11,7 +11,8 @@ class FQNameTest extends Specification {
 		FQName.fromString(what)
 
 		then:
-		thrown IllegalArgumentException
+		def ex = thrown IllegalArgumentException
+		ex.message == "Qualified name cannot be empty"
 
 		where:
 		what | _

spaghetti-core/src/test/groovy/com/prezi/spaghetti/ast/internal/NodeSetTest.groovy

@@ -78,23 +78,18 @@ class NodeSetTest extends Specification {
 	def "Duplicate: #value (#key)"() {
 		set.add value
 
-		def exception = null
-		try {
-			set.add value
-			assert false
-		} catch (InternalAstParserException ex) {
-			exception = ex
-		}
+		when:
+		set.add value
 
-		expect:
-		exception instanceof InternalAstParserException
-		exception.message == "Test with the same name already exists: " + key
+		then:
+		def ex = thrown InternalAstParserException
+		ex.message == message
 
 		where:
-		key          | value                           | set
-		""           | new TestNode("")                | new DefaultNamedNodeSet<TestNode>("test")
-		"lajos"      | new TestNode("lajos")           | new DefaultNamedNodeSet<TestNode>("test")
-		"alma.lajos" | new TestQNode(qn("alma.lajos")) | new DefaultQualifiedNodeSet<TestQNode>("test")
+		message                                              | value                           | set
+		"Test with the same name already exists: "           | new TestNode("")                | new DefaultNamedNodeSet<TestNode>("test")
+		"Test with the same name already exists: lajos"      | new TestNode("lajos")           | new DefaultNamedNodeSet<TestNode>("test")
+		"Test with the same name already exists: alma.lajos" | new TestQNode(qn("alma.lajos")) | new DefaultQualifiedNodeSet<TestQNode>("test")
 	}
 
 

spaghetti-core/src/test/groovy/com/prezi/spaghetti/bundle/ModuleBundleTest.groovy

@@ -59,7 +59,7 @@ class ModuleBundleTest extends Specification {
 		then:
 		1 * source.hasFile("META-INF/MANIFEST.MF") >> false
 		0 * _
-		IllegalArgumentException ex = thrown()
+		def ex = thrown IllegalArgumentException
 		ex.message.contains "Not a module, missing manifest"
 	}
 

spaghetti-core/src/test/groovy/com/prezi/spaghetti/packaging/DependencyTreeResolverTest.groovy

@@ -42,7 +42,8 @@ class DependencyTreeResolverTest extends Specification {
 
 		then:
 		0 * _
-		thrown IllegalStateException
+		def ex = thrown IllegalStateException
+		ex.message == "Cyclic dependency detected among modules: [a, b, c]"
 	}
 
 	def "cyclic dependency 2"() {
@@ -58,7 +59,8 @@ class DependencyTreeResolverTest extends Specification {
 		then:
 		1 * processor.processDependency("b", [])
 		0 * _
-		thrown IllegalStateException
+		def ex = thrown IllegalStateException
+		ex.message == "Cyclic dependency detected among modules: [a, c]"
 	}
 
 	def "non-existent module"() {
@@ -72,8 +74,8 @@ class DependencyTreeResolverTest extends Specification {
 		], processor)
 
 		then:
-		1 * processor.processDependency("b", [])
 		0 * _
-		thrown IllegalStateException
+		def ex = thrown IllegalArgumentException
+		ex.message == "Module found: d (dependency of module a)"
 	}
 }