Fix tests not finding project base directory 'org.aspectj'

Several LTW tests using class TestServer failed on my machine because
there was a hard-coded project base directory name 'org.aspectj' in the
class. This class has several other problems, but my quick fix for now -
I did not want to rename my project base directory - was to match on a
regex '(?i)(org[.])?aspectj' now. That also works for my root directory
'AspectJ'.

I also moved the code determining the project dir into a protected
(hence testable) method and added a sanity test case checking if the
directory can be determined. If not, the test will fail with a rather
lengthy warning to developers about the need to have a matching project
root folder.

Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>

2 files changed, 39 insertions, 14 deletions

diff --git a/testing-client/src/main/java/org/aspectj/testing/server/TestServer.java b/testing-client/src/main/java/org/aspectj/testing/server/TestServer.java
index a8b500ff0..25a1ad464 100644
--- a/testing-client/src/main/java/org/aspectj/testing/server/TestServer.java
+++ b/testing-client/src/main/java/org/aspectj/testing/server/TestServer.java
@@ -29,6 +29,8 @@ import java.util.StringTokenizer;
 public class TestServer implements Runnable {
 
 	private static final boolean debug = Boolean.getBoolean("org.aspectj.testing.server.debug");
+	// AspectJ project root folder nameing pattern, case-insensitive (i.e. org.aspectj, AspectJ)
+	protected static final String REGEX_PROJECT_ROOT_FOLDER = "(?i)(org[.])?aspectj";
 
 	private boolean exitOnError = true;
 	private File workingDirectory;
@@ -98,20 +100,11 @@ public class TestServer implements Runnable {
 		List<URL> urlList = new ArrayList<>();
 
 		// Sandbox
-		URL url = workingDirectory.getCanonicalFile().toURI().toURL();
-		urlList.add(url);
-
-		// Find the AspectJ root folder (i.e. org.aspectj)
-		File aspectjBase = new File(".").getCanonicalFile();
-		while (aspectjBase!= null && !aspectjBase.getName().equals("org.aspectj")) {
-			aspectjBase = aspectjBase.getParentFile();
-		}
-		if (aspectjBase == null) {
-			error("Unable to locate 'org.aspectj' in "+new File(".").getCanonicalPath());
-		}
-		urlList.add(new File(aspectjBase,"runtime/target/classes").toURI().toURL());
-//		urlList.add(new File(aspectjBase,"aspectjrt/target/classes").toURI().toURL());
-//		urlList.add(new File(aspectjBase,"aspectj5rt/target/classes").toURI().toURL());
+		urlList.add(workingDirectory.getCanonicalFile().toURI().toURL());
+		File projectRootFolder = findProjectRootFolder();
+		urlList.add(new File(projectRootFolder,"runtime/target/classes").toURI().toURL());
+//		urlList.add(new File(projectRootFolder,"aspectjrt/target/classes").toURI().toURL());
+//		urlList.add(new File(projectRootFolder,"aspectj5rt/target/classes").toURI().toURL());
 
 		URL[] urls = new URL[urlList.size()];
 		urlList.toArray(urls);
@@ -120,6 +113,22 @@ public class TestServer implements Runnable {
 		if (debug) System.err.println("? TestServer.createRootLoader() loader=" + rootLoader + ", urlList=" + urlList + ", parent=" + parent);
 	}
 
+	// Make protected in order to at least make this part of the code testable -> TestServerTest
+	protected File findProjectRootFolder() throws IOException {
+		// Find the AspectJ project root folder
+		File currentFolder = new File(".").getCanonicalFile();
+		while (currentFolder != null && !currentFolder.getName().matches(REGEX_PROJECT_ROOT_FOLDER)) {
+			currentFolder = currentFolder.getParentFile();
+		}
+		if (currentFolder == null) {
+			error(
+				"Unable to locate project root folder matching regex '" + REGEX_PROJECT_ROOT_FOLDER +
+					"' in " + new File(".").getCanonicalPath()
+			);
+		}
+		return currentFolder;
+	}
+
 	public void setExitOntError (boolean b) {
 		exitOnError = b;
 	}
@@ -183,6 +192,9 @@ public class TestServer implements Runnable {
 
 	private void error (String message) {
 		System.out.println(message);
+		// FIXME:
+		//   This is horrible: Why not just throw an exception? And if we exit, why with exit code 0?
+		//   It basically makes the tests useless because they might log errors without checking any conditions.
 		if (exitOnError) System.exit(0);
 	}
 }
diff --git a/testing-client/src/test/java/org/aspectj/testing/server/TestServerTest.java b/testing-client/src/test/java/org/aspectj/testing/server/TestServerTest.java
index 0c278588e..0f40d3a03 100644
--- a/testing-client/src/test/java/org/aspectj/testing/server/TestServerTest.java
+++ b/testing-client/src/test/java/org/aspectj/testing/server/TestServerTest.java
@@ -14,6 +14,8 @@ import java.io.IOException;
 
 import junit.framework.TestCase;
 
+import static org.aspectj.testing.server.TestServer.REGEX_PROJECT_ROOT_FOLDER;
+
 public class TestServerTest extends TestCase {
 
 	private TestServer server;
@@ -34,6 +36,17 @@ public class TestServerTest extends TestCase {
 		}
 	}
 
+	public void testFindProjectRootDirectory() throws IOException {
+		// Give developers some advice in the build log about why their LTW tests fail
+		assertNotNull(
+			"Cannot find AspectJ project root folder. " +
+				"This will lead to subsequent failures in all tests using class TestServer. " +
+				"Please make sure to name your project root folder 'org.aspectj', 'AspectJ' or " +
+				"something else matching regex '"+ REGEX_PROJECT_ROOT_FOLDER+"'.",
+			server.findProjectRootFolder()
+		);
+	}
+
 	public void testSetWorkingDirectory() {
 		server.setWorkingDirectory("../testing-client/testdata");
 	}