From: wisberg Date: Mon, 6 Jan 2003 22:50:34 +0000 (+0000) Subject: - test class loader fix for test classes loaded from jars X-Git-Tag: V_1_1_b5~159 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=47b257cb438cc74cf708d7edbb4beb11405402f6;p=aspectj.git - 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 --- 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: + * + * Programmer notes: + * */ 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: - *
  • calculate and set sandbox testBaseSrcDir as - * {Sandbox.testBaseDir}/{Spec.testSrcDirOffset}/
  • - *
  • the list of source File to compile as - * {Sandbox.testBaseSrcDir}/{Spec.getPaths..}
    • + * * 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 + * "[#] {null || not {superType}", + * 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 + * {name} invalid entries: {invalidEntriesResult} + * @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);