From edc98593b9fec70cdf6c582587b1508ca711d3dd Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 21 Dec 2023 14:45:33 +0100 Subject: [PATCH] All unit tests use the same testing logging provider (#8593) Make sure that the correct test logging provider is loaded in `project-manager/Test`, so that only WARN and ERROR log messages are displayed. Also, make sure that the test log provider parses the correct configuration file - Rename all the `application.conf` files in the test resources to `application-test.conf`. The problem was introduced in #8467 --- build.sbt | 25 ++- ...application.conf => application-test.conf} | 0 ...application.conf => application-test.conf} | 0 .../test/TestLogProviderOnClasspath.java | 38 ++++ ...application.conf => application-test.conf} | 0 .../src/test/resources/application-test.conf | 1 + .../src/test/resources/application-test.conf | 1 + lib/scala/logger/README.md | 3 - .../src/main/java/org/enso/package-info.java | 1 - .../src/main/scala/org/enso/Logger.scala | 165 ------------------ .../java/org/enso/logger/TestLogProvider.java | 1 + .../TestLogProviderOnClasspath.java | 46 +++++ ...application.conf => application-test.conf} | 0 ...application.conf => application-test.conf} | 0 14 files changed, 99 insertions(+), 182 deletions(-) rename engine/language-server/src/test/resources/{application.conf => application-test.conf} (100%) rename engine/launcher/src/test/resources/{application.conf => application-test.conf} (100%) create mode 100644 engine/runtime/src/test/java/org/enso/interpreter/test/TestLogProviderOnClasspath.java rename engine/runtime/src/test/resources/{application.conf => application-test.conf} (100%) create mode 100644 lib/scala/filewatcher/src/test/resources/application-test.conf create mode 100644 lib/scala/library-manager-test/src/test/resources/application-test.conf delete mode 100644 lib/scala/logger/README.md delete mode 100644 lib/scala/logger/src/main/java/org/enso/package-info.java delete mode 100644 lib/scala/logger/src/main/scala/org/enso/Logger.scala create mode 100644 lib/scala/project-manager/src/test/java/org/enso/projectmanager/TestLogProviderOnClasspath.java rename lib/scala/project-manager/src/test/resources/{application.conf => application-test.conf} (100%) rename lib/scala/runtime-version-manager-test/src/test/resources/{application.conf => application-test.conf} (100%) diff --git a/build.sbt b/build.sbt index 93033b50e5b0..2fc6869592c3 100644 --- a/build.sbt +++ b/build.sbt @@ -272,7 +272,6 @@ lazy val enso = (project in file(".")) `syntax-definition`, `syntax-rust-definition`, `text-buffer`, - logger, pkg, cli, `task-progress-notifications`, @@ -528,13 +527,6 @@ lazy val compileModuleInfo = taskKey[Unit]("Compiles `module-info.java`") // === Internal Libraries ===================================================== // ============================================================================ -lazy val logger = (project in file("lib/scala/logger")) - .settings( - frgaalJavaCompilerSetting, - version := "0.1", - libraryDependencies ++= scalaCompiler - ) - lazy val `syntax-definition` = project in file("lib/scala/syntax/definition") @@ -907,7 +899,10 @@ lazy val `project-manager` = (project in file("lib/scala/project-manager")) "org.apache.commons" % "commons-lang3" % commonsLangVersion, "com.beachape" %% "enumeratum-circe" % enumeratumCirceVersion, "com.miguno.akka" %% "akka-mock-scheduler" % akkaMockSchedulerVersion % Test, - "org.mockito" %% "mockito-scala" % mockitoScalaVersion % Test + "org.mockito" %% "mockito-scala" % mockitoScalaVersion % Test, + "junit" % "junit" % junitVersion % Test, + "com.github.sbt" % "junit-interface" % junitIfVersion % Test, + "org.hamcrest" % "hamcrest-all" % hamcrestVersion % Test ), addCompilerPlugin( "org.typelevel" %% "kind-projector" % kindProjectorVersion cross CrossVersion.full @@ -1016,7 +1011,7 @@ lazy val `project-manager` = (project in file("lib/scala/project-manager")) .dependsOn(`json-rpc-server-test` % Test) .dependsOn(testkit % Test) .dependsOn(`runtime-version-manager-test` % Test) - .dependsOn(`logging-service-logback` % Test) + .dependsOn(`logging-service-logback` % "test->test") /* Note [Classpath Separation] * ~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -1207,8 +1202,13 @@ val truffleRunOptionsSettings = Seq( javaOptions ++= "-ea" +: benchOnlyOptions ) +/** Explicitly provide `application-test.conf` as the resource that should be used for + * parsing the logging configuration. Explicitly setting `config.resource` prevents + * the potential conflicts with other *.conf files. + */ val testLogProviderOptions = Seq( - "-Dslf4j.provider=org.enso.logger.TestLogProvider" + "-Dslf4j.provider=org.enso.logger.TestLogProvider", + "-Dconfig.resource=application-test.conf" ) lazy val `polyglot-api` = project @@ -2126,8 +2126,7 @@ lazy val launcher = project .dependsOn(buildNativeImage) .dependsOn(LauncherShimsForTest.prepare()) .evaluated, - Test / fork := true, - Test / javaOptions += s"-Dconfig.file=${sourceDirectory.value}/test/resources/application.conf" + Test / fork := true ) .dependsOn(cli) .dependsOn(`runtime-version-manager`) diff --git a/engine/language-server/src/test/resources/application.conf b/engine/language-server/src/test/resources/application-test.conf similarity index 100% rename from engine/language-server/src/test/resources/application.conf rename to engine/language-server/src/test/resources/application-test.conf diff --git a/engine/launcher/src/test/resources/application.conf b/engine/launcher/src/test/resources/application-test.conf similarity index 100% rename from engine/launcher/src/test/resources/application.conf rename to engine/launcher/src/test/resources/application-test.conf diff --git a/engine/runtime/src/test/java/org/enso/interpreter/test/TestLogProviderOnClasspath.java b/engine/runtime/src/test/java/org/enso/interpreter/test/TestLogProviderOnClasspath.java new file mode 100644 index 000000000000..31ab81725ede --- /dev/null +++ b/engine/runtime/src/test/java/org/enso/interpreter/test/TestLogProviderOnClasspath.java @@ -0,0 +1,38 @@ +package org.enso.interpreter.test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; + +import java.util.ArrayList; +import java.util.List; +import java.util.ServiceLoader; +import java.util.stream.Collectors; +import org.junit.Test; +import org.slf4j.spi.SLF4JServiceProvider; + +/** + * In the `runtime/Test` testing suite, {@link org.enso.logger.TestLogProvider} should be among the + * logging providers, because it is explicitly chosen as the logging provider for the tests. + */ +public class TestLogProviderOnClasspath { + @Test + public void testLogProviderOnClasspath() { + var sl = ServiceLoader.load(SLF4JServiceProvider.class); + var serviceIterator = sl.iterator(); + List providers = new ArrayList<>(); + while (serviceIterator.hasNext()) { + providers.add(serviceIterator.next()); + } + List providerNames = + providers.stream().map(elem -> elem.getClass().getName()).collect(Collectors.toList()); + assertThat(providerNames, hasItem("org.enso.logger.TestLogProvider")); + } + + @Test + public void testLogProviderIsInUnnamedModule() throws Exception { + var testLogProviderClass = Class.forName("org.enso.logger.TestLogProvider"); + var mod = testLogProviderClass.getModule(); + assertThat(mod, notNullValue()); + assertThat("Should be an unnamed module - with null name", mod.getName(), nullValue()); + } +} diff --git a/engine/runtime/src/test/resources/application.conf b/engine/runtime/src/test/resources/application-test.conf similarity index 100% rename from engine/runtime/src/test/resources/application.conf rename to engine/runtime/src/test/resources/application-test.conf diff --git a/lib/scala/filewatcher/src/test/resources/application-test.conf b/lib/scala/filewatcher/src/test/resources/application-test.conf new file mode 100644 index 000000000000..b7db25411d06 --- /dev/null +++ b/lib/scala/filewatcher/src/test/resources/application-test.conf @@ -0,0 +1 @@ +# Empty diff --git a/lib/scala/library-manager-test/src/test/resources/application-test.conf b/lib/scala/library-manager-test/src/test/resources/application-test.conf new file mode 100644 index 000000000000..b7db25411d06 --- /dev/null +++ b/lib/scala/library-manager-test/src/test/resources/application-test.conf @@ -0,0 +1 @@ +# Empty diff --git a/lib/scala/logger/README.md b/lib/scala/logger/README.md deleted file mode 100644 index 891fb5ce0aee..000000000000 --- a/lib/scala/logger/README.md +++ /dev/null @@ -1,3 +0,0 @@ -# Logger - -A configurable logging library. diff --git a/lib/scala/logger/src/main/java/org/enso/package-info.java b/lib/scala/logger/src/main/java/org/enso/package-info.java deleted file mode 100644 index 8d7814555bc6..000000000000 --- a/lib/scala/logger/src/main/java/org/enso/package-info.java +++ /dev/null @@ -1 +0,0 @@ -package org.enso; diff --git a/lib/scala/logger/src/main/scala/org/enso/Logger.scala b/lib/scala/logger/src/main/scala/org/enso/Logger.scala deleted file mode 100644 index 874ed3474fdf..000000000000 --- a/lib/scala/logger/src/main/scala/org/enso/Logger.scala +++ /dev/null @@ -1,165 +0,0 @@ -package org.enso - -import scala.annotation.unused -import scala.reflect.macros.blackbox.Context - -class Logger { - import Logger._ - - var nesting = 0 - - def log(s: String): Unit = - macro funcRedirect - - def warn(s: String): Unit = - macro funcRedirect - - def err(s: String): Unit = - macro funcRedirect - - def group[T](msg: String)(body: => T): T = - macro groupRedirect[T] - - def trace[T](body: => T): T = - macro targetRedirect[T] - - def trace_[T](body: => T): T = - macro targetRedirect_[T] - - def _log(msg: String): Unit = - println("| " * nesting + msg) - - def _warn(msg: String): Unit = - println("| " * nesting + Console.YELLOW + msg + Console.RESET) - - def _err(msg: String): Unit = - println("| " * nesting + Console.RED + msg + Console.RESET) - - def _group[T](msg: String)(body: => T): T = { - _log(msg) - beginGroup() - val out = body - endGroup() - out - } - - def _trace[T](msg: String)(body: => T): T = { - _log(msg) - beginGroup() - val out = body - endGroup() - out - } - - def _trace_[T](msg: String)(body: => T): T = { - _log(msg) - beginGroup() - val out = body - endGroup() - out - } - - def beginGroup(): Unit = - nesting += 1 - - def endGroup(): Unit = - nesting -= 1 - -} - -object Logger { - def groupRedirect[R: c.WeakTypeTag]( - c: Context - )(@unused msg: c.Tree)(body: c.Tree): c.Expr[R] = { - import c.universe._ - val target = c.macroApplication match { - case Apply(Apply(TypeApply(Select(base, name), typ), msg2), body2) => - val newName = TermName("_" + name.toString) - Apply(Apply(TypeApply(Select(base, newName), typ), msg2), body2) - case _ => throw new Error("Unsupported shape") - } - if (checkEnabled(c)) c.Expr(q"$target") else c.Expr(q"$body") - } - - def targetRedirect[R: c.WeakTypeTag](c: Context)(body: c.Tree): c.Expr[R] = { - import c.universe._ - val target = c.macroApplication match { - case Apply(TypeApply(Select(base, name), typ), body2) => - val newName = TermName("_" + name.toString) - val owner = c.internal.enclosingOwner.asMethod - val owner2 = owner.owner - val parentObject = !owner2.isStatic - val oname = - if (parentObject) owner2.name.toString + "." + owner.name.toString - else owner.name.toString - val ownerName = Literal(Constant(oname)) - owner.paramLists match { - case lst :: _ => - val lst2 = lst.map(x => q"$x") - val msg = - if (lst2.isEmpty) List(q"$ownerName") - else List(q"$ownerName + $lst2.toString().drop(4)") - Apply(Apply(TypeApply(Select(base, newName), typ), msg), body2) - case _ => throw new Error("Unsupported shape") - } - case _ => throw new Error("Unsupported shape") - } - if (checkEnabled(c)) c.Expr(q"$target") else c.Expr(q"$body") - } - - def targetRedirect_[R: c.WeakTypeTag](c: Context)(body: c.Tree): c.Expr[R] = { - import c.universe._ - val target = c.macroApplication match { - case Apply(TypeApply(Select(base, name), typ), body2) => - val newName = TermName("_" + name.toString) - val owner = c.internal.enclosingOwner.asMethod - val owner2 = owner.owner - val parentObject = !owner2.isStatic - val oname = - if (parentObject) owner2.name.toString + "." + owner.name.toString - else owner.name.toString - val ownerName = Literal(Constant(oname)) - owner.paramLists match { - case _ :: _ => - val msg = List(q"$ownerName") - Apply(Apply(TypeApply(Select(base, newName), typ), msg), body2) - case _ => throw new Error("Unsupported shape") - } - case _ => throw new Error("Unsupported shape") - } - if (checkEnabled(c)) c.Expr(q"$target") else c.Expr(q"$body") - } - - def funcRedirect(c: Context)(@unused s: c.Tree): c.Expr[Unit] = { - import c.universe._ - val target = c.macroApplication match { - case Apply(Select(base, name), args) => - val newName = TermName("_" + name.toString) - Apply(Select(base, newName), args) - case _ => throw new Error("Unsupported shape") - } - if (checkEnabled(c)) c.Expr(q"$target") else c.Expr(q"{}") - } - - def checkEnabled(c: Context): Boolean = { - val optPfx = "logging" - val opts = c.settings.filter(_.matches(s"(\\+|\\-)$optPfx.*")) - val owner = c.internal.enclosingOwner.fullName - var enabled = true - opts.foreach { opt => - val sign = opt.head - val body = opt.tail.drop(optPfx.length) - val status = sign == '+' - val applies = - if (body == "") true - else { - val pathPfx = body.head - val path = body.tail - pathPfx == '@' && owner.startsWith(path) - } - if (applies) enabled = status - } - enabled - } - -} diff --git a/lib/scala/logging-service-logback/src/test/java/org/enso/logger/TestLogProvider.java b/lib/scala/logging-service-logback/src/test/java/org/enso/logger/TestLogProvider.java index 3fc5870a084c..f8ccb63e9294 100644 --- a/lib/scala/logging-service-logback/src/test/java/org/enso/logger/TestLogProvider.java +++ b/lib/scala/logging-service-logback/src/test/java/org/enso/logger/TestLogProvider.java @@ -18,6 +18,7 @@ public class TestLogProvider implements SLF4JServiceProvider { @Override public ILoggerFactory getLoggerFactory() { ILoggerFactory factory = underlying.getLoggerFactory(); + assert factory instanceof LoggerContext; if (!initialized) { try { new LogbackSetup((LoggerContext) factory).setup(); diff --git a/lib/scala/project-manager/src/test/java/org/enso/projectmanager/TestLogProviderOnClasspath.java b/lib/scala/project-manager/src/test/java/org/enso/projectmanager/TestLogProviderOnClasspath.java new file mode 100644 index 000000000000..4de17cdbd2b7 --- /dev/null +++ b/lib/scala/project-manager/src/test/java/org/enso/projectmanager/TestLogProviderOnClasspath.java @@ -0,0 +1,46 @@ +package org.enso.projectmanager; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.fail; + +import java.util.ArrayList; +import java.util.List; +import java.util.ServiceLoader; +import java.util.stream.Collectors; +import org.junit.Test; +import org.slf4j.spi.SLF4JServiceProvider; + +/** + * In the `runtime/Test` testing suite, {@link }org.enso.logger.TestLogProvider} should be among the + * logging providers, because it is explicitly chosen as the logging provider for the tests. + * + *

Note that the same test is in the `runtime/Test` project. + */ +public class TestLogProviderOnClasspath { + @Test + public void testLogProviderIsOnClasspath() { + var sl = ServiceLoader.load(SLF4JServiceProvider.class); + var serviceIterator = sl.iterator(); + List providers = new ArrayList<>(); + while (serviceIterator.hasNext()) { + providers.add(serviceIterator.next()); + } + List providerNames = + providers.stream().map(elem -> elem.getClass().getName()).collect(Collectors.toList()); + assertThat(providerNames, hasItem("org.enso.logger.TestLogProvider")); + } + + @Test + public void testLogProviderIsInUnnamedModule() { + Class testLogProviderClass = null; + try { + testLogProviderClass = Class.forName("org.enso.logger.TestLogProvider"); + } catch (ClassNotFoundException e) { + fail("TestLogProvider class not found"); + } + var mod = testLogProviderClass.getModule(); + assertThat(mod, notNullValue()); + assertThat("Should be an unnamed module - with null name", mod.getName(), nullValue()); + } +} diff --git a/lib/scala/project-manager/src/test/resources/application.conf b/lib/scala/project-manager/src/test/resources/application-test.conf similarity index 100% rename from lib/scala/project-manager/src/test/resources/application.conf rename to lib/scala/project-manager/src/test/resources/application-test.conf diff --git a/lib/scala/runtime-version-manager-test/src/test/resources/application.conf b/lib/scala/runtime-version-manager-test/src/test/resources/application-test.conf similarity index 100% rename from lib/scala/runtime-version-manager-test/src/test/resources/application.conf rename to lib/scala/runtime-version-manager-test/src/test/resources/application-test.conf