Move test_timeout to BuildConfiguration and TargetCompleteEvent.

- Move test_timeout to BuildConfiguration.
- Add test_timeout_seconds field to BEP TargetCompleteEvent.
- Deprecate test_timeout field in ExecutionInfo. Data is still written to deprecated field to allow consumer transition.

PiperOrigin-RevId: 193192636

6 files changed, 49 insertions, 22 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java b/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java
index 97dde9c546..49e470c9ad 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java
@@ -47,6 +47,7 @@ import com.google.devtools.build.lib.packages.AttributeMap;
 import com.google.devtools.build.lib.packages.ConfiguredAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.TestSize;
+import com.google.devtools.build.lib.packages.TestTimeout;
 import com.google.devtools.build.lib.rules.AliasConfiguredTarget;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
 import com.google.devtools.build.lib.syntax.Type;
@@ -209,6 +210,7 @@ public final class TargetCompleteEvent
     builder.addAllOutputGroup(getOutputFilesByGroup(converters.artifactGroupNamer()));
 
     if (isTest) {
+      builder.setTestTimeoutSeconds(getTestTimeoutSeconds(targetAndData));
       builder.setTestSize(
           TargetConfiguredEvent.bepTestSize(
               TestSize.getTestSize(targetAndData.getTarget().getAssociatedRule())));
@@ -292,4 +294,17 @@ public final class TargetCompleteEvent
     }
     return groups.build();
   }
+
+  /**
+   * Returns timeout value in seconds that should be used for all test actions under this configured
+   * target. We always use the "categorical timeouts" which are based on the --test_timeout flag. A
+   * rule picks its timeout but ends up with the same effective value as all other rules in that
+   * category and configuration.
+   */
+  private Long getTestTimeoutSeconds(ConfiguredTargetAndData targetAndData) {
+    BuildConfiguration configuration = targetAndData.getConfiguration();
+    Rule associatedRule = targetAndData.getTarget().getAssociatedRule();
+    TestTimeout categoricalTimeout = TestTimeout.getTestTimeout(associatedRule);
+    return configuration.getTestTimeout().get(categoricalTimeout).getSeconds();
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index c137781357..80f35abd4f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -49,6 +49,7 @@ import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.RuleClassProvider;
 import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.packages.TestTimeout;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
@@ -66,6 +67,7 @@ import com.google.devtools.common.options.OptionEffectTag;
 import com.google.devtools.common.options.OptionMetadataTag;
 import com.google.devtools.common.options.OptionsParsingException;
 import com.google.devtools.common.options.TriState;
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
@@ -564,6 +566,20 @@ public class BuildConfiguration {
     )
     public List<Map.Entry<String, String>> testEnvironment;
 
+    @Option(
+        name = "test_timeout",
+        defaultValue = "-1",
+        converter = TestTimeout.TestTimeoutConverter.class,
+        documentationCategory = OptionDocumentationCategory.TESTING,
+        effectTags = {OptionEffectTag.UNKNOWN},
+        help =
+            "Override the default test timeout values for test timeouts (in secs). If a single "
+                + "positive integer value is specified it will override all categories.  If 4 "
+                + "comma-separated integers are specified, they will override the timeouts for "
+                + "short, moderate, long and eternal (in that order). In either form, a value of "
+                + "-1 tells blaze to use its default timeouts for that category.")
+    public Map<TestTimeout, Duration> testTimeout;
+
     // TODO(bazel-team): The set of available variables from the client environment for actions
     // is computed independently in CommandEnvironment to inject a more restricted client
     // environment to skyframe.
@@ -1094,6 +1110,7 @@ public class BuildConfiguration {
 
   private final ActionEnvironment actionEnv;
   private final ActionEnvironment testEnv;
+  private final ImmutableMap<TestTimeout, Duration> testTimeout;
 
   private final BuildOptions buildOptions;
   private final BuildOptions.OptionsDiffForReconstruction buildOptionsDiff;
@@ -1274,6 +1291,8 @@ public class BuildConfiguration {
 
     this.testEnv = setupTestEnvironment();
 
+    this.testTimeout = ImmutableMap.copyOf(options.testTimeout);
+
     this.transitiveOptionDetails = computeOptionsMap(buildOptions, fragments.values());
 
     ImmutableMap.Builder<String, String> globalMakeEnvBuilder = ImmutableMap.builder();
@@ -1717,6 +1736,11 @@ public class BuildConfiguration {
     return testEnv.getFixedEnv();
   }
 
+  /** Returns test timeout mapping as set by --test_timeout options. */
+  public ImmutableMap<TestTimeout, Duration> getTestTimeout() {
+    return testTimeout;
+  }
+
   /**
    * Returns user-specified test environment variables and their values, as set by the
    * {@code --test_env} options. It is incomplete in that it is not a superset of the
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto
index 33844eef69..85439806c7 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto
@@ -475,6 +475,9 @@ message TargetComplete {
 
   // List of tags associated with this configured target.
   repeated string tag = 3;
+
+  // The timeout specified for test actions under this configured target.
+  int64 test_timeout_seconds = 7;
 }
 
 enum TestStatus {
@@ -518,7 +521,8 @@ message TestResult {
   // Message providing optional meta data on the execution of the test action,
   // if available.
   message ExecutionInfo {
-    int32 timeout_seconds = 1;
+    // Deprecated, use TargetComplete.test_timeout_seconds instead.
+    int32 timeout_seconds = 1 [deprecated = true];
 
     // Name of the strategy to execute this test action (e.g., "local",
     // "remote")
diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java
index 3c959b68bc..8c7e2e1248 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java
@@ -16,7 +16,6 @@ package com.google.devtools.build.lib.exec;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.ResourceSet;
 import com.google.devtools.build.lib.analysis.config.PerLabelOptions;
-import com.google.devtools.build.lib.packages.TestTimeout;
 import com.google.devtools.build.lib.util.OptionsUtils;
 import com.google.devtools.build.lib.util.RegexFilter;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -26,10 +25,8 @@ import com.google.devtools.common.options.OptionEffectTag;
 import com.google.devtools.common.options.Options;
 import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsParsingException;
-import java.time.Duration;
 import java.util.Collections;
 import java.util.List;
-import java.util.Map;
 
 /**
  * Options affecting the execution phase of a build.
@@ -185,21 +182,6 @@ public class ExecutionOptions extends OptionsBase {
   public TestStrategy.TestSummaryFormat testSummary;
 
   @Option(
-    name = "test_timeout",
-    defaultValue = "-1",
-    converter = TestTimeout.TestTimeoutConverter.class,
-    documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-    effectTags = {OptionEffectTag.UNKNOWN},
-    help =
-        "Override the default test timeout values for test timeouts (in secs). If a single "
-            + "positive integer value is specified it will override all categories.  If 4 "
-            + "comma-separated integers are specified, they will override the timeouts for short, "
-            + "moderate, long and eternal (in that order). In either form, a value of -1 tells "
-            + "blaze to use its default timeouts for that category."
-  )
-  public Map<TestTimeout, Duration> testTimeout;
-
-  @Option(
     name = "resource_autosense",
     defaultValue = "false",
     documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
diff --git a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
index 07a1942a68..87ebde4199 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
@@ -29,6 +29,7 @@ import com.google.devtools.build.lib.actions.SpawnResult;
 import com.google.devtools.build.lib.actions.TestExecException;
 import com.google.devtools.build.lib.actions.UserExecException;
 import com.google.devtools.build.lib.analysis.ShellConfiguration;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.PerLabelOptions;
 import com.google.devtools.build.lib.analysis.test.TestActionContext;
 import com.google.devtools.build.lib.analysis.test.TestResult;
@@ -238,7 +239,8 @@ public abstract class TestStrategy implements TestActionContext {
    * but ends up with the same effective value as all other rules in that bucket.
    */
   protected final Duration getTimeout(TestRunnerAction testAction) {
-    return executionOptions.testTimeout.get(testAction.getTestProperties().getTimeout());
+    BuildConfiguration configuration = testAction.getConfiguration();
+    return configuration.getTestTimeout().get(testAction.getTestProperties().getTimeout());
   }
 
   /*
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
index 10b437107f..4229374a1d 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
@@ -465,8 +465,8 @@ public class RunCommand implements BlazeCommand  {
       Path tmpDirRoot = TestStrategy.getTmpRoot(
           env.getWorkspace(), env.getExecRoot(), executionOptions);
       PathFragment relativeTmpDir = tmpDirRoot.relativeTo(env.getExecRoot());
-      Duration timeout = executionOptions.testTimeout.get(
-          testAction.getTestProperties().getTimeout());
+      Duration timeout =
+          configuration.getTestTimeout().get(testAction.getTestProperties().getTimeout());
       runEnvironment.putAll(StandaloneTestStrategy.DEFAULT_LOCAL_POLICY.computeTestEnvironment(
           testAction,
           env.getClientEnv(),