fix(tooling-api): logs are not properly appended in tooling API tests

AndroidIDEOfficial · Apr 30, 2024 · fc96f28 · fc96f28
1 parent 0abaeca
commit fc96f28
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 42 deletions.
diff --git a/logger/src/main/java/com/itsaky/androidide/logging/encoder/IDELogFormatEncoder.java b/logger/src/main/java/com/itsaky/androidide/logging/encoder/IDELogFormatEncoder.java
@@ -17,13 +17,18 @@
 
 package com.itsaky.androidide.logging.encoder;
 
+import ch.qos.logback.classic.spi.ILoggingEvent;
 import ch.qos.logback.core.encoder.LayoutWrappingEncoder;
 
 /**
  * Encoder to format the log events in AndroidIDE.
  *
  * @author Akash Yadav
  */
-public class IDELogFormatEncoder extends LayoutWrappingEncoder<IDELogFormatLayout> {
+public class IDELogFormatEncoder extends LayoutWrappingEncoder<ILoggingEvent> {
 
+  public IDELogFormatEncoder() {
+    super();
+    setLayout(new IDELogFormatLayout());
+  }
 }
diff --git a/logger/src/main/resources/logback.xml b/logger/src/main/resources/logback.xml
@@ -16,17 +16,21 @@
   -->
 
 <configuration>
-  <appender name="LOGCAT" class="com.itsaky.androidide.logging.LogcatAppender">
+  <!-- From Logback docs: -->
+  <!-- Recommendation: place status listeners towards the the top of the configuration file -->
+  <statuslistener class="com.itsaky.androidide.tooling.impl.util.LogbackStatusListener" />
+
+  <appender class="com.itsaky.androidide.logging.LogcatAppender" name="LOGCAT">
     <checkLoggable>false</checkLoggable>
     <encoder class="com.itsaky.androidide.logging.encoder.IDELogFormatEncoder" />
   </appender>
-  <appender name="JVM_STDERR" class="com.itsaky.androidide.logging.JvmStdErrAppender">
+  <appender class="com.itsaky.androidide.logging.JvmStdErrAppender" name="JVM_STDERR">
     <encoder class="com.itsaky.androidide.logging.encoder.IDELogFormatEncoder" />
   </appender>
-
+  <appender class="com.itsaky.androidide.tooling.impl.logging.ToolingApiAppender" name="TOOLING" />
   <root level="DEBUG">
     <appender-ref ref="LOGCAT" />
     <appender-ref ref="JVM_STDERR" />
+    <appender-ref ref="TOOLING" />
   </root>
-
 </configuration>
diff --git a/subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/Main.java b/subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/Main.java
@@ -22,7 +22,6 @@
 import com.itsaky.androidide.tooling.api.IToolingApiClient;
 import com.itsaky.androidide.tooling.api.util.ToolingApiLauncher;
 import com.itsaky.androidide.tooling.impl.internal.ProjectImpl;
-import com.itsaky.androidide.tooling.impl.logging.ToolingApiAppender;
 import com.itsaky.androidide.tooling.impl.progress.ForwardingProgressListener;
 import com.itsaky.androidide.tooling.impl.util.LogbackStatusListener;
 import java.io.ByteArrayInputStream;
@@ -44,14 +43,10 @@ public class Main {
   public static IToolingApiClient client;
   public static Future<Void> future;
 
-  private static ToolingApiAppender toolingApiLogAppender;
-
   public static void main(String[] args) {
 
     // disable the JVM std.err appender
     System.setProperty(JvmStdErrAppender.PROP_JVM_STDERR_APPENDER_ENABLED, "false");
-    System.setProperty(CoreConstants.STATUS_LISTENER_CLASS_KEY,
-        LogbackStatusListener.class.getName());
 
     LOG.debug("Starting Tooling API server...");
     final var project = new ProjectImpl();
@@ -62,9 +57,6 @@ public static void main(String[] args) {
     Main.client = (IToolingApiClient) launcher.getRemoteProxy();
     server.connect(client);
 
-    Main.toolingApiLogAppender = new ToolingApiAppender(client);
-    Main.toolingApiLogAppender.attachToRoot();
-
     LOG.debug("Server started. Will run until shutdown message is received...");
     LOG.debug("Running on Java version: {}", System.getProperty("java.version", "<unknown>"));
 
@@ -89,14 +81,11 @@ public static void main(String[] args) {
           server.shutdown().get();
         }
       } catch (InterruptedException | ExecutionException e) {
-        e.printStackTrace();
+        LOG.error("An error occurred while shutting down tooling API server", e);
       } finally {
         Main.future = null;
         Main.client = null;
 
-        Main.toolingApiLogAppender.detachFromRoot();
-        Main.toolingApiLogAppender = null;
-
         LOG.info("Tooling API server shutdown complete");
       }
     }

diff --git a/...g-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/logging/ToolingApiAppender.kt b/...g-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/logging/ToolingApiAppender.kt
@@ -17,44 +17,24 @@
 
 package com.itsaky.androidide.tooling.impl.logging
 
-import ch.qos.logback.classic.Logger
-import ch.qos.logback.classic.LoggerContext
 import ch.qos.logback.classic.spi.ILoggingEvent
 import ch.qos.logback.core.AppenderBase
-import com.itsaky.androidide.tooling.api.IToolingApiClient
 import com.itsaky.androidide.tooling.api.messages.LogMessageParams
-import org.slf4j.LoggerFactory
+import com.itsaky.androidide.tooling.impl.Main
 
 /**
  * [AppenderBase] implementation which forwards all logs to the tooling API client.
  *
  * @author Akash Yadav
  */
-class ToolingApiAppender(
-  private val client: IToolingApiClient
-) : AppenderBase<ILoggingEvent>() {
-
-  fun attachToRoot() {
-    val rootLogger = LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME) as Logger
-    setContext(LoggerFactory.getILoggerFactory() as LoggerContext)
-    start()
-
-    rootLogger.addAppender(this)
-  }
-
-  fun detachFromRoot() {
-    stop()
-
-    val rootLogger = LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME) as Logger
-    rootLogger.detachAppender(this)
-  }
+class ToolingApiAppender : AppenderBase<ILoggingEvent>() {
 
   override fun append(eventObject: ILoggingEvent?) {
     if (eventObject == null || !isStarted) {
       return
     }
 
-    client.logMessage(
+    Main.client?.logMessage(
       LogMessageParams(
         eventObject.level.levelStr[0],
         eventObject.loggerName,

diff --git a/testing/common/src/main/java/com/itsaky/androidide/testing/common/CIOnlyTestRule.kt b/testing/common/src/main/java/com/itsaky/androidide/testing/common/CIOnlyTestRule.kt
@@ -7,9 +7,15 @@ import org.junit.runners.model.Statement
 /**
  * A [TestRule] which only runs the tests in a CI environment.
  */
-class CIOnlyTestRule : TestRule {
+class CIOnlyTestRule(
+  var isEnabled: Boolean = true
+) : TestRule {
 
   override fun apply(base: Statement, description: Description?): Statement {
+    if (!isEnabled) {
+      return base
+    }
+
     return object : Statement() {
       override fun evaluate() {
         if (isCi()) {

diff --git a/...ing/tooling/src/main/java/com/itsaky/androidide/testing/tooling/ToolingApiTestLauncher.kt b/...ing/tooling/src/main/java/com/itsaky/androidide/testing/tooling/ToolingApiTestLauncher.kt
@@ -17,7 +17,7 @@
 
 package com.itsaky.androidide.testing.tooling
 
-import com.google.common.truth.Truth.assertThat
+import ch.qos.logback.core.CoreConstants
 import com.itsaky.androidide.testing.tooling.models.ToolingApiTestLauncherParams
 import com.itsaky.androidide.testing.tooling.models.ToolingApiTestScope
 import com.itsaky.androidide.tooling.api.IProject
@@ -33,6 +33,7 @@ import com.itsaky.androidide.tooling.api.messages.toLogLine
 import com.itsaky.androidide.tooling.api.util.ToolingApiLauncher
 import com.itsaky.androidide.tooling.api.util.ToolingProps
 import com.itsaky.androidide.tooling.events.ProgressEvent
+import com.itsaky.androidide.tooling.impl.util.LogbackStatusListener
 import com.itsaky.androidide.utils.FileProvider
 import com.itsaky.androidide.utils.ILogger
 import org.slf4j.Logger
@@ -182,6 +183,9 @@ object ToolingApiTestLauncher {
       cmd.add("-D$key=$value")
     }
 
+    cmd.add(
+      "-D${CoreConstants.STATUS_LISTENER_CLASS_KEY}=${LogbackStatusListener::class.java.name}")
+
     Collections.addAll(cmd, "-jar", jar)
 
     println(