diff --git a/src/java/com/google/devtools/mobileharness/infra/ats/common/SessionRequestHandlerUtil.java b/src/java/com/google/devtools/mobileharness/infra/ats/common/SessionRequestHandlerUtil.java index a954cf2ccd..dc39331a01 100644 --- a/src/java/com/google/devtools/mobileharness/infra/ats/common/SessionRequestHandlerUtil.java +++ b/src/java/com/google/devtools/mobileharness/infra/ats/common/SessionRequestHandlerUtil.java @@ -145,8 +145,8 @@ public class SessionRequestHandlerUtil { Pattern.compile(".*\\[(?.*)]$"); private static final Duration JOB_TEST_TIMEOUT_DIFF = Duration.ofMinutes(1L); - private static final Duration DEFAULT_TRADEFED_JOB_TIMEOUT = Duration.ofDays(15L); - private static final Duration DEFAULT_TRADEFED_START_TIMEOUT = Duration.ofDays(14L); + public static final Duration DEFAULT_TRADEFED_JOB_TIMEOUT = Duration.ofDays(15L); + public static final Duration DEFAULT_TRADEFED_START_TIMEOUT = Duration.ofDays(14L); private static final Duration DEFAULT_NON_TRADEFED_JOB_TIMEOUT = Duration.ofDays(5L); private static final Duration DEFAULT_NON_TRADEFED_START_TIMEOUT = Duration.ofDays(4L); diff --git a/src/java/com/google/devtools/mobileharness/infra/ats/common/jobcreator/BUILD b/src/java/com/google/devtools/mobileharness/infra/ats/common/jobcreator/BUILD index afbec271f6..f631dc3c09 100644 --- a/src/java/com/google/devtools/mobileharness/infra/ats/common/jobcreator/BUILD +++ b/src/java/com/google/devtools/mobileharness/infra/ats/common/jobcreator/BUILD @@ -51,6 +51,8 @@ java_library( "//src/java/com/google/devtools/mobileharness/shared/util/flags", "//src/java/com/google/devtools/mobileharness/shared/util/jobconfig:job_info_creator", "//src/java/com/google/devtools/mobileharness/shared/util/logging:google_logger", + "//src/java/com/google/devtools/mobileharness/shared/util/system", + "//src/java/com/google/devtools/mobileharness/shared/util/time:time_utils", "//src/java/com/google/wireless/qa/mobileharness/shared/api/decorator/constant:phase_skippable_decorator_constants", "//src/java/com/google/wireless/qa/mobileharness/shared/api/metadata", "//src/java/com/google/wireless/qa/mobileharness/shared/model/job", @@ -86,6 +88,7 @@ java_library( "//src/java/com/google/devtools/mobileharness/platform/android/xts/suite/retry:retry_generator", "//src/java/com/google/devtools/mobileharness/platform/android/xts/suite/subplan:sub_plan", "//src/java/com/google/devtools/mobileharness/shared/util/file/local", + "//src/java/com/google/devtools/mobileharness/shared/util/system", "//src/java/com/google/wireless/qa/mobileharness/shared/api/spec:tradefed_test_spec", "@maven//:com_google_code_findbugs_jsr305", "@maven//:com_google_guava_guava", @@ -123,6 +126,7 @@ java_library( "//src/java/com/google/devtools/mobileharness/shared/util/flags", "//src/java/com/google/devtools/mobileharness/shared/util/logging:google_logger", "//src/java/com/google/devtools/mobileharness/shared/util/path", + "//src/java/com/google/devtools/mobileharness/shared/util/system", "//src/java/com/google/wireless/qa/mobileharness/shared/api/spec:tradefed_test_spec", "@maven//:com_google_code_findbugs_jsr305", "@maven//:com_google_guava_guava", diff --git a/src/java/com/google/devtools/mobileharness/infra/ats/common/jobcreator/ConsoleJobCreator.java b/src/java/com/google/devtools/mobileharness/infra/ats/common/jobcreator/ConsoleJobCreator.java index d98b893b91..3381d7b889 100644 --- a/src/java/com/google/devtools/mobileharness/infra/ats/common/jobcreator/ConsoleJobCreator.java +++ b/src/java/com/google/devtools/mobileharness/infra/ats/common/jobcreator/ConsoleJobCreator.java @@ -39,6 +39,7 @@ import com.google.devtools.mobileharness.platform.android.xts.suite.retry.RetryGenerator; import com.google.devtools.mobileharness.platform.android.xts.suite.subplan.SubPlan; import com.google.devtools.mobileharness.shared.util.file.local.LocalFileUtil; +import com.google.devtools.mobileharness.shared.util.system.SystemUtil; import com.google.wireless.qa.mobileharness.shared.api.spec.TradefedTestSpec; import java.nio.file.Path; import java.util.Map; @@ -58,8 +59,14 @@ public class ConsoleJobCreator extends XtsJobCreator { LocalFileUtil localFileUtil, PreviousResultLoader previousResultLoader, RetryGenerator retryGenerator, - ModuleShardingArgsGenerator moduleShardingArgsGenerator) { - super(sessionRequestHandlerUtil, localFileUtil, retryGenerator, moduleShardingArgsGenerator); + ModuleShardingArgsGenerator moduleShardingArgsGenerator, + SystemUtil systemUtil) { + super( + sessionRequestHandlerUtil, + localFileUtil, + retryGenerator, + moduleShardingArgsGenerator, + systemUtil); this.previousResultLoader = previousResultLoader; } diff --git a/src/java/com/google/devtools/mobileharness/infra/ats/common/jobcreator/ServerJobCreator.java b/src/java/com/google/devtools/mobileharness/infra/ats/common/jobcreator/ServerJobCreator.java index b36443694d..c90ec1374d 100644 --- a/src/java/com/google/devtools/mobileharness/infra/ats/common/jobcreator/ServerJobCreator.java +++ b/src/java/com/google/devtools/mobileharness/infra/ats/common/jobcreator/ServerJobCreator.java @@ -47,6 +47,7 @@ import com.google.devtools.mobileharness.shared.util.file.local.LocalFileUtil; import com.google.devtools.mobileharness.shared.util.flags.Flags; import com.google.devtools.mobileharness.shared.util.path.PathUtil; +import com.google.devtools.mobileharness.shared.util.system.SystemUtil; import com.google.wireless.qa.mobileharness.shared.api.spec.TradefedTestSpec; import java.io.FileOutputStream; import java.io.IOException; @@ -73,8 +74,14 @@ public class ServerJobCreator extends XtsJobCreator { PreviousResultLoader previousResultLoader, RetryGenerator retryGenerator, ModuleShardingArgsGenerator moduleShardingArgsGenerator, - AtsServerSessionUtil atsServerSessionUtil) { - super(sessionRequestHandlerUtil, localFileUtil, retryGenerator, moduleShardingArgsGenerator); + AtsServerSessionUtil atsServerSessionUtil, + SystemUtil systemUtil) { + super( + sessionRequestHandlerUtil, + localFileUtil, + retryGenerator, + moduleShardingArgsGenerator, + systemUtil); this.previousResultLoader = previousResultLoader; this.atsServerSessionUtil = atsServerSessionUtil; diff --git a/src/java/com/google/devtools/mobileharness/infra/ats/common/jobcreator/XtsJobCreator.java b/src/java/com/google/devtools/mobileharness/infra/ats/common/jobcreator/XtsJobCreator.java index 19f9874807..95ebac1cfc 100644 --- a/src/java/com/google/devtools/mobileharness/infra/ats/common/jobcreator/XtsJobCreator.java +++ b/src/java/com/google/devtools/mobileharness/infra/ats/common/jobcreator/XtsJobCreator.java @@ -18,6 +18,8 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static com.google.common.primitives.Ints.saturatedCast; +import static com.google.devtools.mobileharness.shared.util.time.TimeUtils.toJavaDuration; import static java.util.stream.Collectors.joining; import com.google.common.annotations.VisibleForTesting; @@ -53,6 +55,7 @@ import com.google.devtools.mobileharness.shared.util.file.local.LocalFileUtil; import com.google.devtools.mobileharness.shared.util.flags.Flags; import com.google.devtools.mobileharness.shared.util.jobconfig.JobInfoCreator; +import com.google.devtools.mobileharness.shared.util.system.SystemUtil; import com.google.gson.Gson; import com.google.gson.JsonObject; import com.google.wireless.qa.mobileharness.shared.api.decorator.constant.PhaseSkippableDecoratorConstants; @@ -68,6 +71,7 @@ import java.io.InputStream; import java.io.OutputStream; import java.nio.file.Path; +import java.time.Duration; import java.time.Instant; import java.util.HashMap; import java.util.Locale; @@ -102,16 +106,19 @@ public abstract class XtsJobCreator { protected final LocalFileUtil localFileUtil; private final RetryGenerator retryGenerator; private final ModuleShardingArgsGenerator moduleShardingArgsGenerator; + private final SystemUtil systemUtil; protected XtsJobCreator( SessionRequestHandlerUtil sessionRequestHandlerUtil, LocalFileUtil localFileUtil, RetryGenerator retryGenerator, - ModuleShardingArgsGenerator moduleShardingArgsGenerator) { + ModuleShardingArgsGenerator moduleShardingArgsGenerator, + SystemUtil systemUtil) { this.sessionRequestHandlerUtil = sessionRequestHandlerUtil; this.localFileUtil = localFileUtil; this.retryGenerator = retryGenerator; this.moduleShardingArgsGenerator = moduleShardingArgsGenerator; + this.systemUtil = systemUtil; } public static boolean isSkippableException(MobileHarnessException e) { @@ -717,13 +724,35 @@ private JobInfo createPreconditionJob( .build()) .collect(toImmutableList()); + // Dynamic MCTS downloads large artifacts, requiring standard Tradefed timeouts. + Duration jobTimeout; + Duration testTimeout; + Duration startTimeout; + if (isDynamicMctsEnabled) { + jobTimeout = + (sessionRequestInfo.getJobTimeout().getSeconds() == 0 + && sessionRequestInfo.getJobTimeout().getNanos() == 0) + ? SessionRequestHandlerUtil.DEFAULT_TRADEFED_JOB_TIMEOUT + : toJavaDuration(sessionRequestInfo.getJobTimeout()); + testTimeout = SessionRequestHandlerUtil.calculateTestTimeout(jobTimeout); + startTimeout = + (sessionRequestInfo.getStartTimeout().getSeconds() == 0 + && sessionRequestInfo.getStartTimeout().getNanos() == 0) + ? SessionRequestHandlerUtil.DEFAULT_TRADEFED_START_TIMEOUT + : toJavaDuration(sessionRequestInfo.getStartTimeout()); + } else { + jobTimeout = Duration.ofMinutes(5); + testTimeout = Duration.ofMinutes(5); + startTimeout = Duration.ofMinutes(5); + } + JobConfig jobConfig = JobConfig.newBuilder() .setName(name) .setExecMode("local") - .setJobTimeoutSec(300) - .setTestTimeoutSec(300) - .setStartTimeoutSec(300) + .setJobTimeoutSec(saturatedCast(jobTimeout.toSeconds())) + .setTestTimeoutSec(saturatedCast(testTimeout.toSeconds())) + .setStartTimeoutSec(startTimeout.toSeconds()) .setPriority(Priority.HIGH) .setTestAttempts(1) .setTests(StringList.newBuilder().addContent(name)) @@ -737,7 +766,8 @@ private JobInfo createPreconditionJob( jobConfig, /* nonstandardFlags= */ ImmutableList.of(), sessionRequestHandlerUtil.createJobGenDir(name).toString(), - sessionRequestHandlerUtil.createJobTmpDir(name).toString()); + sessionRequestHandlerUtil.createJobTmpDir(name).toString(), + systemUtil); if (decorators.stream() .anyMatch(d -> DriverDecoratorMetadata.isPhaseSkippableDecorator(d.getDecoratorName()))) { diff --git a/src/javatests/com/google/devtools/mobileharness/infra/ats/common/jobcreator/BUILD b/src/javatests/com/google/devtools/mobileharness/infra/ats/common/jobcreator/BUILD index 8cf3f7a91e..b159d0b9df 100644 --- a/src/javatests/com/google/devtools/mobileharness/infra/ats/common/jobcreator/BUILD +++ b/src/javatests/com/google/devtools/mobileharness/infra/ats/common/jobcreator/BUILD @@ -30,6 +30,7 @@ java_library( "//src/devtools/mobileharness/infra/ats/common/proto:xts_common_java_proto", "//src/devtools/mobileharness/infra/ats/server/proto:service_java_proto", "//src/java/com/google/devtools/mobileharness/api/model/error", + "//src/java/com/google/devtools/mobileharness/api/model/job/in", "//src/java/com/google/devtools/mobileharness/infra/ats/common:session_handler_helper", "//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_util", @@ -49,6 +50,8 @@ java_library( "//src/java/com/google/devtools/mobileharness/platform/android/xts/suite/subplan:sub_plan", "//src/java/com/google/devtools/mobileharness/shared/util/file/local", "//src/java/com/google/devtools/mobileharness/shared/util/flags/core:testing", + "//src/java/com/google/devtools/mobileharness/shared/util/system", + "//src/java/com/google/devtools/mobileharness/shared/util/time:time_utils", "//src/java/com/google/wireless/qa/mobileharness/shared/api/decorator/constant:phase_skippable_decorator_constants", "//src/java/com/google/wireless/qa/mobileharness/shared/api/spec:tradefed_test_spec", "//src/java/com/google/wireless/qa/mobileharness/shared/model/job", diff --git a/src/javatests/com/google/devtools/mobileharness/infra/ats/common/jobcreator/ConsoleJobCreatorTest.java b/src/javatests/com/google/devtools/mobileharness/infra/ats/common/jobcreator/ConsoleJobCreatorTest.java index d6eac38325..2309856860 100644 --- a/src/javatests/com/google/devtools/mobileharness/infra/ats/common/jobcreator/ConsoleJobCreatorTest.java +++ b/src/javatests/com/google/devtools/mobileharness/infra/ats/common/jobcreator/ConsoleJobCreatorTest.java @@ -17,6 +17,7 @@ package com.google.devtools.mobileharness.infra.ats.common.jobcreator; import static com.google.common.truth.Truth.assertThat; +import static com.google.devtools.mobileharness.shared.util.time.TimeUtils.toProtoDuration; import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.any; @@ -32,6 +33,7 @@ import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; import com.google.devtools.mobileharness.api.model.error.MobileHarnessException; +import com.google.devtools.mobileharness.api.model.job.in.Timeout; import com.google.devtools.mobileharness.infra.ats.common.SessionRequestHandlerUtil; import com.google.devtools.mobileharness.infra.ats.common.SessionRequestHandlerUtil.TradefedJobInfo; import com.google.devtools.mobileharness.infra.ats.common.SessionRequestInfoUtil; @@ -49,6 +51,7 @@ import com.google.devtools.mobileharness.platform.android.xts.suite.subplan.SubPlan; import com.google.devtools.mobileharness.shared.util.file.local.LocalFileUtil; import com.google.devtools.mobileharness.shared.util.flags.core.SetFlags; +import com.google.devtools.mobileharness.shared.util.system.SystemUtil; import com.google.inject.Guice; import com.google.inject.testing.fieldbinder.Bind; import com.google.inject.testing.fieldbinder.BoundFieldModule; @@ -65,6 +68,7 @@ import java.io.FileOutputStream; import java.nio.file.Files; import java.nio.file.Path; +import java.time.Duration; import java.util.Map; import java.util.Optional; import java.util.Properties; @@ -108,6 +112,7 @@ public final class ConsoleJobCreatorTest { @Bind @Mock private PreviousResultLoader previousResultLoader; @Bind @Mock private RetryGenerator retryGenerator; @Bind @Mock private ModuleShardingArgsGenerator moduleShardingArgsGenerator; + @Bind @Mock private SystemUtil systemUtil; @Inject private ConsoleJobCreator jobCreator; @@ -116,6 +121,7 @@ public void setUp() throws Exception { flags.setAll(ImmutableMap.of("enable_ats_mode", "true", "use_tf_retry", "false")); Guice.createInjector(BoundFieldModule.of(this)).injectMembers(this); + when(systemUtil.isBlazeTest()).thenReturn(false); when(sessionRequestHandlerUtil.getSessionSubDeviceSpecList(any(), anyBoolean())) .thenReturn(MOCK_SUB_DEVICE_SPEC_LIST); } @@ -760,6 +766,7 @@ public void createXtsNonTradefedJobs_moblyOnly_setupAndTeardownJobsInjected() th assertThat(setupJob.properties().get(PhaseSkippableDecoratorConstants.PROP_EXECUTION_MODE)) .isEqualTo(PhaseSkippableDecoratorConstants.ExecutionMode.SETUP_ONLY.name()); assertThat(setupJob.type().getDriver()).isEqualTo("NoOpDriver"); + assertThat(setupJob.setting().getNewTimeout().jobTimeout()).isEqualTo(Duration.ofMinutes(5)); assertThat(setupJob.subDeviceSpecs().getAllSubDevices().get(0).decorators().getAll()) .containsExactly( "AndroidCleanAppsDecorator", @@ -991,7 +998,21 @@ public void createXtsSetupAndTearDownJob_dynamicMctsEnabled_createsJobsWithoutDe .isEqualTo("true"); assertThat(setupJob.properties().get(XtsConstants.XTS_JOB_NAME)) .isEqualTo(XtsConstants.SETUP_JOB_NAME); - assertThat(setupJob.subDeviceSpecs().getAllSubDevices().get(0).decorators().getAll()).isEmpty(); + Duration expectedJobTimeout = + min(SessionRequestHandlerUtil.DEFAULT_TRADEFED_JOB_TIMEOUT, Timeout.MAX_JOB_TIMEOUT); + Duration expectedTestTimeout = + min( + SessionRequestHandlerUtil.calculateTestTimeout( + SessionRequestHandlerUtil.DEFAULT_TRADEFED_JOB_TIMEOUT), + Timeout.MAX_TEST_TIMEOUT); + Duration expectedStartTimeout = + min( + SessionRequestHandlerUtil.DEFAULT_TRADEFED_START_TIMEOUT, + expectedJobTimeout.minusMinutes(1)); + + assertThat(setupJob.setting().getNewTimeout().jobTimeout()).isEqualTo(expectedJobTimeout); + assertThat(setupJob.setting().getNewTimeout().testTimeout()).isEqualTo(expectedTestTimeout); + assertThat(setupJob.setting().getNewTimeout().startTimeout()).isEqualTo(expectedStartTimeout); Optional teardownJobOpt = jobCreator.createXtsTearDownJob(sessionRequestInfo); assertThat(teardownJobOpt).isPresent(); @@ -1003,6 +1024,39 @@ public void createXtsSetupAndTearDownJob_dynamicMctsEnabled_createsJobsWithoutDe .isEqualTo(XtsConstants.TEARDOWN_JOB_NAME); assertThat(teardownJob.subDeviceSpecs().getAllSubDevices().get(0).decorators().getAll()) .isEmpty(); + assertThat(teardownJob.setting().getNewTimeout().jobTimeout()).isEqualTo(expectedJobTimeout); + assertThat(teardownJob.setting().getNewTimeout().testTimeout()).isEqualTo(expectedTestTimeout); + assertThat(teardownJob.setting().getNewTimeout().startTimeout()) + .isEqualTo(expectedStartTimeout); + } + + @Test + public void createXtsSetupAndTearDownJob_dynamicMctsEnabledWithCustomTimeout_usesCustomTimeout() + throws Exception { + SessionRequestInfo sessionRequestInfo = + SessionRequestInfoUtil.buildAndValidate( + SessionRequestInfo.newBuilder() + .setTestPlan("cts") + .setCommandLineArgs("cts") + .setXtsRootDir(XTS_ROOT_DIR_PATH) + .setXtsType("cts") + .setIsXtsDynamicDownloadEnabled(true) + .setJobTimeout(toProtoDuration(Duration.ofHours(2))) + .setStartTimeout(toProtoDuration(Duration.ofHours(1)))); + + when(sessionRequestHandlerUtil.canCreateNonTradefedJobs(sessionRequestInfo)).thenReturn(false); + when(sessionRequestHandlerUtil.getFilteredTradefedModules(sessionRequestInfo)) + .thenReturn(ImmutableList.of("CtsSampleDeviceTestCases")); + when(sessionRequestHandlerUtil.createJobGenDir(any())).thenReturn(Path.of("/tmp/gen")); + when(sessionRequestHandlerUtil.createJobTmpDir(any())).thenReturn(Path.of("/tmp/tmp")); + + Optional setupJobOpt = jobCreator.createXtsSetupJob(sessionRequestInfo); + assertThat(setupJobOpt).isPresent(); + JobInfo setupJob = setupJobOpt.get(); + assertThat(setupJob.setting().getNewTimeout().jobTimeout()).isEqualTo(Duration.ofHours(2)); + assertThat(setupJob.setting().getNewTimeout().testTimeout()) + .isEqualTo(Duration.ofHours(2).minusMinutes(1)); + assertThat(setupJob.setting().getNewTimeout().startTimeout()).isEqualTo(Duration.ofHours(1)); } @Test @@ -1074,4 +1128,8 @@ public void createXtsSetupAndTearDownJob_retryNonCtsPlan_doesNotCreateJobs() thr Optional teardownJobOpt = jobCreator.createXtsTearDownJob(sessionRequestInfo); assertThat(teardownJobOpt).isEmpty(); } + + private static Duration min(Duration d1, Duration d2) { + return d1.compareTo(d2) <= 0 ? d1 : d2; + } }