- test class loader fix for test classes loaded from jars

- implemented aspectpath, classpath,
  - removed CompilerRun state duplicated in sandbox
  - sandbox owns path assembly/formatting for javarun

5 files changed, 258 insertions, 105 deletions

diff --git a/testing/src/org/aspectj/testing/harness/bridge/CompilerRun.java b/testing/src/org/aspectj/testing/harness/bridge/CompilerRun.java
index ccb38b2f4..6e0e2f5fb 100644
--- a/testing/src/org/aspectj/testing/harness/bridge/CompilerRun.java
+++ b/testing/src/org/aspectj/testing/harness/bridge/CompilerRun.java
@@ -36,6 +36,24 @@ import java.util.ListIterator;
 
 /**
  * Run the compiler once in non-incremental mode.
+ * The lifecycle is as follows:
+ * <ul>
+ * <li>Spec (specification) is created.</li>
+ * <li>This is created using the Spec.</li>
+ * <li>setupAjcRun(Sandbox, Validator) is invoked,
+ *     at which point this populates the shared sandbox
+ *     with values derived from the spec and also
+ *     sets up internal state based on both the sandbox
+ *     and the spec.</li>
+ * <li>run(IRunStatus) is invoked, and this runs the compiler
+ *     based on internal state, the spec, and the sandbox.</li>
+ * </ul>
+ * Programmer notes:
+ * <ul>
+ * <li>Paths are resolved absolutely, which fails to test the
+ *     compiler's ability to find files relative to a source base</li>
+ * <li>This does not enforce the lifecycle.</li>
+ * </ul>
  */
 public class CompilerRun implements IAjcRun {
 	static final String[] RA_String = new String[0];
@@ -64,12 +82,6 @@ public class CompilerRun implements IAjcRun {
      */
     final List /*String*/ injars;
    
-    /** 
-     * During run, these String are collapsed and passed as the aspectpath option.
-     * The list is set up in setupAjcRun(..).
-     */
-    final List /*String*/ aspectpath;
-
     private CompilerRun(Spec spec) {
         if (null == spec) {
             throw new IllegalArgumentException("null spec");
@@ -77,15 +89,20 @@ public class CompilerRun implements IAjcRun {
         this.spec = spec;
         arguments = new ArrayList();
         injars = new ArrayList();
-        aspectpath = new ArrayList();
     }           
     
     /** 
      * This checks that the spec is reasonable and does setup:
-     * <li>calculate and set sandbox testBaseSrcDir as 
-     *     {Sandbox.testBaseDir}/{Spec.testSrcDirOffset}/<li>
-     * <li>the list of source File to compile as
-     *     {Sandbox.testBaseSrcDir}/{Spec.getPaths..}</li>
+     * <ul>
+     * <li>calculate and set sandbox testBaseSrcDir as {Sandbox.testBaseDir}/
+     * {Spec.testSrcDirOffset}/<li>
+     * <li>get the list of source File to compile as {Sandbox.testBaseSrcDir} /
+     * {Spec.getPaths..}</li>
+     * <li>get the list of extraClasspath entries to add to default classpath as
+     * {Sandbox.testBaseSrcDir} / {Spec.classpath..}</li>
+     * <li>get the list of aspectpath entries to use as the aspectpath as
+     * {Sandbox. testBaseSrcDir} / {Spec.aspectpath..}</li>
+     * </ul>
      * All sources must be readable at this time.
      * @param classesDir the File
 	 * @see org.aspectj.testing.harness.bridge.AjcTest.IAjcRun#setup(File, File)
@@ -99,17 +116,12 @@ public class CompilerRun implements IAjcRun {
             || !validator.nullcheck(spec.compiler, "compilerName")
             || !validator.canRead(Globals.F_aspectjrt_jar, "aspectjrt.jar")
             || !validator.canRead(Globals.F_testingclient_jar, "testing-client.jar")
-            //|| !validator.canRead(Main.F_bridge_jar, "bridge.jar")
             ) {
             return false;
         }
-        
+         
         this.sandbox = sandbox;
         
-        File[] cp = new File[] 
-            { sandbox.classesDir, Globals.F_aspectjrt_jar, Globals.F_testingclient_jar };
-        sandbox.setClasspath(cp, true, this);
-        
         String rdir = spec.testSrcDirOffset;
         File testBaseSrcDir;
         if ((null == rdir) || (0 == rdir.length())) {
@@ -122,12 +134,10 @@ public class CompilerRun implements IAjcRun {
         }
         sandbox.setTestBaseSrcDir(testBaseSrcDir, this);
         
-        arguments.clear();
         
-        // sources come as relative paths - check read, copy if staging
-
-        // this renders paths absolute before run(RunStatusI) is called
-        // for a compile run to support relative paths + source base
+        // Sources come as relative paths - check read, copy if staging.
+        // This renders paths absolute before run(RunStatusI) is called.
+        // For a compile run to support relative paths + source base,
         // change so the run calculates the paths (differently when staging)
 
         final String[] injarPaths; 
@@ -140,7 +150,9 @@ public class CompilerRun implements IAjcRun {
         // validate readable for sources
         if (!validator.canRead(testBaseSrcDir, srcPaths, "sources")
             || !validator.canRead(testBaseSrcDir, injarPaths, "injars")
-            || !validator.canRead(testBaseSrcDir, spec.argfiles, "argfiles")) {
+            || !validator.canRead(testBaseSrcDir, spec.argfiles, "argfiles")
+            || !validator.canRead(testBaseSrcDir, spec.classpath, "classpath")
+            || !validator.canRead(testBaseSrcDir, spec.aspectpath, "aspectpath")) {
             return false;
         }
         int numSources = srcPaths.length + injarPaths.length + spec.argfiles.length;
@@ -149,21 +161,26 @@ public class CompilerRun implements IAjcRun {
             return false;
         }
         
-        File[] srcFiles;
-        File[] argFiles;
-        File[] injarFiles;
-        File[] aspectFiles;
+        final File[] argFiles = FileUtil.getBaseDirFiles(testBaseSrcDir, spec.argfiles);
+        final File[] injarFiles = FileUtil.getBaseDirFiles(testBaseSrcDir, injarPaths);
+        final File[] aspectFiles = FileUtil.getBaseDirFiles(testBaseSrcDir, spec.aspectpath);
+        final File[] classFiles = FileUtil.getBaseDirFiles(testBaseSrcDir, spec.classpath);
+        // hmm - duplicates validation above, verifying getBaseDirFiles?
+        if (!validator.canRead(argFiles, "argFiles")
+            || !validator.canRead(injarFiles, "injarfiles")
+            || !validator.canRead(aspectFiles, "aspectfiles")
+            || !validator.canRead(classFiles, "classfiles")) {
+            return false;
+        }
+
+        final File[] srcFiles;
+
+        // source text files are copied when staging incremental tests
         if (!spec.isStaging()) { // XXX why this? was always? || (testBaseSrcDir != sandbox.stagingDir))) {
             srcFiles = FileUtil.getBaseDirFiles(testBaseSrcDir, srcPaths, CompilerRun.SOURCE_SUFFIXES);
-            argFiles = FileUtil.getBaseDirFiles(testBaseSrcDir, spec.argfiles);
-            injarFiles = FileUtil.getBaseDirFiles(testBaseSrcDir, injarPaths);
-            aspectFiles = FileUtil.getBaseDirFiles(testBaseSrcDir, spec.aspectpath);
         } else { // staging - copy files
             try {
                 srcFiles = FileUtil.copyFiles(testBaseSrcDir, srcPaths, sandbox.stagingDir);
-                argFiles = FileUtil.copyFiles(testBaseSrcDir, spec.argfiles, sandbox.stagingDir);
-                injarFiles = FileUtil.copyFiles(testBaseSrcDir, injarPaths, sandbox.stagingDir);
-                aspectFiles = FileUtil.copyFiles(testBaseSrcDir, spec.aspectpath, sandbox.stagingDir);
             } catch (IllegalArgumentException e) {
                 validator.fail("staging - bad input", e);
                 return false;
@@ -171,24 +188,18 @@ public class CompilerRun implements IAjcRun {
                 validator.fail("staging - operations", e);
                 return false;
             }
-            // validate readable for copied sources
-            if (!validator.canRead(srcFiles, "copied paths")
-                || !validator.canRead(argFiles, "copied argfiles")) {
-                return false;
-            }
+        }
+        if (!validator.canRead(srcFiles, "copied paths")) {
+            return false;
         }
         arguments.clear();
-        injars.clear();
-        aspectpath.clear();
         if (!LangUtil.isEmpty(srcFiles)) {
             arguments.addAll(Arrays.asList(FileUtil.getPaths(srcFiles)));
         }
+        injars.clear();
         if (!LangUtil.isEmpty(injarFiles)) {
             injars.addAll(Arrays.asList(FileUtil.getPaths(injarFiles)));
         }
-        if (!LangUtil.isEmpty(aspectFiles)) {
-            aspectpath.addAll(Arrays.asList(FileUtil.getPaths(aspectFiles)));
-        }
         if (!LangUtil.isEmpty(argFiles)) {
             String[] ra = FileUtil.getPaths(argFiles);
             for (int j = 0; j < ra.length; j++) {
@@ -199,12 +210,21 @@ public class CompilerRun implements IAjcRun {
             }               
         }
 
+        // save classpath and aspectpath in sandbox for this and other clients
+        final boolean checkReadable = true; // hmm - third validation?
+        File[] cp = new File[3 + classFiles.length];
+        System.arraycopy(classFiles, 0, cp, 0, classFiles.length);
+        int index = classFiles.length;
+        cp[index++] = sandbox.classesDir;
+        cp[index++] = Globals.F_aspectjrt_jar;
+        cp[index++] = Globals.F_testingclient_jar;
+        sandbox.setClasspath(cp, checkReadable, this);
+        // set aspectpath
+        if (0 < aspectFiles.length) {
+            sandbox.setAspectpath(aspectFiles, checkReadable, this);
+        }
         return true;
     }
-//        if ((null != spec.dirChanges) && (0 < spec.numMessages(IMessage.ERROR, false))) {
-//            validator.info("CompilerRun warning: when expecting errors, cannot expect dir changes");
-//        }
-
     
     /**
      * Setup result evaluation and command line, run, and evaluate result.
@@ -251,24 +271,28 @@ public class CompilerRun implements IAjcRun {
                 MessageUtil.abort(status, "canonical " + sandbox.classesDir, e);
             }
             argList.add(outputDirPath);
-            argList.add("-classpath");
-            argList.add(sandbox.classpathToString(this));
-            // XXX classpath additions here?
 
-            if (0 < injars.size()) {
-                argList.add("-injars");
-                argList.add(FileUtil.flatten((String[]) injars.toArray(new String[0]), null));
+            String path = sandbox.classpathToString(this);
+            if (!LangUtil.isEmpty(path)) {
+                argList.add("-classpath");
+                argList.add(path);
             }
 
-            if (0 < aspectpath.size()) {
+            path = sandbox.aspectpathToString(this);
+            if (!LangUtil.isEmpty(path)) {
                 argList.add("-aspectpath");
-                argList.add(FileUtil.flatten((String[]) aspectpath.toArray(new String[0]), null));
+                argList.add(path);
+            }
+
+            if (0 < injars.size()) {
+                argList.add("-injars");
+                argList.add(FileUtil.flatten((String[]) injars.toArray(new String[0]), null));
             }
 
             // add both java/aspectj and argfiles
             argList.addAll(arguments);
             
-            // hack - do seeking on request as a side effect. reimplement as listener 
+            // XXX hack - seek on request as a side effect. reimplement as listener 
             if (null != setupResult.seek) {
                 String slopPrefix = Spec.SEEK_MESSAGE_PREFIX + " slop - ";
                 PrintStream slop = MessageUtil.handlerPrintStream(
@@ -409,6 +433,7 @@ public class CompilerRun implements IAjcRun {
         
         protected String[] argfiles = new String[0];
         protected String[] aspectpath = new String[0];
+        protected String[] classpath = new String[0];
         
         /** src path = {suiteParentDir}/{testBaseDirOffset}/{testSrcDirOffset}/{path} */
         protected String testSrcDirOffset;
@@ -455,9 +480,22 @@ public class CompilerRun implements IAjcRun {
             addPaths(paths);
         }
         
+        /**
+         * Add to default classpath
+         * (which includes aspectjrt.jar and testing-client.jar).
+         * @param files comma-delimited list of classpath entries - ignored if
+         * null or empty
+         */
+        public void setClasspath(String files) {
+            if (!LangUtil.isEmpty(files)) {
+                classpath = XMLWriter.unflattenList(files);
+            }
+        }
+        
         /** 
          * Set aspectpath, deleting any old ones
-         * @param files comma-delimited list of argfiles - ignored if null or empty
+         * @param files comma-delimited list of aspect jars - ignored if null or
+         * empty
          */        
         public void setAspectpath(String files) {
             if (!LangUtil.isEmpty(files)) {
diff --git a/testing/src/org/aspectj/testing/harness/bridge/Sandbox.java b/testing/src/org/aspectj/testing/harness/bridge/Sandbox.java
index 6f4984bac..0cef8f680 100644
--- a/testing/src/org/aspectj/testing/harness/bridge/Sandbox.java
+++ b/testing/src/org/aspectj/testing/harness/bridge/Sandbox.java
@@ -21,6 +21,7 @@ import org.aspectj.util.LangUtil;
 
 import java.io.File;
 import java.util.ArrayList;
+import java.util.Arrays;
 
 /**
  * A sandbox holds state shared by AjcTest sub-runs,
@@ -106,7 +107,10 @@ public class Sandbox {
     private File testBaseSrcDir;
 
     /** directories and libraries on the classpath, set by CompileRun.setup(..)  */
-    private File[] classpath;
+    private File[] compileClasspath;
+
+    /** aspectpath entries, set by CompileRun.setup(..)  */
+    private File[] aspectpath;
 
     /** track whether classpath getter ran */
     private boolean gotClasspath;
@@ -316,36 +320,72 @@ public class Sandbox {
         testBaseSrcDir = dir;
     }
     
-    /** @param readable if true, then throw IllegalArgumentException if not readable */
+    /** 
+     * Set aspectpath.
+     * @param readable if true, then throw IllegalArgumentException if not readable 
+     */
+    void setAspectpath(File[] files, boolean readable, CompilerRun caller) {
+        LangUtil.throwIaxIfNull(files, "files");
+        LangUtil.throwIaxIfNull(caller, "caller");
+        assertState(null == aspectpath, "aspectpath already written");
+        aspectpath = new File[files.length];
+        for (int i = 0; i < files.length; i++) {
+            LangUtil.throwIaxIfNull(files[i], "files[i]");
+            if (readable && !files[i].canRead()) {
+                throw new IllegalArgumentException("bad aspectpath entry: " + files[i]);
+            }
+            aspectpath[i] = files[i];
+        }
+    }
+
+    /** 
+     * Set compile classpath.
+     * @param readable if true, then throw IllegalArgumentException if not readable 
+     */
     void setClasspath(File[] files, boolean readable, CompilerRun caller) {
         LangUtil.throwIaxIfNull(files, "files");
         LangUtil.throwIaxIfNull(caller, "caller");
         assertState(!gotClasspath, "classpath already read");
-        classpath = new File[files.length];
+        compileClasspath = new File[files.length];
         for (int i = 0; i < files.length; i++) {
             LangUtil.throwIaxIfNull(files[i], "files[i]");
             if (readable && !files[i].canRead()) {
                 throw new IllegalArgumentException("bad classpath entry: " + files[i]);
             }
-            classpath[i] = files[i];
+            compileClasspath[i] = files[i];
 		}
     }
 
-    File[] getClasspath(JavaRun caller) {
-        LangUtil.throwIaxIfNull(caller, "caller");
-        assertState(null != classpath, "classpath not set");
-        
-        File[] result = new File[classpath.length];
-        System.arraycopy(classpath, 0, result, 0, result.length);
-        return result;
-    }
+//    /**
+//     * Get run classpath 
+//     * @param caller unused except to restrict usage to non-null JavaRun.
+//     * @throws IllegalStateException if compileClasspath was not set.
+//     * @throws IllegalArgumentException if caller is null
+//     */
+//    File[] getRunClasspath(JavaRun caller) {
+//        LangUtil.throwIaxIfNull(caller, "caller");
+//        assertState(null != compileClasspath, "classpath not set");
+//        int compilePathLength = compileClasspath.length;
+//        int aspectPathLength = (null == aspectpath ? 0 : aspectpath.length);
+//        File[] result = new File[aspectPathLength + compilePathLength];
+//        System.arraycopy(compileClasspath, 0, result, 0, compilePathLength);
+//        if (0 < aspectPathLength) {
+//            System.arraycopy(aspectpath, 0, result, compilePathLength, aspectPathLength);
+//        }
+//        return result;
+//    }
     
-    /** @param readable if true, omit non-readable directories */
+    /** 
+     * Get directories for the run classpath by selecting them
+     * from the compile classpath.
+     * This ignores aspectpath since it may contain only jar files.
+     * @param readable if true, omit non-readable directories 
+     */
     File[] getClasspathDirectories(boolean readable, JavaRun caller) {
         LangUtil.throwIaxIfNull(caller, "caller");
-        assertState(null != classpath, "classpath not set");
+        assertState(null != compileClasspath, "classpath not set");
         ArrayList result = new ArrayList();
-        File[] src = classpath;
+        File[] src = compileClasspath;
         for (int i = 0; i < src.length; i++) {
 			File f = src[i];
             if ((null != f) && (f.isDirectory()) && (!readable || f.canRead())) {
@@ -355,32 +395,44 @@ public class Sandbox {
         return (File[]) result.toArray(new File[0]);
     }
 
-    /** @param readable if true, omit non-readable directories */
+    /** 
+     * Get the jars belonging on the run classpath, including classpath
+     * and aspectpath entries.
+     * @param readable if true, omit non-readable directories 
+     */
     File[] getClasspathJars(boolean readable, JavaRun caller) {
         LangUtil.throwIaxIfNull(caller, "caller");
-        assertState(null != classpath, "classpath not set");
+        assertState(null != compileClasspath, "classpath not set");
         ArrayList result = new ArrayList();
-        File[] src = classpath;
+        File[][] src = new File[][] { compileClasspath, aspectpath };
         for (int i = 0; i < src.length; i++) {
-            File f = src[i];
-            if (FileUtil.hasZipSuffix(f) && (!readable || f.canRead())) {
-                result.add(f);
-            }
+            File[] paths = src[i];
+            int len = (null == paths ? 0 : paths.length);
+            for (int j = 0; j < len; j++) {
+                File f = paths[j];
+                if (FileUtil.hasZipSuffix(f) && (!readable || f.canRead())) {
+                    result.add(f);
+                }
+			}
         }
         return (File[]) result.toArray(new File[0]);
     }
     
-    /** @return String of classpath entries delimited internally by File.pathSeparator */
+    /**
+     * Get the list of aspect jars as a String. 
+     * @return String of classpath entries delimited internally by File.pathSeparator 
+     */
+    String aspectpathToString(CompilerRun caller) {
+        LangUtil.throwIaxIfNull(caller, "caller");
+        return FileUtil.flatten(aspectpath, null);
+    }
+    
+    /** 
+     * Get the compile classpath as a String.
+     * @return String of classpath entries delimited internally by File.pathSeparator 
+     */
     String classpathToString(CompilerRun caller) {
         LangUtil.throwIaxIfNull(caller, "caller");
-        assertState(null != classpath, "classpath not set");
-        StringBuffer sb = new StringBuffer();
-        for (int i = 0; i < classpath.length; i++) {
-          if (i > 0) {
-            sb.append(File.pathSeparator);
-          }
-          sb.append(classpath[i].getAbsolutePath());    
-        }
-        return sb.toString();
+        return FileUtil.flatten(compileClasspath, null);
     }
 }
diff --git a/testing/src/org/aspectj/testing/util/LangUtil.java b/testing/src/org/aspectj/testing/util/LangUtil.java
index 472d35a31..d6b44333c 100644
--- a/testing/src/org/aspectj/testing/util/LangUtil.java
+++ b/testing/src/org/aspectj/testing/util/LangUtil.java
@@ -28,6 +28,7 @@ import java.security.AccessController;
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.BitSet;
 import java.util.Collections;
 import java.util.Comparator;
@@ -225,7 +226,42 @@ public class LangUtil {
         }
     }
 	
-	/** @return ((null == ra) || (0 == ra.length)) */
+    /**
+     * Get indexes of any invalid entries in array.
+     * @param ra the Object[] entries to check
+     *  (if null, this returns new int[] { -1 })
+     * @param superType the Class, if any, to verify that
+     *         any entries are assignable.
+     * @return null if all entries are non-null, assignable to superType
+     * or comma-delimited error String, with components 
+     *     <code>"[#] {null || not {superType}"</code>,
+     * e.g., "[3] null, [5] not String"
+     */
+    public static String invalidComponents(Object[] ra, Class superType) {
+        if (null == ra) {
+            return "null input array";
+        } else if (0 == ra.length) {
+            return null;
+        }
+        StringBuffer result = new StringBuffer();
+        final String cname = LangUtil.unqualifiedClassName(superType);
+        int index = 0;
+        for (int i = 0; i < ra.length; i++) {
+			if (null == ra[i]) {
+                result.append(", [" + i + "] null");
+            } else if ((null != superType) 
+                && !superType.isAssignableFrom(ra[i].getClass())) {
+                result.append(", [" + i + "] not " + cname);
+            }
+		}
+        if (0 == result.length()) {
+            return null;
+        } else {
+            return result.toString().substring(2);
+        }
+    }
+	
+    /** @return ((null == ra) || (0 == ra.length)) */
     public static boolean isEmpty(Object[] ra) {
         return ((null == ra) || (0 == ra.length));
     }
@@ -235,6 +271,26 @@ public class LangUtil {
         return ((null == s) || (0 == s.length()));
     }
 
+
+
+    /**
+     * Throw IllegalArgumentException if any component in input array
+     * is null or (if superType is not null) not assignable to superType.
+     * The exception message takes the form 
+     * <code>{name} invalid entries: {invalidEntriesResult}</code>
+     * @throws IllegalArgumentException if any components bad
+     * @see #invalidComponents(Object[], Class)
+     */
+    public static final void throwIaxIfComponentsBad(
+        final Object[] input, 
+        final String name,
+        final Class superType) {
+        String errs = invalidComponents(input, superType);
+        if (null != errs) {
+            String err = name + " invalid entries: " + errs;
+            throw new IllegalArgumentException(err);
+        }
+    }
 	/**
      * Shorthand for "if false, throw IllegalArgumentException"
      * @throws IllegalArgumentException "{message}" if test is false
diff --git a/testing/src/org/aspectj/testing/util/TestClassLoader.java b/testing/src/org/aspectj/testing/util/TestClassLoader.java
index 2af325e32..ce93f68c1 100644
--- a/testing/src/org/aspectj/testing/util/TestClassLoader.java
+++ b/testing/src/org/aspectj/testing/util/TestClassLoader.java
@@ -31,21 +31,19 @@ import java.util.List;
  */
 public class TestClassLoader extends URLClassLoader {
 
+    /** seek classes in dirs first */
     List /*File*/ dirs;
+
+    /** save URL[] only for toString */
+    private URL[] urlsForDebugString;
     
     public TestClassLoader(URL[] urls, File[] dirs) {
         super(urls);
-        if (null == dirs) {
-            throw new IllegalArgumentException("null dir");
-        }
-        for (int i = 0; i < dirs.length; i++) {
-            if (null == dirs[i]) {
-                throw new IllegalArgumentException("null dir[" + i + "]");
-            }
-		}
+        this.urlsForDebugString = urls;
+        LangUtil.throwIaxIfComponentsBad(dirs, "dirs", null);
         ArrayList dcopy = new ArrayList();
         
-        if ((null != dirs) && (0 < dirs.length)) {
+        if (!LangUtil.isEmpty(dirs)) {
             dcopy.addAll(Arrays.asList(dirs));
         }
         this.dirs = Collections.unmodifiableList(dcopy);
@@ -75,7 +73,6 @@ public class TestClassLoader extends URLClassLoader {
         // and our dirs again (if not maybe test)
         ClassNotFoundException thrown = null;
         final boolean maybeTestClass = maybeTestClassName(name);
-            
         Class result =  findLoadedClass(name);
         if (null != result) {
             resolve = false;
@@ -119,19 +116,16 @@ public class TestClassLoader extends URLClassLoader {
         return result;
     }
     
+    /** @return null if class not found or byte[] of class otherwise */
     private byte[] readClass(String className) throws ClassNotFoundException {
-        byte[] data= null;
         final String fileName = className.replace('.', '/')+".class";
         for (Iterator iter = dirs.iterator(); iter.hasNext();) {
             File file = new File((File) iter.next(), fileName);
             if (file.canRead()) { 
                 return getClassData(file);
             }
-            if (data != null) {
-                return data;
-            }
         }
-        throw new ClassNotFoundException(className); // expensive - fix?
+        return null; 
     }
         
     private byte[] getClassData(File f) {
@@ -150,5 +144,14 @@ public class TestClassLoader extends URLClassLoader {
         }
         return null;
     }
+    
+    /** @return String with debug info: urls and classes used */
+    public String toString() {
+        return "TestClassLoader(urls=" 
+            + Arrays.asList(urlsForDebugString)
+            + ", dirs="
+            + dirs
+            + ")";
+    }
 }
 
diff --git a/testing/src/org/aspectj/testing/xml/AjcSpecXmlReader.java b/testing/src/org/aspectj/testing/xml/AjcSpecXmlReader.java
index cc484f82d..42d0a0e66 100644
--- a/testing/src/org/aspectj/testing/xml/AjcSpecXmlReader.java
+++ b/testing/src/org/aspectj/testing/xml/AjcSpecXmlReader.java
@@ -18,6 +18,7 @@ import org.aspectj.bridge.AbortException;
 import org.aspectj.bridge.IMessage;
 import org.aspectj.bridge.ISourceLocation;
 import org.aspectj.bridge.MessageUtil;
+import org.aspectj.bridge.SourceLocation;
 import org.aspectj.testing.harness.bridge.AbstractRunSpec;
 import org.aspectj.testing.harness.bridge.AjcTest;
 import org.aspectj.testing.harness.bridge.CompilerRun;
@@ -242,8 +243,11 @@ public class AjcSpecXmlReader {
         }
         AjcTest.Suite.Spec result = holder.spec;
         if (null != result) {
+            file = file.getAbsoluteFile();
+            result.setSourceLocation(new SourceLocation(file, 1));
             File suiteDir = file.getParentFile();
             if (null == suiteDir) {
+                // should not be the case if absolute
                 suiteDir = new File("."); // user.dir?
             }
             result.setSuiteDirFile(suiteDir);