aspectj tests are not runnable due to wrong classloader setup

[Andrei-Pozolotin]

in projects such as kamon
https://github.com/kamon-io/Kamon

eclipse/scala-test aspectj tests are not runnable due to wrong classloader setup
https://github.com/scalatest/scalatest-eclipse-plugin/blob/master/org.scala-ide.sdt.scalatest/src/scala/tools/eclipse/scalatest/launching/ScalaTestLauncher.scala#L45

problem arises as follows:

1. scala-test is launched with -javaagent:aspectjweaving.jar

• system classloader is empty and injected by jvm into aspectj
• aspectj finds no META-INF/aop.xml and disables any further class transform form system classloader

2. scala-test launcher is hacking system classloader in main() by adding all the jars from eclipse class path, but aspectj will never see the change

3. all following tests fail, since META-INF/aop.xml are present, but ignored by aspectj.

solution:

• create a class path url array first and build child classloader from final array
• when child classloader is injected by jvm into aspectj, it will discover all metadata

[Andrei-Pozolotin]

work around by instrumenting scala test launcher:

@Aspect
public class ScalaTestAspect {

    static final String RUNNER_CLASS = "org.scalatest.tools.Runner";

    static final String LOGGER_PROPERY = "scala.tools.eclipse.scalatest.launching.ScalaTestLauncher.logging";

    static final Logger logger = Logger.getLogger(ScalaTestAspect.class
            .getName());

    void log(final String title, final List<String> lineList) {
        final StringBuilder text = new StringBuilder();
        for (final String line : lineList) {
            text.append(line);
            text.append("\n");
        }
        logger.info(title + "\n" + text.toString());
    }

    @Pointcut("execution(* scala.tools.eclipse.scalatest.launching.ScalaTestLauncher.main(*)) && args(args)")
    public void ScalaTestLauncherMain(final String[] args) {
    }

    @Around("ScalaTestLauncherMain(args)")
    public void ScalaTestLauncherMainAround(final ProceedingJoinPoint point,
            final String[] args) throws Exception {

        final Charset charset = Charset.forName("UTF-8");

        final String cpFilePath = args[0];
        final String mainArgsPath = args[1];

        final List<String> testPathList = Files.readAllLines(
                Paths.get(cpFilePath), charset);

        final int cpListSize = testPathList.size();

        final URL[] urls = new URL[cpListSize];

        for (int index = 0; index < cpListSize; index++) {
            urls[index] = new File(testPathList.get(index)).toURI().toURL();
        }

        final List<String> mainArgsList = Files.readAllLines(
                Paths.get(mainArgsPath), charset);

        if (Boolean.parseBoolean(System.getProperty(LOGGER_PROPERY, "false"))) {
            log("Test classpath:", testPathList);
            log("Main arguments:", mainArgsList);
        }

        final URLClassLoader loader = new URLClassLoader(urls,
                ClassLoader.getSystemClassLoader());

        Thread.currentThread().setContextClassLoader(loader);

        final Class<?> runnerClass = loader.loadClass(RUNNER_CLASS);

        final Method mainMethod = runnerClass
                .getMethod("main", args.getClass());

        mainMethod.setAccessible(true);

        mainMethod.invoke(null,
                new Object[] { mainArgsList.toArray(new String[] {}) });

    }

}

[cheeseng]

LGTM for the changes in following commit:

carrot-garden@c2594bc

Like many projects, we do ask for a signed CLA from contributors, would you mind to signing the following CLA first:

http://www.artima.com/cla/contributorsLicenseAgreement.pdf

Thanks. :)

[Andrei-Pozolotin]

1. I emailed Artima CLA

2. please upgrade your CLA to use google docs, such as:
   https://docs.google.com/forms/d/1G_IDrBTFzOMwHvhxfKRBwNtpRelSa_MZ6jecH8lpTlc/viewform

[cheeseng]

@Andrei-Pozolotin Thanks! We'll pull the changes in and build a version today, and thanks also for the CLA suggestion to use google docs, we'll look into it.

[cheeseng]

I merged and rebuilt the plugin, now we need to wait for the Scala IDE update site to pick it up, normally within a day.

[cheeseng]

@Andrei-Pozolotin Scala IDE has picked up the new version, can you verify if it fixes your problem?

[Andrei-Pozolotin]

@cheeseng great, thank you.

1. the kamon project testing issues are resolved

2. but you are missing one more commit from PR
   carrot-garden@7410d77
   it is critical when classloader must inherit from TCCL, please pull.

3. there is an annoying banner

WARNING: -p has been deprecated and will be reused for a different (but still very cool) purpose in ScalaTest 2.0. Please change all uses of -p to -R.

since you are the one who controls the plugin, can you

Please change all uses of -p to -R.

thanks

[cheeseng]

@Andrei-Pozolotin Yes sorry I missed that commit, it is getting late over here, I shall be able to work on 2) when start tomorrow.

As for the 3), it was an easy old workaround to support both ScalaTest 1.x and 2.x (ScalaTest 1.x uses -p while ScalaTest 2.x deprecated -p and uses -R, and that annoying banner comes from ScalaTest 2.x actually), we do have a better way to detect the ScalaTest version now, so I think we can fix this one too.

[cheeseng]

@Andrei-Pozolotin Sorry got distracted, I built a new version that addresses the mentioned issue. Let's wait for the Scala IDE update site to pick it up.

[Andrei-Pozolotin]

still do not see the change. how long does it normally take?

[cheeseng]

@Andrei-Pozolotin Just wake up in the morning and seems like it is not being picked up yet, they told me it is within one day, I uploaded it at 6pm GMT+8 so I'll wait for a little longer, and if it is still not being picked after 24 hours I'll try to ping them. :)

[Andrei-Pozolotin]

thanks

[cheeseng]

@Andrei-Pozolotin just an update, I reported the problem here:

https://groups.google.com/forum/#!topic/scala-ide-dev/VPQOL1W8Np4

hopefully can get a feedback soon.

[cheeseng]

@Andrei-Pozolotin it is picked up, can you update to try and see if it fixes your issues?

[Andrei-Pozolotin]

@cheeseng yes, I see the change, and it does resolve the issue. thank you.