Add better error message if run fails

It is possible with the new layering strategies that tests may fail if a
java package private class is accessed across classloader layers. This
will result in an IllegalAccessError that is hard to debug. With this
commit, I add an error message that will be displayed if run throws an
IllegalAccessError that suggests that the user try the
ScalaInstance layering strategy or the flat layering strategy.

main/src/main/scala/sbt/Defaults.scala

@@ -1056,22 +1056,21 @@ object Defaults extends BuildCommon {
     val result = output map { out =>
       out.events.foreach {
         case (suite, e) =>
-          e.throwables
-            .collectFirst {
-              case t
-                  if t
-                    .isInstanceOf[NoClassDefFoundError] && strategy != ClassLoaderLayeringStrategy.Flat =>
-                t
-            }
-            .foreach { t =>
-              s.log.error(
-                s"Test suite $suite failed with $t. This may be due to the ClassLoaderLayeringStrategy"
-                  + s" ($strategy) used by your task. This issue may be resolved by changing the"
-                  + " ClassLoaderLayeringStrategy in your configuration (generally Test or IntegrationTest),"
-                  + "e.g.:\nTest / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.Flat\n"
-                  + "See ClassLoaderLayeringStrategy.scala for the full list of options."
-              )
-            }
+          if (strategy != ClassLoaderLayeringStrategy.Flat) {
+            e.throwables
+              .find { t =>
+                t.isInstanceOf[NoClassDefFoundError] || t.isInstanceOf[IllegalAccessError]
+              }
+              .foreach { t =>
+                s.log.error(
+                  s"Test suite $suite failed with $t. This may be due to the ClassLoaderLayeringStrategy"
+                    + s" ($strategy) used by your task. This issue may be resolved by changing the"
+                    + " ClassLoaderLayeringStrategy in your configuration (generally Test or IntegrationTest),"
+                    + " e.g.:\nTest / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.Flat\n"
+                    + "See ClassLoaderLayeringStrategy.scala for the full list of options."
+                )
+              }
+          }
       }
       val summaries =
         runners map {

run/src/main/scala/sbt/Run.scala

@@ -93,11 +93,26 @@ class Run(newLoader: Seq[File] => ClassLoader, trapExit: Boolean) extends ScalaR
     val main = getMainMethod(mainClassName, loader)
     invokeMain(loader, main, options)
   }
-  private def invokeMain(loader: ClassLoader, main: Method, options: Seq[String]): Unit = {
+  private def invokeMain(
+      loader: ClassLoader,
+      main: Method,
+      options: Seq[String]
+  ): Unit = {
     val currentThread = Thread.currentThread
     val oldLoader = Thread.currentThread.getContextClassLoader
     currentThread.setContextClassLoader(loader)
-    try { main.invoke(null, options.toArray[String]); () } finally {
+    try { main.invoke(null, options.toArray[String]); () } catch {
+      case t: Throwable =>
+        t.getCause match {
+          case e: java.lang.IllegalAccessError =>
+            val msg = s"Error running $main.\n$e\n" +
+              "If using a layered classloader, this can occur if jvm package private classes are " +
+              "accessed across layers. This can be fixed by changing to the Flat or " +
+              "ScalaInstance class loader layering strategies."
+            throw new IllegalAccessError(msg)
+          case _ => throw t
+        }
+    } finally {
       currentThread.setContextClassLoader(oldLoader)
     }
   }

sbt/src/sbt-test/classloader-cache/package-private/build.sbt

@@ -0,0 +1 @@
+libraryDependencies += "org.scala-sbt.ivy" % "ivy" % "2.3.0-sbt-cb9cc189e9f3af519f9f102e6c5d446488ff6832"

sbt/src/sbt-test/classloader-cache/package-private/src/main/scala/sbt/classpath/Run.scala

@@ -0,0 +1,12 @@
+package org.apache.ivy.plugins.parser.m2
+
+import org.apache.ivy.core.module.descriptor.DefaultDependencyDescriptor
+
+object Run {
+  def main(args: Array[String]): Unit = {
+    new PomModuleDescriptorBuilder.ConfMapper {
+      override def addMappingConfs(dd: DefaultDependencyDescriptor, isOptional: Boolean): Unit = {}
+    }
+    ()
+  }
+}

sbt/src/sbt-test/classloader-cache/package-private/test

@@ -0,0 +1,9 @@
+-> run
+
+> set Compile / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.Flat
+
+> run
+
+> set Runtime / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.ScalaInstance
+
+> run

scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala

@@ -177,6 +177,7 @@ final class ScriptedTests(
       case "classloader-cache/jni"                                  => LauncherBased // sbt/Package$
       case "classloader-cache/library-mismatch"                     => LauncherBased // sbt/Package$
       case "classloader-cache/runtime-layers"                       => LauncherBased // sbt/Package$
+      case "classloader-cache/package-private"                      => LauncherBased // sbt/Package$
       case "compiler-project/dotty-compiler-plugin"                 => LauncherBased // sbt/Package$
       case "compiler-project/run-test"                              => LauncherBased // sbt/Package$
       case "compiler-project/src-dep-plugin"                        => LauncherBased // sbt/Package$

testing/src/main/scala/sbt/TestFramework.scala

@@ -122,9 +122,25 @@ final class TestRunner(
       val results = new scala.collection.mutable.ListBuffer[Event]
       val handler = new EventHandler { def handle(e: Event): Unit = { results += e } }
       val loggers: Vector[ContentLogger] = listeners.flatMap(_.contentLogger(testDefinition))
+      def errorEvents(e: Throwable): Array[sbt.testing.Task] = {
+        val taskDef = testTask.taskDef
+        val event = new Event {
+          val status = Status.Error
+          val throwable = new OptionalThrowable(e)
+          val fullyQualifiedName = taskDef.fullyQualifiedName
+          val selector = new TestSelector(name)
+          val fingerprint = taskDef.fingerprint
+          val duration = -1L
+        }
+        results += event
+        Array.empty
+      }
       val nestedTasks =
         try testTask.execute(handler, loggers.map(_.log).toArray)
-        finally {
+        catch {
+          case NonFatal(e)           => errorEvents(e)
+          case e: IllegalAccessError => errorEvents(e)
+        } finally {
           loggers.foreach(_.flush())
         }
       val event = TestEvent(results)