Deprecate "xctest" attribute from ios_test. This removes any functionality related to using xctest = 0, but keeps the attribute to not break the users who have xctest = 1.

PiperOrigin-RevId: 179855856

4 files changed, 32 insertions, 118 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/IosTest.java b/src/main/java/com/google/devtools/build/lib/rules/objc/IosTest.java
index 436a2aa3d0..bb888f0f90 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/IosTest.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/IosTest.java
@@ -42,7 +42,6 @@ import com.google.devtools.build.lib.rules.objc.CompilationSupport.ExtraLinkArgs
 import com.google.devtools.build.lib.rules.objc.ObjcCommon.ResourceAttributes;
 import com.google.devtools.build.lib.rules.objc.ReleaseBundlingSupport.LinkedBinary;
 import com.google.devtools.build.lib.rules.proto.ProtoSourcesProvider;
-import com.google.devtools.build.lib.syntax.Type;
 
 /** Implementation for {@code ios_test} rule in Bazel. */
 public final class IosTest implements RuleConfiguredTargetFactory {
@@ -118,36 +117,30 @@ public final class IosTest implements RuleConfiguredTargetFactory {
     ExtraLinkArgs extraLinkArgs;
     Iterable<Artifact> extraLinkInputs;
     String bundleFormat;
-    if (!isXcTest(ruleContext)) {
-      extraLinkArgs = new ExtraLinkArgs();
-      extraLinkInputs = ImmutableList.of();
-      bundleFormat = ReleaseBundlingSupport.APP_BUNDLE_DIR_FORMAT;
-    } else {
-      XcTestAppProvider testApp = xcTestAppProvider(ruleContext);
-      Artifact bundleLoader = testApp.getBundleLoader();
-
-      // -bundle causes this binary to be linked as a bundle and not require an entry point
-      // (i.e. main())
-      // -bundle_loader causes the code in this test to have access to the symbols in the test rig,
-      // or more specifically, the flag causes ld to consider the given binary when checking for
-      // missing symbols.
-      // -rpath @loader_path/Frameworks allows test bundles to load dylibs from the app's
-      // Frameworks directory.
-      extraLinkArgs =
-          new ExtraLinkArgs(
-              "-bundle",
-              "-bundle_loader",
-              bundleLoader.getExecPathString(),
-              "-Xlinker",
-              "-rpath",
-              "-Xlinker",
-              "@loader_path/Frameworks");
-
-      extraLinkInputs = ImmutableList.of(bundleLoader);
-      bundleFormat = ReleaseBundlingSupport.XCTEST_BUNDLE_DIR_FORMAT;
-
-      filesToBuild.add(testApp.getIpa());
-    }
+    XcTestAppProvider testApp = xcTestAppProvider(ruleContext);
+    Artifact bundleLoader = testApp.getBundleLoader();
+
+    // -bundle causes this binary to be linked as a bundle and not require an entry point
+    // (i.e. main())
+    // -bundle_loader causes the code in this test to have access to the symbols in the test rig,
+    // or more specifically, the flag causes ld to consider the given binary when checking for
+    // missing symbols.
+    // -rpath @loader_path/Frameworks allows test bundles to load dylibs from the app's
+    // Frameworks directory.
+    extraLinkArgs =
+        new ExtraLinkArgs(
+            "-bundle",
+            "-bundle_loader",
+            bundleLoader.getExecPathString(),
+            "-Xlinker",
+            "-rpath",
+            "-Xlinker",
+            "@loader_path/Frameworks");
+
+    extraLinkInputs = ImmutableList.of(bundleLoader);
+    bundleFormat = ReleaseBundlingSupport.XCTEST_BUNDLE_DIR_FORMAT;
+
+    filesToBuild.add(testApp.getIpa());
 
     J2ObjcMappingFileProvider j2ObjcMappingFileProvider =
         J2ObjcMappingFileProvider.union(
@@ -271,11 +264,9 @@ public final class IosTest implements RuleConfiguredTargetFactory {
             .setIntermediateArtifacts(ObjcRuleClasses.intermediateArtifacts(ruleContext))
             .setHasModuleMap();
 
-    if (isXcTest(ruleContext)) {
-      builder
-          .addExtraSdkFrameworks(AUTOMATIC_SDK_FRAMEWORKS_FOR_XCTEST)
-          .addDepObjcProviders(ImmutableList.of(xcTestAppProvider(ruleContext).getObjcProvider()));
-    }
+    builder
+        .addExtraSdkFrameworks(AUTOMATIC_SDK_FRAMEWORKS_FOR_XCTEST)
+        .addDepObjcProviders(ImmutableList.of(xcTestAppProvider(ruleContext).getObjcProvider()));
 
     // Add the memleaks library if the --ios_memleaks flag is true.  The library pauses the test
     // after all tests have been executed so that leaks can be run.
@@ -289,10 +280,6 @@ public final class IosTest implements RuleConfiguredTargetFactory {
     return builder.build();
   }
 
-  protected static boolean isXcTest(RuleContext ruleContext) {
-    return ruleContext.attributes().get(IS_XCTEST_ATTR, Type.BOOLEAN);
-  }
-
   /** Returns the {@link XcTestAppProvider} of the {@code xctest_app} attribute. */
   protected static XcTestAppProvider xcTestAppProvider(RuleContext ruleContext) {
     return ruleContext.getPrerequisite(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/IosTestRule.java b/src/main/java/com/google/devtools/build/lib/rules/objc/IosTestRule.java
index b43f3020b3..892da8535c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/IosTestRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/IosTestRule.java
@@ -26,15 +26,12 @@ import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.HostTransition;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.packages.Attribute.ComputedDefault;
 import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault;
-import com.google.devtools.build.lib.packages.AttributeMap;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
 import com.google.devtools.build.lib.rules.apple.AppleConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CppConfiguration;
 import com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.BundlingRule;
-import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.FileType;
 
 /**
@@ -67,7 +64,7 @@ public class IosTestRule implements RuleDefinition {
                 .allowedRuleClasses("ios_device")
                 .value(env.getToolsLabel("//tools/objc/sim_devices:default")))
         /* <!-- #BLAZE_RULE(ios_test).ATTRIBUTE(xctest) -->
-        Whether this target contains tests using the XCTest testing framework.
+        Deprecated. Does not affect how the test is built.
         <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/
         .add(attr(IosTest.IS_XCTEST_ATTR, BOOLEAN).value(true))
         /* <!-- #BLAZE_RULE(ios_test).ATTRIBUTE(xctest_app) -->
@@ -76,34 +73,12 @@ public class IosTestRule implements RuleDefinition {
         <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/
         .add(
             attr(IosTest.XCTEST_APP_ATTR, LABEL)
-                .value(
-                    new ComputedDefault(IosTest.IS_XCTEST_ATTR) {
-                      @Override
-                      public Object getDefault(AttributeMap rule) {
-                        return rule.get(IosTest.IS_XCTEST_ATTR, Type.BOOLEAN)
-                            // No TOOLS_REPOSITORY prefix for the xctest_app tool; xcode projects
-                            // referencing a dependency under a repository do not work. Thus,
-                            // this target must be available in the target depot.
-                            ? env.getLabel("//tools/objc:xctest_app")
-                            : null;
-                      }
-                    })
+                .value(env.getLabel("//tools/objc:xctest_app"))
                 .allowedFileTypes()
                 .mandatoryProviders(XcTestAppProvider.SKYLARK_CONSTRUCTOR.id()))
         .override(
             attr(BundlingRule.INFOPLIST_ATTR, LABEL)
-                .value(
-                    new ComputedDefault(IosTest.IS_XCTEST_ATTR) {
-                      @Override
-                      public Object getDefault(AttributeMap rule) {
-                        return rule.get(IosTest.IS_XCTEST_ATTR, Type.BOOLEAN)
-                            // No TOOLS_REPOSITORY prefix for the xctest_app tool; xcode projects
-                            // referencing a dependency under a repository do not work. Thus,
-                            // this target must be available in the target depot.
-                            ? env.getLabel("//tools/objc:xctest_infoplist")
-                            : null;
-                      }
-                    })
+                .value(env.getLabel("//tools/objc:xctest_infoplist"))
                 .allowedFileTypes(ObjcRuleClasses.PLIST_TYPE))
         /* <!-- #BLAZE_RULE(ios_test).ATTRIBUTE(ios_test_target_device) -->
         The device against how to run the test. If this attribute is defined, the test will run on
@@ -175,7 +150,7 @@ public class IosTestRule implements RuleDefinition {
 (<a href="https://github.com/bazelbuild/rules_apple">https://github.com/bazelbuild/rules_apple</a>)
 to build Apple targets.</p>
 
-<p>This rule provides a way to build iOS unit tests written in KIF, GTM and XCTest test frameworks
+<p>This rule provides a way to build iOS unit tests written in the XCTest test framework
 on both iOS simulator and real devices.
 </p>
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/TestSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/TestSupport.java
index 52e6a63f5a..6969bee9f6 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/TestSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/TestSupport.java
@@ -109,11 +109,7 @@ public class TestSupport {
           .add(Substitution.of("%(test_host_path)s", ""));
     }
 
-    if (ruleContext.attributes().get(IosTest.IS_XCTEST_ATTR, Type.BOOLEAN)) {
-      substitutions.add(Substitution.of("%(test_type)s", "XCTEST"));
-    } else {
-      substitutions.add(Substitution.of("%(test_type)s", "KIF"));
-    }
+    substitutions.add(Substitution.of("%(test_type)s", "XCTEST"));
 
     Artifact template;
     if (!runWithLabDevice()) {
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/IosTestTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/IosTestTest.java
index a0927fad67..b79613b77b 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/IosTestTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/IosTestTest.java
@@ -593,7 +593,6 @@ public class IosTestTest extends ObjcRuleTestCase {
         "ios_test(",
         "  name = 'some_test',",
         "  srcs = ['SomeTest.m'],",
-        "  xctest = 0,",
         "  target_device = ':test_device',",
         ")");
 
@@ -623,7 +622,6 @@ public class IosTestTest extends ObjcRuleTestCase {
         "ios_test(",
         "  name = 'some_test',",
         "  srcs = ['SomeTest.m'],",
-        "  xctest = 0,",
         "  target_device = ':test_device',",
         ")");
 
@@ -671,48 +669,6 @@ public class IosTestTest extends ObjcRuleTestCase {
   }
 
   @Test
-  public void testNonXcTestSubstitution() throws Exception {
-    scratch.file("test/BUILD",
-        "ios_test(",
-        "  name = 'some_test',",
-        "  srcs = ['SomeTest.m'],",
-        "  xctest = 0,",
-        ")");
-
-    scratch.file("test/SomeTest.m");
-
-    ConfiguredTarget target = getConfiguredTarget("//test:some_test");
-
-    TemplateExpansionAction action =
-        getTestScriptGenerationAction(target);
-    assertThat(action.getSubstitutions()).containsExactly(
-        Substitution.of("%(memleaks)s", "false"),
-
-        Substitution.of("%(test_app_ipa)s", "test/some_test.ipa"),
-        Substitution.of("%(test_bundle_path)s", "test/some_test.ipa"),
-        Substitution.of("%(test_app_name)s", "some_test"),
-
-        Substitution.of("%(xctest_app_ipa)s", ""),
-        Substitution.of("%(xctest_app_name)s", ""),
-        Substitution.of("%(test_host_path)s", ""),
-
-        Substitution.of("%(plugin_jars)s", ""),
-        Substitution.of("%(device_type)s", "iChimpanzee"),
-        Substitution.of("%(locale)s", "en"),
-        Substitution.of("%(simulator_sdk)s", "9.8"),
-        Substitution.of("%(testrunner_binary)s", "tools/objc/testrunner"),
-        Substitution.of("%(std_redirect_dylib_path)s", "tools/objc/StdRedirect.dylib"),
-        Substitution.of("%(test_env)s", ""),
-        Substitution.of("%(test_type)s", "KIF")
-    );
-
-    assertRunfilesContainsRootRelativePaths(target,
-        "test/some_test.ipa",
-        "test/some_test_test_script",
-        "tools/objc/testrunner");
-  }
-
-  @Test
   public void testRunnerWithDevice() throws Exception {
     scratch.file("test/BUILD",
         "ios_test(",