google · copybara-service · Nov 25, 2024
diff --git a/src/devtools/mobileharness/infra/ats/server/proto/service.proto b/src/devtools/mobileharness/infra/ats/server/proto/service.proto
@@ -185,6 +185,7 @@ enum ErrorReason {
   INVALID_RESOURCE = 4;
   TOO_MANY_LOST_DEVICES = 5;
   RESULT_PROCESSING_ERROR = 6;
+  TRADEFED_INVOCATION_ERROR = 7;
 }
 
 message CommandAttemptDetail {

diff --git a/src/java/com/google/devtools/mobileharness/infra/ats/common/BUILD b/src/java/com/google/devtools/mobileharness/infra/ats/common/BUILD
@@ -228,6 +228,7 @@ java_library(
         "//src/java/com/google/devtools/mobileharness/platform/android/xts/plugin:__pkg__",
         "//src/javatests/com/google/devtools/mobileharness/infra/ats/common/jobcreator:__pkg__",
         "//src/javatests/com/google/devtools/mobileharness/infra/ats/console/controller/sessionplugin:__pkg__",
+        "//src/javatests/com/google/devtools/mobileharness/infra/ats/server/sessionplugin:__pkg__",
         "//src/javatests/com/google/devtools/mobileharness/platform/android/xts/plugin:__pkg__",
     ],
     deps = ["//src/java/com/google/wireless/qa/mobileharness/shared/constant:property"],

diff --git a/src/java/com/google/devtools/mobileharness/infra/ats/common/SessionResultHandlerUtil.java b/src/java/com/google/devtools/mobileharness/infra/ats/common/SessionResultHandlerUtil.java
@@ -546,16 +546,7 @@ private Optional<Result> processResultHelper(
    */
   private void copyTradefedTestLogFiles(TestInfo tradefedTestInfo, Path logRootDir)
       throws MobileHarnessException, InterruptedException {
-    Path invocationDir;
-    if (tradefedTestInfo.properties().has(XtsConstants.TRADEFED_INVOCATION_DIR_NAME)) {
-      invocationDir =
-          logRootDir.resolve(
-              tradefedTestInfo.properties().get(XtsConstants.TRADEFED_INVOCATION_DIR_NAME));
-    } else {
-      invocationDir =
-          localFileUtil.createTempDir(logRootDir, XtsConstants.TRADEFED_INVOCATION_DIR_NAME_PREFIX);
-      localFileUtil.setFilePermission(invocationDir, "rwxr-xr-x");
-    }
+    Path invocationDir = prepareTradefedInvocationDir(tradefedTestInfo, logRootDir);
     Path testLogDir = prepareLogOrResultDirForTest(tradefedTestInfo, invocationDir);
 
     ImmutableList<Path> genFiles = getGenFilesFromTest(tradefedTestInfo);
@@ -844,6 +835,28 @@ private Optional<String> getLabGenFileDir(TestInfo test) {
     }
   }
 
+  public Path getTradefedInvocationLogDir(TestInfo tradefedTestInfo, Path logRootDir)
+      throws MobileHarnessException {
+    Path invocationDir = prepareTradefedInvocationDir(tradefedTestInfo, logRootDir);
+    return prepareLogOrResultDirForTest(tradefedTestInfo, invocationDir);
+  }
+
+  private Path prepareTradefedInvocationDir(TestInfo tradefedTestInfo, Path logRootDir)
+      throws MobileHarnessException {
+    Path invocationDir;
+    if (tradefedTestInfo.properties().has(XtsConstants.TRADEFED_INVOCATION_DIR_NAME)) {
+      invocationDir =
+          logRootDir.resolve(
+              tradefedTestInfo.properties().get(XtsConstants.TRADEFED_INVOCATION_DIR_NAME));
+    } else {
+      invocationDir =
+          localFileUtil.createTempDir(logRootDir, XtsConstants.TRADEFED_INVOCATION_DIR_NAME_PREFIX);
+      localFileUtil.setFilePermission(invocationDir, "rwxr-xr-x");
+    }
+
+    return invocationDir;
+  }
+
   private Path prepareLogOrResultDirForTest(TestInfo test, Path parentDir)
       throws MobileHarnessException {
     Path targetDir =

diff --git a/src/java/com/google/devtools/mobileharness/infra/ats/server/sessionplugin/BUILD b/src/java/com/google/devtools/mobileharness/infra/ats/server/sessionplugin/BUILD
@@ -88,6 +88,7 @@ java_library(
         "//src/java/com/google/devtools/mobileharness/infra/client/longrunningservice/model:session_info",
         "//src/java/com/google/devtools/mobileharness/infra/lab/common/dir",
         "//src/java/com/google/devtools/mobileharness/platform/android/xts/common/util:xts_constants",
+        "//src/java/com/google/devtools/mobileharness/platform/android/xts/runtime:xts_tradefed_runtime_info_file_util",
         "//src/java/com/google/devtools/mobileharness/shared/util/auto:auto_value",
         "//src/java/com/google/devtools/mobileharness/shared/util/command",
         "//src/java/com/google/devtools/mobileharness/shared/util/error:more_throwables",

diff --git a/.../devtools/mobileharness/infra/ats/server/sessionplugin/NewMultiCommandRequestHandler.java b/.../devtools/mobileharness/infra/ats/server/sessionplugin/NewMultiCommandRequestHandler.java
@@ -16,9 +16,11 @@
 
 package com.google.devtools.mobileharness.infra.ats.server.sessionplugin;
 
+import static com.google.common.base.Strings.isNullOrEmpty;
 import static com.google.common.base.Throwables.getStackTraceAsString;
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
+import static com.google.common.collect.Iterables.getLast;
 import static com.google.devtools.mobileharness.shared.util.error.MoreThrowables.shortDebugString;
 import static com.google.devtools.mobileharness.shared.util.time.TimeUtils.toJavaDuration;
 import static java.nio.charset.StandardCharsets.UTF_8;
@@ -44,6 +46,7 @@
 import com.google.devtools.mobileharness.infra.ats.common.SessionRequestInfo;
 import com.google.devtools.mobileharness.infra.ats.common.SessionResultHandlerUtil;
 import com.google.devtools.mobileharness.infra.ats.common.XtsPropertyName;
+import com.google.devtools.mobileharness.infra.ats.common.XtsPropertyName.Job;
 import com.google.devtools.mobileharness.infra.ats.common.XtsTypeLoader;
 import com.google.devtools.mobileharness.infra.ats.common.jobcreator.XtsJobCreator;
 import com.google.devtools.mobileharness.infra.ats.common.proto.XtsCommonProto.ShardingMode;
@@ -62,6 +65,8 @@
 import com.google.devtools.mobileharness.infra.client.longrunningservice.model.SessionInfo;
 import com.google.devtools.mobileharness.infra.lab.common.dir.DirUtil;
 import com.google.devtools.mobileharness.platform.android.xts.common.util.XtsConstants;
+import com.google.devtools.mobileharness.platform.android.xts.runtime.XtsTradefedRuntimeInfoFileUtil;
+import com.google.devtools.mobileharness.platform.android.xts.runtime.XtsTradefedRuntimeInfoFileUtil.XtsTradefedRuntimeInfoFileDetail;
 import com.google.devtools.mobileharness.shared.util.command.Command;
 import com.google.devtools.mobileharness.shared.util.command.CommandExecutor;
 import com.google.devtools.mobileharness.shared.util.file.local.LocalFileUtil;
@@ -70,6 +75,7 @@
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import com.google.protobuf.util.Timestamps;
 import com.google.wireless.qa.mobileharness.shared.model.job.JobInfo;
+import com.google.wireless.qa.mobileharness.shared.model.job.TestInfo;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
@@ -118,6 +124,7 @@ final class NewMultiCommandRequestHandler {
   private final SessionRequestHandlerUtil sessionRequestHandlerUtil;
   private final SessionResultHandlerUtil sessionResultHandlerUtil;
   private final LocalFileUtil localFileUtil;
+  private final XtsTradefedRuntimeInfoFileUtil xtsTradefedRuntimeInfoFileUtil;
   private final CommandExecutor commandExecutor;
   private final Clock clock;
   private final XtsTypeLoader xtsTypeLoader;
@@ -136,13 +143,15 @@ final class NewMultiCommandRequestHandler {
       SessionRequestHandlerUtil sessionRequestHandlerUtil,
       SessionResultHandlerUtil sessionResultHandlerUtil,
       LocalFileUtil localFileUtil,
+      XtsTradefedRuntimeInfoFileUtil xtsTradefedRuntimeInfoFileUtil,
       CommandExecutor commandExecutor,
       Clock clock,
       XtsTypeLoader xtsTypeLoader,
       XtsJobCreator xtsJobCreator) {
     this.sessionRequestHandlerUtil = sessionRequestHandlerUtil;
     this.sessionResultHandlerUtil = sessionResultHandlerUtil;
     this.localFileUtil = localFileUtil;
+    this.xtsTradefedRuntimeInfoFileUtil = xtsTradefedRuntimeInfoFileUtil;
     this.commandExecutor = commandExecutor;
     this.clock = clock;
     this.xtsTypeLoader = xtsTypeLoader;
@@ -625,24 +634,26 @@ private HandleResultProcessingResult handleResultProcessingInternal(
     for (CommandDetail commandDetail : commandDetails) {
       CommandDetail.Builder commandDetailBuilder = commandDetail.toBuilder();
       String commandId = commandDetail.getId();
+      Path outputDirPath =
+          Path.of(outputUrl.getPath()).resolve(sessionInfo.getSessionId()).resolve(commandId);
+      String resultDirectoryName =
+          TIMESTAMP_DIR_NAME_FORMATTER.format(Instant.now()) + "_" + getRandom4Digits();
+      Path resultDir = outputDirPath.resolve(resultDirectoryName);
+      Path logDir = outputDirPath.resolve("logs");
+      ImmutableList<JobInfo> jobs =
+          commandToJobsMap.get(commandId).stream()
+              .map(jobIdToJobMap::get)
+              .collect(toImmutableList());
       try {
         SessionRequestInfo sessionRequestInfo =
             getSessionRequestInfo(request, commandDetail.getOriginalCommandInfo(), sessionInfo);
-        Path outputDirPath =
-            Path.of(outputUrl.getPath()).resolve(sessionInfo.getSessionId()).resolve(commandId);
-        String resultDirectoryName =
-            TIMESTAMP_DIR_NAME_FORMATTER.format(Instant.now()) + "_" + getRandom4Digits();
-        Path resultDir = outputDirPath.resolve(resultDirectoryName);
-        Path logDir = outputDirPath.resolve("logs");
         Optional<Result> processResult =
             sessionResultHandlerUtil.processResult(
                 resultDir,
                 logDir,
                 /* latestResultLink= */ null,
                 /* latestLogLink= */ null,
-                commandToJobsMap.get(commandId).stream()
-                    .map(jobIdToJobMap::get)
-                    .collect(toImmutableList()),
+                jobs,
                 sessionRequestInfo);
         if (processResult.isPresent() && processResult.get().hasSummary()) {
           commandDetailBuilder
@@ -705,7 +716,7 @@ private HandleResultProcessingResult handleResultProcessingInternal(
               ErrorReason.RESULT_PROCESSING_ERROR,
               "No valid test cases found in the result.");
         }
-        // TODO: Collect state and error message from TF agent if exists.
+        checkTradefedInvocationError(commandDetailBuilder, jobs, logDir);
       }
       commandDetailBuilder
           .setEndTime(Timestamps.fromMillis(clock.millis()))
@@ -764,6 +775,66 @@ void cleanup(SessionInfo sessionInfo) throws InterruptedException {
     }
   }
 
+  /**
+   * Reads the Tradefed runtime info file and checks for the Tradefed invocation error message, and
+   * set it in the command detail proto if present.
+   */
+  private void checkTradefedInvocationError(
+      CommandDetail.Builder commandDetailBuilder, ImmutableList<JobInfo> jobs, Path logDir) {
+    Optional<TestInfo> tradefedTestInfoOptional =
+        jobs.stream()
+            .filter(jobInfo -> jobInfo.properties().getBoolean(Job.IS_XTS_TF_JOB).orElse(false))
+            .findFirst()
+            .flatMap(jobInfo -> jobInfo.tests().getAll().values().stream().findFirst());
+    if (tradefedTestInfoOptional.isEmpty()) {
+      return;
+    }
+
+    TestInfo tradefedTestInfo = tradefedTestInfoOptional.get();
+    Path testLogDir;
+    try {
+      testLogDir = sessionResultHandlerUtil.getTradefedInvocationLogDir(tradefedTestInfo, logDir);
+    } catch (MobileHarnessException e) {
+      logger.atWarning().withCause(e).log(
+          "Failed to get Tradefed invocation log dir for test %s",
+          tradefedTestInfo.locator().getId());
+      return;
+    }
+
+    Path runtimeInfoFilePath = testLogDir.resolve(XtsConstants.TRADEFED_RUNTIME_INFO_FILE_NAME);
+    if (!localFileUtil.isFileExist(runtimeInfoFilePath)) {
+      return;
+    }
+
+    Optional<XtsTradefedRuntimeInfoFileDetail> fileDetailOptional;
+    try {
+      fileDetailOptional =
+          xtsTradefedRuntimeInfoFileUtil.readInfo(
+              runtimeInfoFilePath, /* lastModifiedTime= */ null);
+    } catch (IOException | RuntimeException | Error e) {
+      logger.atWarning().log(
+          "Failed to read Tradefed runtime info of test %s from file %s",
+          tradefedTestInfo.locator().getId(), runtimeInfoFilePath);
+      return;
+    }
+
+    if (fileDetailOptional.isEmpty()) {
+      return;
+    }
+
+    String errorMessage =
+        getLast(fileDetailOptional.get().runtimeInfo().invocations()).errorMessage();
+
+    if (!isNullOrEmpty(errorMessage)) {
+      setCommandError(
+          commandDetailBuilder,
+          ErrorReason.TRADEFED_INVOCATION_ERROR,
+          commandDetailBuilder.getErrorMessage().isEmpty()
+              ? errorMessage
+              : commandDetailBuilder.getErrorMessage() + "\n" + errorMessage);
+    }
+  }
+
   private URL getTestResourceUrl(TestResource testResource) throws MobileHarnessException {
     try {
       return URI.create(testResource.getUrl()).toURL();

diff --git a/...java/com/google/devtools/mobileharness/platform/android/xts/common/util/XtsConstants.java b/...java/com/google/devtools/mobileharness/platform/android/xts/common/util/XtsConstants.java
@@ -43,6 +43,8 @@ public class XtsConstants {
    */
   public static final String TRADEFED_RUNTIME_INFO_FILE_PATH = "tf_runtime_info_file_path";
 
+  public static final String TRADEFED_RUNTIME_INFO_FILE_NAME = "tf_runtime_info";
+
   public static final String INVOCATION_SUMMARY_FILE_NAME = "invocation_summary.txt";
 
   /** A MH job property key to indicate whether xTS dynamic download is enabled. */

diff --git a/src/java/com/google/devtools/mobileharness/platform/android/xts/runtime/BUILD b/src/java/com/google/devtools/mobileharness/platform/android/xts/runtime/BUILD
@@ -22,6 +22,8 @@ java_library(
     srcs = ["XtsTradefedRuntimeInfoFileUtil.java"],
     visibility = [
         "//src/java/com/google/devtools/mobileharness/infra/ats/console/controller/sessionplugin:__pkg__",
+        "//src/java/com/google/devtools/mobileharness/infra/ats/server/sessionplugin:__pkg__",
+        "//src/javatests/com/google/devtools/mobileharness/infra/ats/server/sessionplugin:__pkg__",
         "//src/javatests/com/google/devtools/mobileharness/platform/android/xts/agent:__pkg__",
     ],
     deps = [
@@ -35,6 +37,7 @@ java_library(
     srcs = ["XtsTradefedRuntimeInfo.java"],
     visibility = [
         "//src/java/com/google/devtools/mobileharness/infra/ats/console/controller/sessionplugin:__pkg__",
+        "//src/javatests/com/google/devtools/mobileharness/infra/ats/server/sessionplugin:__pkg__",
         "//src/javatests/com/google/devtools/mobileharness/platform/android/xts/agent:__pkg__",
         "//src/javatests/com/google/devtools/mobileharness/platform/android/xts/runtime:__pkg__",
     ],

diff --git a/src/java/com/google/wireless/qa/mobileharness/shared/api/driver/XtsTradefedTest.java b/src/java/com/google/wireless/qa/mobileharness/shared/api/driver/XtsTradefedTest.java
@@ -354,7 +354,8 @@ private Optional<Integer> runXtsCommand(
         getEnvironmentToTradefedConsole(tmpXtsRootDir, xtsType, spec);
 
     // Create runtime info file path.
-    Path runtimeInfoFilePath = Path.of(testInfo.getGenFileDir()).resolve("tf_runtime_info");
+    Path runtimeInfoFilePath =
+        Path.of(testInfo.getGenFileDir()).resolve(XtsConstants.TRADEFED_RUNTIME_INFO_FILE_NAME);
     testInfo
         .properties()
         .add(XtsConstants.TRADEFED_RUNTIME_INFO_FILE_PATH, runtimeInfoFilePath.toString());

diff --git a/src/javatests/com/google/devtools/mobileharness/infra/ats/server/sessionplugin/BUILD b/src/javatests/com/google/devtools/mobileharness/infra/ats/server/sessionplugin/BUILD
@@ -47,6 +47,7 @@ java_library(
         "//src/java/com/google/devtools/mobileharness/infra/ats/common:session_request_handler_util",
         "//src/java/com/google/devtools/mobileharness/infra/ats/common:session_request_info",
         "//src/java/com/google/devtools/mobileharness/infra/ats/common:session_result_handler_util",
+        "//src/java/com/google/devtools/mobileharness/infra/ats/common:xts_property_name",
         "//src/java/com/google/devtools/mobileharness/infra/ats/common:xts_type_loader",
         "//src/java/com/google/devtools/mobileharness/infra/ats/common/jobcreator:xts_job_creator",
         "//src/java/com/google/devtools/mobileharness/infra/ats/server/sessionplugin:ats_server_session_plugin_lib",
@@ -60,6 +61,8 @@ java_library(
         "//src/java/com/google/devtools/mobileharness/infra/controller/scheduler/model/job/in:device_requirements",
         "//src/java/com/google/devtools/mobileharness/infra/lab/common/dir",
         "//src/java/com/google/devtools/mobileharness/platform/android/xts/common/util:xts_constants",
+        "//src/java/com/google/devtools/mobileharness/platform/android/xts/runtime:xts_tradefed_runtime_info",
+        "//src/java/com/google/devtools/mobileharness/platform/android/xts/runtime:xts_tradefed_runtime_info_file_util",
         "//src/java/com/google/devtools/mobileharness/shared/util/command",
         "//src/java/com/google/devtools/mobileharness/shared/util/file/local",
         "//src/java/com/google/devtools/mobileharness/shared/util/flags",