Make tests that mess with the WORKSPACE file work with untrimmed dynamic configs.

They already work with static configs because those configs get created in
test setup, before WORKSPACE is modified. That means they get the necessary
bindings needed to load their fragments (e.g. local_config_xcode for
AppleConfiguration).

But dynamic configs get instantiated on-demand. Which means in this case
*after* WORKSPACE is overwritten. So they can't find the bindings and fail.

This fix has offending tests *append* WORKSPACE instead of overwrite.

This unblocks defaulting --experimental_dynamic_configs to "notrim_partial".

--
PiperOrigin-RevId: 147377195
MOS_MIGRATED_REVID=147377195

6 files changed, 141 insertions, 79 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
index 76d24e09d4..438c0d4b4e 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
@@ -23,6 +23,7 @@ import static com.google.devtools.build.lib.packages.BuildType.LABEL;
 import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
 import static org.junit.Assert.fail;
 
+import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.util.ActionsTestUtil.NullAction;
 import com.google.devtools.build.lib.analysis.util.AnalysisTestCase;
@@ -137,7 +138,10 @@ public class AspectTest extends AnalysisTestCase {
         "honest(name='b', foo=[])");
 
     scratch.overwriteFile("WORKSPACE",
-        "bind(name='b', actual='//a:b')");
+        new ImmutableList.Builder<String>()
+            .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+            .add("bind(name='b', actual='//a:b')")
+            .build());
 
     skyframeExecutor.invalidateFilesUnderPathForTesting(reporter,
         ModifiedFileSet.EVERYTHING_MODIFIED, rootDirectory);
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
index aff2791531..9ca17e7cb2 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
@@ -58,24 +58,27 @@ public final class BazelAnalysisMock extends AnalysisMock {
   }
 
   @Override
-  public void setupMockClient(MockToolsConfig config) throws IOException {
+  public List<String> getWorkspaceContents(MockToolsConfig config) {
     String bazelToolWorkspace = config.getPath("/bazel_tools_workspace").getPathString();
-    ArrayList<String> workspaceContents =
-        new ArrayList<>(
-            ImmutableList.of(
-                "local_repository(name = 'bazel_tools', path = '" + bazelToolWorkspace + "')",
-                "local_repository(name = 'local_config_xcode', path = '/local_config_xcode')",
-                "bind(",
-                "  name = 'objc_proto_lib',",
-                "  actual = '//objcproto:ProtocolBuffers_lib',",
-                ")",
-                "bind(",
-                "  name = 'objc_protobuf_lib',",
-                "  actual = '//objcproto:protobuf_lib',",
-                ")",
-                "bind(name = 'android/sdk', actual='@bazel_tools//tools/android:sdk')",
-                "bind(name = 'tools/python', actual='//tools/python')"));
+    return new ArrayList<>(
+        ImmutableList.of(
+            "local_repository(name = 'bazel_tools', path = '" + bazelToolWorkspace + "')",
+            "local_repository(name = 'local_config_xcode', path = '/local_config_xcode')",
+            "bind(",
+            "  name = 'objc_proto_lib',",
+            "  actual = '//objcproto:ProtocolBuffers_lib',",
+            ")",
+            "bind(",
+            "  name = 'objc_protobuf_lib',",
+            "  actual = '//objcproto:protobuf_lib',",
+            ")",
+            "bind(name = 'android/sdk', actual='@bazel_tools//tools/android:sdk')",
+            "bind(name = 'tools/python', actual='//tools/python')"));
+  }
 
+  @Override
+  public void setupMockClient(MockToolsConfig config) throws IOException {
+    List<String> workspaceContents = getWorkspaceContents(config);
     config.create(
         "/local_config_xcode/BUILD", "xcode_config(name = 'host_xcodes')");
     config.overwrite("WORKSPACE", workspaceContents.toArray(new String[workspaceContents.size()]));
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java
index 04d1bd22fb..3d4709778a 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java
@@ -88,6 +88,11 @@ public abstract class AnalysisMock extends LoadingMock {
   public abstract void setupMockClient(MockToolsConfig mockToolsConfig) throws IOException;
 
   /**
+   * Returns the contents of WORKSPACE.
+   */
+  public abstract List<String> getWorkspaceContents(MockToolsConfig config);
+
+  /**
    * This is called from test setup to create any necessary mock workspace files in the
    * <code>_embedded_binaries</code> directory.
    */
@@ -141,6 +146,11 @@ public abstract class AnalysisMock extends LoadingMock {
     }
 
     @Override
+    public List<String> getWorkspaceContents(MockToolsConfig mockToolsConfig) {
+      return delegate.getWorkspaceContents(mockToolsConfig);
+    }
+
+    @Override
     public void setupMockWorkspaceFiles(Path embeddedBinariesRoot) throws IOException {
       delegate.setupMockWorkspaceFiles(embeddedBinariesRoot);
     }
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java
index 915ec37359..1ffa3bd9b7 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java
@@ -17,6 +17,7 @@ package com.google.devtools.build.lib.bazel.repository.skylark;
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.fail;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
@@ -133,10 +134,12 @@ public class SkylarkRepositoryIntegrationTest extends BuildViewTestCase {
         "    local=True,",
         "    attrs={'path': attr.string(mandatory=True)})");
     scratch.file(rootDirectory.getRelative("BUILD").getPathString());
-    scratch.overwriteFile(
-        rootDirectory.getRelative("WORKSPACE").getPathString(),
-        "load('//:def.bzl', 'repo')",
-        "repo(name='foo', path='/repo2')");
+    scratch.overwriteFile(rootDirectory.getRelative("WORKSPACE").getPathString(),
+        new ImmutableList.Builder<String>()
+            .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+            .add("load('//:def.bzl', 'repo')")
+            .add("repo(name='foo', path='/repo2')")
+            .build());
     invalidatePackages();
     ConfiguredTarget target = getConfiguredTarget("@foo//:bar");
     Object path = target.getTarget().getAssociatedRule().getAttributeContainer().getAttr("path");
@@ -158,11 +161,13 @@ public class SkylarkRepositoryIntegrationTest extends BuildViewTestCase {
         "    implementation=_impl,",
         "    local=True)");
     scratch.file(rootDirectory.getRelative("BUILD").getPathString());
-    scratch.overwriteFile(
-        rootDirectory.getRelative("WORKSPACE").getPathString(),
-        "local_repository(name='repo2', path='/repo2')",
-        "load('//:def.bzl', 'repo')",
-        "repo(name='foo')");
+    scratch.overwriteFile(rootDirectory.getRelative("WORKSPACE").getPathString(),
+        new ImmutableList.Builder<String>()
+            .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+            .add("local_repository(name='repo2', path='/repo2')")
+            .add("load('//:def.bzl', 'repo')")
+            .add("repo(name='foo')")
+            .build());
     invalidatePackages();
     ConfiguredTarget target = getConfiguredTarget("@foo//:bar");
     Object path = target.getTarget().getAssociatedRule().getAttributeContainer().getAttr("path");
@@ -185,11 +190,13 @@ public class SkylarkRepositoryIntegrationTest extends BuildViewTestCase {
         "    implementation=_impl,",
         "    local=True)");
     scratch.file(rootDirectory.getRelative("BUILD").getPathString());
-    scratch.overwriteFile(
-        rootDirectory.getRelative("WORKSPACE").getPathString(),
-        "local_repository(name='repo2', path='/repo2')",
-        "load('//:def.bzl', 'repo')",
-        "repo(name='foo')");
+    scratch.overwriteFile(rootDirectory.getRelative("WORKSPACE").getPathString(),
+        new ImmutableList.Builder<String>()
+            .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+            .add("local_repository(name='repo2', path='/repo2')")
+            .add("load('//:def.bzl', 'repo')")
+            .add("repo(name='foo')")
+            .build());
     invalidatePackages();
     ConfiguredTarget target = getConfiguredTarget("@foo//:bar");
     Object path = target.getTarget().getAssociatedRule().getAttributeContainer().getAttr("path");
@@ -213,11 +220,13 @@ public class SkylarkRepositoryIntegrationTest extends BuildViewTestCase {
         "    implementation=_impl,",
         "    local=True)");
     scratch.file(rootDirectory.getRelative("BUILD").getPathString());
-    scratch.overwriteFile(
-        rootDirectory.getRelative("WORKSPACE").getPathString(),
-        "local_repository(name='repo2', path='/repo2')",
-        "load('//:def.bzl', 'repo')",
-        "repo(name='foobar')");
+    scratch.overwriteFile(rootDirectory.getRelative("WORKSPACE").getPathString(),
+        new ImmutableList.Builder<String>()
+            .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+            .add("local_repository(name='repo2', path='/repo2')")
+            .add("load('//:def.bzl', 'repo')")
+            .add("repo(name='foobar')")
+            .build());
     invalidatePackages();
     ConfiguredTarget target = getConfiguredTarget("@foobar//:bar");
     Object path = target.getTarget().getAssociatedRule().getAttributeContainer().getAttr("path");
@@ -305,12 +314,15 @@ public class SkylarkRepositoryIntegrationTest extends BuildViewTestCase {
     scratch.file("/repo2/BUILD", "exports_files_(['data.txt'])");
     scratch.file("/repo2/def.bzl", "def macro():", "  print('bleh')");
     scratch.file("/repo2/WORKSPACE");
-    scratch.overwriteFile(
-        rootDirectory.getRelative("WORKSPACE").getPathString(),
-        "local_repository(name='bleh')",
-        "local_repository(name='foo', path='/repo2')",
-        "load('@foo//:def.bzl', 'repo')",
-        "repo(name='foobar')");
+
+    scratch.overwriteFile(rootDirectory.getRelative("WORKSPACE").getPathString(),
+        new ImmutableList.Builder<String>()
+            .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+            .add("local_repository(name='bleh')")
+            .add("local_repository(name='foo', path='/repo2')")
+            .add("load('@foo//:def.bzl', 'repo')")
+            .add("repo(name='foobar')")
+            .build());
     try {
       invalidatePackages();
       getTarget("@foo//:data.txt");
@@ -325,11 +337,12 @@ public class SkylarkRepositoryIntegrationTest extends BuildViewTestCase {
   @Test
   public void testLoadDoesNotHideWorkspaceFunction() throws Exception {
     scratch.file("def.bzl", "def macro():", "  print('bleh')");
-    scratch.overwriteFile(
-        rootDirectory.getRelative("WORKSPACE").getPathString(),
-        "workspace(name='bleh')",
-        "local_repository(name='bazel_tools', path=__workspace_dir__)",
-        "load('//:def.bzl', 'macro')");
+    scratch.overwriteFile(rootDirectory.getRelative("WORKSPACE").getPathString(),
+        new ImmutableList.Builder<String>()
+            .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+            .add("workspace(name='bleh')")
+            .add("load('//:def.bzl', 'macro')")
+            .build());
     scratch.file("data.txt");
     scratch.file("BUILD", "filegroup(", "  name='files', ", "  srcs=['data.txt'])");
     invalidatePackages();
@@ -365,15 +378,17 @@ public class SkylarkRepositoryIntegrationTest extends BuildViewTestCase {
 
   @Test
   public void testMultipleLoadSameExtension() throws Exception {
-    scratch.overwriteFile(
-        rootDirectory.getRelative("WORKSPACE").getPathString(),
-        "load('//:def.bzl', 'f1')",
-        "f1()",
-        "load('//:def.bzl', 'f2')",
-        "f2()",
-        "load('//:def.bzl', 'f1')",
-        "f1()",
-        "local_repository(name = 'foo', path = '')");
+    scratch.overwriteFile(rootDirectory.getRelative("WORKSPACE").getPathString(),
+        new ImmutableList.Builder<String>()
+            .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+            .add("load('//:def.bzl', 'f1')")
+            .add("f1()")
+            .add("load('//:def.bzl', 'f2')")
+            .add("f2()")
+            .add("load('//:def.bzl', 'f1')")
+            .add("f1()")
+            .add("local_repository(name = 'foo', path = '')")
+            .build());
     scratch.file(
         rootDirectory.getRelative("BUILD").getPathString(), "filegroup(name = 'bar', srcs = [])");
     scratch.file(
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
index 063da16ae2..078d6af669 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -339,7 +339,11 @@ public class SkylarkRuleContextTest extends SkylarkTestCase {
   public void testPackageBoundaryError_ExternalRepository() throws Exception {
     scratch.file("/r/BUILD", "cc_library(name = 'cclib',", "  srcs = ['sub/my_sub_lib.h'])");
     scratch.file("/r/sub/BUILD", "cc_library(name = 'my_sub_lib', srcs = ['my_sub_lib.h'])");
-    scratch.overwriteFile("WORKSPACE", "local_repository(name='r', path='/r')");
+    scratch.overwriteFile("WORKSPACE",
+        new ImmutableList.Builder<String>()
+            .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+            .add("local_repository(name='r', path='/r')")
+            .build());
     invalidatePackages(/*alsoConfigs=*/false); // Repository shuffling messes with toolchain labels.
     reporter.removeHandler(failFastHandler);
     getConfiguredTarget("@r//:cclib");
@@ -825,7 +829,10 @@ public class SkylarkRuleContextTest extends SkylarkTestCase {
         "external_rule(name='r')");
 
     scratch.overwriteFile("WORKSPACE",
-        "local_repository(name='r', path='/r')");
+        new ImmutableList.Builder<String>()
+            .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+            .add("local_repository(name='r', path='/r')")
+            .build());
 
     invalidatePackages(/*alsoConfigs=*/false); // Repository shuffling messes with toolchain labels.
     SkylarkRuleContext context = createRuleContext("@r//a:r");
@@ -856,7 +863,10 @@ public class SkylarkRuleContextTest extends SkylarkTestCase {
         "external_rule(name='r')");
 
     scratch.overwriteFile("WORKSPACE",
-        "local_repository(name='r', path='/r')");
+        new ImmutableList.Builder<String>()
+            .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+            .add("local_repository(name='r', path='/r')")
+            .build());
 
     invalidatePackages(/*alsoConfigs=*/false); // Repository shuffling messes with toolchain labels.
     SkylarkRuleContext context = createRuleContext("@r//a:r");
@@ -885,15 +895,19 @@ public class SkylarkRuleContextTest extends SkylarkTestCase {
         "  print(name + ': ' + path)"
     );
     scratch.file("BUILD");
-    scratch.overwriteFile(
-        "WORKSPACE",
-        "local_repository(name='r2', path='/r2')",
-        "load('@r2//:test.bzl', 'macro')",
-        "macro('r1', '/r1')",
-        "NEXT_NAME = 'r3'",
-        "load('@r2//:other_test.bzl', 'other_macro')",  // We can still refer to r2 in other chunks.
-        "macro(NEXT_NAME, '/r2')"  // and we can still use macro outside of its chunk.
-    );
+
+    scratch.overwriteFile("WORKSPACE",
+        new ImmutableList.Builder<String>()
+            .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+            .add("local_repository(name='r2', path='/r2')")
+            .add("load('@r2//:test.bzl', 'macro')")
+            .add("macro('r1', '/r1')")
+            .add("NEXT_NAME = 'r3'")
+            // We can still refer to r2 in other chunks:
+            .add("load('@r2//:other_test.bzl', 'other_macro')")
+            .add("macro(NEXT_NAME, '/r2')") // and we can still use macro outside of its chunk.
+            .build());
+
     invalidatePackages(/*alsoConfigs=*/false); // Repository shuffling messes with toolchain labels.
     assertThat(getConfiguredTarget("@r1//:test")).isNotNull();
   }
@@ -908,10 +922,13 @@ public class SkylarkRuleContextTest extends SkylarkTestCase {
     scratch.file("/baz/WORKSPACE");
     scratch.file("/baz/baz.txt");
     scratch.file("/baz/BUILD", "filegroup(name = 'baz', srcs = ['baz.txt'])");
-    scratch.overwriteFile(
-        "WORKSPACE",
-        "local_repository(name = 'foo', path = '/bar')",
-        "local_repository(name = 'foo', path = '/baz')");
+    scratch.overwriteFile("WORKSPACE",
+        new ImmutableList.Builder<String>()
+            .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+            .add("local_repository(name = 'foo', path = '/bar')")
+            .add("local_repository(name = 'foo', path = '/baz')")
+            .build());
+
     invalidatePackages(/*alsoConfigs=*/false); // Repository shuffling messes with toolchain labels.
     assertThat(
             (List<Label>)
@@ -924,19 +941,23 @@ public class SkylarkRuleContextTest extends SkylarkTestCase {
 
     scratch.overwriteFile("BUILD");
     scratch.overwriteFile("bar.bzl", "dummy = 1");
-    scratch.overwriteFile(
-        "WORKSPACE",
-        "local_repository(name = 'foo', path = '/bar')",
-        "load('//:bar.bzl', 'dummy')",
-        "local_repository(name = 'foo', path = '/baz')");
+
+    scratch.overwriteFile("WORKSPACE",
+        new ImmutableList.Builder<String>()
+            .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+            .add("local_repository(name = 'foo', path = '/bar')")
+            .add("load('//:bar.bzl', 'dummy')")
+            .add("local_repository(name = 'foo', path = '/baz')")
+            .build());
+
     try {
       invalidatePackages(/*alsoConfigs=*/false); // Repository shuffling messes with toolchains.
       createRuleContext("@foo//:baz");
       fail("Should have failed because repository 'foo' is overloading after a load!");
     } catch (Exception ex) {
       assertContainsEvent(
-          "ERROR /workspace/WORKSPACE:3:1: Cannot redefine repository after any load statement "
-              + "in the WORKSPACE file (for repository 'foo')");
+          "Cannot redefine repository after any load statement in the WORKSPACE file "
+              + "(for repository 'foo')");
     }
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/Scratch.java b/src/test/java/com/google/devtools/build/lib/testutil/Scratch.java
index 6004661321..0cca601b29 100644
--- a/src/test/java/com/google/devtools/build/lib/testutil/Scratch.java
+++ b/src/test/java/com/google/devtools/build/lib/testutil/Scratch.java
@@ -24,6 +24,7 @@ import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
 import java.io.IOException;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
+import java.util.Collection;
 
 /**
  * Allow tests to easily manage scratch files in a FileSystem.
@@ -141,6 +142,14 @@ public final class Scratch {
    * Like {@code scratch.file}, but the file is first deleted if it already
    * exists.
    */
+  public Path overwriteFile(String pathName, Collection<String> lines)  throws IOException {
+    return overwriteFile(pathName, lines.toArray(new String[lines.size()]));
+  }
+
+  /**
+   * Like {@code scratch.file}, but the file is first deleted if it already
+   * exists.
+   */
   public Path overwriteFile(String pathName, String... lines) throws IOException {
     return overwriteFile(pathName, DEFAULT_CHARSET, lines);
   }