Delete --test_sharding_strategy=experimental_heuristic. Test sharding…

… is a well-established feature, and I expect users are able to experiment by simply modifying the "shard_count" directly. RELNOTES: --test_sharding_strategy=experimental_heuristic is no more PiperOrigin-RevId: 229579455
weixiao-huang · Jan 31, 2019 · 3b2cef0 · 3b2cef0
1 parent 7e5532e
commit 3b2cef0
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 43 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java
@@ -169,19 +169,6 @@ public static enum TestShardingStrategy {
       }
     },
 
-    EXPERIMENTAL_HEURISTIC {
-      @Override public int getNumberOfShards(boolean isLocal, int shardCountFromAttr,
-          boolean testShardingCompliant, TestSize testSize) {
-        if (shardCountFromAttr >= 0) {
-          return shardCountFromAttr;
-        }
-        if (isLocal || !testShardingCompliant) {
-          return 0;
-        }
-        return testSize.getDefaultShards();
-      }
-    },
-
     DISABLED {
       @Override public int getNumberOfShards(boolean isLocal, int shardCountFromAttr,
           boolean testShardingCompliant, TestSize testSize) {

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java
@@ -27,8 +27,6 @@
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.analysis.config.PerLabelOptions;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.events.Event;
-import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.TestTimeout;
 import com.google.devtools.build.lib.util.RegexFilter;
 import com.google.devtools.common.options.Option;
@@ -146,19 +144,15 @@ public static class TestOptions extends FragmentOptions {
     public List<String> testArguments;
 
     @Option(
-      name = "test_sharding_strategy",
-      defaultValue = "explicit",
-      converter = TestActionBuilder.ShardingStrategyConverter.class,
-      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-      effectTags = {OptionEffectTag.UNKNOWN},
-      help =
-          "Specify strategy for test sharding: "
-              + "'explicit' to only use sharding if the 'shard_count' BUILD attribute is present. "
-              + "'disabled' to never use test sharding. "
-              + "'experimental_heuristic' to enable sharding on remotely executed tests without an "
-              + "explicit  'shard_count' attribute which link in a supported framework. Considered "
-              + "experimental."
-    )
+        name = "test_sharding_strategy",
+        defaultValue = "explicit",
+        converter = TestActionBuilder.ShardingStrategyConverter.class,
+        documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+        effectTags = {OptionEffectTag.UNKNOWN},
+        help =
+            "Specify strategy for test sharding: "
+                + "'explicit' to only use sharding if the 'shard_count' BUILD attribute is "
+                + "present. 'disabled' to never use test sharding.")
     public TestActionBuilder.TestShardingStrategy testShardingStrategy;
 
     @Option(
@@ -281,18 +275,6 @@ private TestConfiguration(TestOptions options) {
     this.testTimeout = ImmutableMap.copyOf(options.testTimeout);
   }
 
-  @Override
-  public void reportInvalidOptions(EventHandler reporter, BuildOptions buildOptions) {
-    if (options.testShardingStrategy
-        == TestActionBuilder.TestShardingStrategy.EXPERIMENTAL_HEURISTIC) {
-      reporter.handle(
-          Event.warn(
-              "Heuristic sharding is intended as a one-off experimentation tool for determining "
-                  + "the benefit from sharding certain tests. Please don't keep this option in "
-                  + "your .blazerc or continuous build"));
-    }
-  }
-
   /** Returns test timeout mapping as set by --test_timeout options. */
   public ImmutableMap<TestTimeout, Duration> getTestTimeout() {
     return testTimeout;

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java
@@ -450,12 +450,12 @@ public void testExtraActionsCaching() throws Exception {
 
   @Test
   public void testConfigurationCachingWithWarningReplay() throws Exception {
-    useConfiguration("--test_sharding_strategy=experimental_heuristic");
+    useConfiguration("--strip=always", "--copt=-g");
     update();
-    assertContainsEvent("Heuristic sharding is intended as a one-off experimentation tool");
+    assertContainsEvent("Debug information will be generated and then stripped away");
     eventCollector.clear();
     update();
-    assertContainsEvent("Heuristic sharding is intended as a one-off experimentation tool");
+    assertContainsEvent("Debug information will be generated and then stripped away");
   }
 
   @Test