Refactor WorkspaceFactoryTestHelper to be able to add a setSkylarkSemantics method.

RELNOTES: None
PiperOrigin-RevId: 199366685

2 files changed, 94 insertions, 84 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java
index 82b29a1cc6..ce06be666f 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java
@@ -27,10 +27,12 @@ import org.junit.runners.JUnit4;
 @RunWith(JUnit4.class)
 public class WorkspaceFactoryTest {
 
+  private WorkspaceFactoryTestHelper helper = new WorkspaceFactoryTestHelper();
+
   @Test
   public void testLoadError() throws Exception {
     // WS with a syntax error: '//a' should end with .bzl.
-    WorkspaceFactoryTestHelper helper = parse("load('//a', 'a')");
+    helper.parse("load('//a', 'a')");
     helper.assertLexingExceptionThrown();
     assertThat(helper.getLexerError())
         .contains("The label must reference a file with extension '.bzl'");
@@ -38,128 +40,120 @@ public class WorkspaceFactoryTest {
 
   @Test
   public void testWorkspaceName() throws Exception {
-    WorkspaceFactoryTestHelper helper = parse("workspace(name = 'my_ws')");
+    helper.parse("workspace(name = 'my_ws')");
     assertThat(helper.getPackage().getWorkspaceName()).isEqualTo("my_ws");
   }
 
   @Test
   public void testWorkspaceStartsWithNumber() throws Exception {
-    WorkspaceFactoryTestHelper helper = parse("workspace(name = '123abc')");
+    helper.parse("workspace(name = '123abc')");
     assertThat(helper.getParserError()).contains("123abc is not a legal workspace name");
   }
 
   @Test
   public void testWorkspaceWithIllegalCharacters() throws Exception {
-    WorkspaceFactoryTestHelper helper = parse("workspace(name = 'a.b.c')");
+    helper.parse("workspace(name = 'a.b.c')");
     assertThat(helper.getParserError()).contains("a.b.c is not a legal workspace name");
   }
 
   @Test
   public void testIllegalRepoName() throws Exception {
-    WorkspaceFactoryTestHelper helper =
-        parse("local_repository(", "    name = 'foo/bar',", "    path = '/foo/bar',", ")");
+    helper.parse("local_repository(", "    name = 'foo/bar',", "    path = '/foo/bar',", ")");
     assertThat(helper.getParserError()).contains(
         "local_repository rule //external:foo/bar's name field must be a legal workspace name");
   }
 
   @Test
   public void testIllegalWorkspaceFunctionPosition() throws Exception {
-    WorkspaceFactoryTestHelper helper =
-        new WorkspaceFactoryTestHelper(false, "workspace(name = 'foo')");
+    helper = new WorkspaceFactoryTestHelper(/*allowOverride=*/ false);
+    helper.parse("workspace(name = 'foo')");
     assertThat(helper.getParserError()).contains(
         "workspace() function should be used only at the top of the WORKSPACE file");
   }
 
   @Test
   public void testRegisterExecutionPlatforms() throws Exception {
-    WorkspaceFactoryTestHelper helper = parse("register_execution_platforms('//platform:ep1')");
+    helper.parse("register_execution_platforms('//platform:ep1')");
     assertThat(helper.getPackage().getRegisteredExecutionPlatforms())
         .containsExactly("//platform:ep1");
   }
 
   @Test
   public void testRegisterExecutionPlatforms_multipleLabels() throws Exception {
-    WorkspaceFactoryTestHelper helper =
-        parse("register_execution_platforms(", "  '//platform:ep1',", "  '//platform:ep2')");
+    helper.parse("register_execution_platforms(", "  '//platform:ep1',", "  '//platform:ep2')");
     assertThat(helper.getPackage().getRegisteredExecutionPlatforms())
         .containsExactly("//platform:ep1", "//platform:ep2");
   }
 
   @Test
   public void testRegisterExecutionPlatforms_multipleCalls() throws Exception {
-    WorkspaceFactoryTestHelper helper =
-        parse(
-            "register_execution_platforms('//platform:ep1')",
-            "register_execution_platforms('//platform:ep2')",
-            "register_execution_platforms('//platform/...')");
+    helper.parse(
+        "register_execution_platforms('//platform:ep1')",
+        "register_execution_platforms('//platform:ep2')",
+        "register_execution_platforms('//platform/...')");
     assertThat(helper.getPackage().getRegisteredExecutionPlatforms())
         .containsExactly("//platform:ep1", "//platform:ep2", "//platform/...");
   }
 
   @Test
   public void testRegisterToolchains() throws Exception {
-    WorkspaceFactoryTestHelper helper = parse("register_toolchains('//toolchain:tc1')");
+    helper.parse("register_toolchains('//toolchain:tc1')");
     assertThat(helper.getPackage().getRegisteredToolchains()).containsExactly("//toolchain:tc1");
   }
 
   @Test
   public void testRegisterToolchains_multipleLabels() throws Exception {
-    WorkspaceFactoryTestHelper helper =
-        parse("register_toolchains(", "  '//toolchain:tc1',", "  '//toolchain:tc2')");
+    helper.parse("register_toolchains(", "  '//toolchain:tc1',", "  '//toolchain:tc2')");
     assertThat(helper.getPackage().getRegisteredToolchains())
         .containsExactly("//toolchain:tc1", "//toolchain:tc2");
   }
 
   @Test
   public void testRegisterToolchains_multipleCalls() throws Exception {
-    WorkspaceFactoryTestHelper helper =
-        parse(
-            "register_toolchains('//toolchain:tc1')",
-            "register_toolchains('//toolchain:tc2')",
-            "register_toolchains('//toolchain/...')");
+    helper.parse(
+        "register_toolchains('//toolchain:tc1')",
+        "register_toolchains('//toolchain:tc2')",
+        "register_toolchains('//toolchain/...')");
     assertThat(helper.getPackage().getRegisteredToolchains())
         .containsExactly("//toolchain:tc1", "//toolchain:tc2", "//toolchain/...");
   }
 
   @Test
   public void testWorkspaceMappings() throws Exception {
-    WorkspaceFactoryTestHelper helper =
-        parse(
-            "local_repository(",
-            "    name = 'foo',",
-            "    path = '/foo',",
-            "    repo_mapping = {'@x' : '@y'},",
-            ")");
+    helper.parse(
+        "local_repository(",
+        "    name = 'foo',",
+        "    path = '/foo',",
+        "    repo_mapping = {'@x' : '@y'},",
+        ")");
     assertMapping(helper, "@foo", "@x", "@y");
   }
 
   @Test
   public void testMultipleRepositoriesWithMappings() throws Exception {
-    WorkspaceFactoryTestHelper helper =
-        parse(
-            "local_repository(",
-            "    name = 'foo',",
-            "    path = '/foo',",
-            "    repo_mapping = {'@x' : '@y'},",
-            ")",
-            "local_repository(",
-            "    name = 'bar',",
-            "    path = '/bar',",
-            "    repo_mapping = {'@a' : '@b'},",
-            ")");
+    helper.parse(
+        "local_repository(",
+        "    name = 'foo',",
+        "    path = '/foo',",
+        "    repo_mapping = {'@x' : '@y'},",
+        ")",
+        "local_repository(",
+        "    name = 'bar',",
+        "    path = '/bar',",
+        "    repo_mapping = {'@a' : '@b'},",
+        ")");
     assertMapping(helper, "@foo", "@x", "@y");
     assertMapping(helper, "@bar", "@a", "@b");
   }
 
   @Test
   public void testMultipleMappings() throws Exception {
-    WorkspaceFactoryTestHelper helper =
-        parse(
-            "local_repository(",
-            "    name = 'foo',",
-            "    path = '/foo',",
-            "    repo_mapping = {'@a' : '@b', '@c' : '@d', '@e' : '@f'},",
-            ")");
+    helper.parse(
+        "local_repository(",
+        "    name = 'foo',",
+        "    path = '/foo',",
+        "    repo_mapping = {'@a' : '@b', '@c' : '@d', '@e' : '@f'},",
+        ")");
     assertMapping(helper, "@foo", "@a", "@b");
     assertMapping(helper, "@foo", "@c", "@d");
     assertMapping(helper, "@foo", "@e", "@f");
@@ -167,35 +161,32 @@ public class WorkspaceFactoryTest {
 
   @Test
   public void testEmptyMappings() throws Exception {
-    WorkspaceFactoryTestHelper helper =
-        parse(
-            "local_repository(",
-            "    name = 'foo',",
-            "    path = '/foo',",
-            "    repo_mapping = {},",
-            ")");
+    helper.parse(
+        "local_repository(",
+        "    name = 'foo',",
+        "    path = '/foo',",
+        "    repo_mapping = {},",
+        ")");
     assertThat(helper.getPackage().getRepositoryMapping("@foo")).isEmpty();
   }
 
   @Test
   public void testMappingsNotAMap() throws Exception {
-    WorkspaceFactoryTestHelper helper =
-        parse(
-            "local_repository(",
-            "    name = 'foo',",
-            "    path = '/foo',",
-            "    repo_mapping = 1",
-            ")");
+    helper.parse(
+        "local_repository(",
+        "    name = 'foo',",
+        "    path = '/foo',",
+        "    repo_mapping = 1",
+        ")");
     assertThat(helper.getParserError())
         .contains("Invalid value for 'repo_mapping': '1'. Value must be a map.");
 
-    helper =
-        parse(
-            "local_repository(",
-            "    name = 'foo',",
-            "    path = '/foo',",
-            "    repo_mapping = 'hello'",
-            ")");
+    helper.parse(
+        "local_repository(",
+        "    name = 'foo',",
+        "    path = '/foo',",
+        "    repo_mapping = 'hello'",
+        ")");
     assertThat(helper.getParserError())
         .contains("Invalid value for 'repo_mapping': 'hello'. Value must be a map.");
   }
@@ -209,8 +200,4 @@ public class WorkspaceFactoryTest {
         .containsEntry(localRepoName, globalRepoName);
   }
 
-  private WorkspaceFactoryTestHelper parse(String... args) {
-    return new WorkspaceFactoryTestHelper(args);
-  }
-
 }
diff --git a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTestHelper.java b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTestHelper.java
index 60d6df6566..716b78d15e 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTestHelper.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTestHelper.java
@@ -27,24 +27,31 @@ import com.google.devtools.build.lib.testutil.Scratch;
 import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.common.options.OptionsParser;
 import java.io.IOException;
 import java.util.List;
 
 /** Parses a WORKSPACE file with the given content. */
 class WorkspaceFactoryTestHelper {
-  private final Package.Builder builder;
-  private final Exception exception;
-  private final ImmutableList<Event> events;
+  private Package.Builder builder;
+  private Exception exception;
+  private ImmutableList<Event> events;
+  private SkylarkSemantics skylarkSemantics;
 
-  WorkspaceFactoryTestHelper(String... args) {
-    this(true, args);
+  private final boolean allowOverride;
+
+  WorkspaceFactoryTestHelper() {
+    this(true);
   }
 
-  WorkspaceFactoryTestHelper(boolean allowOverride, String... args) {
-    this(new Scratch("/"), allowOverride, args);
+  WorkspaceFactoryTestHelper(boolean allowOverride) {
+    this.exception = null;
+    this.events = null;
+    this.allowOverride = allowOverride;
+    this.skylarkSemantics = SkylarkSemantics.DEFAULT_SEMANTICS;
   }
 
-  WorkspaceFactoryTestHelper(Scratch scratch, boolean allowOverride, String... args) {
+  void parse(Scratch scratch, String... args) {
     Path root = null;
     Path workspaceFilePath = null;
     try {
@@ -73,7 +80,7 @@ class WorkspaceFactoryTestHelper {
           FileSystemUtils.readWithKnownFileSize(workspaceFilePath, workspaceFilePath.getFileSize());
       factory.parse(
           ParserInputSource.create(bytes, workspaceFilePath.asFragment()),
-          SkylarkSemantics.DEFAULT_SEMANTICS,
+          skylarkSemantics,
           eventHandler);
     } catch (BuildFileContainsErrorsException e) {
       exception = e;
@@ -84,6 +91,10 @@ class WorkspaceFactoryTestHelper {
     this.exception = exception;
   }
 
+  void parse(String... args) {
+    parse(new Scratch("/"), args);
+  }
+
   public Package getPackage() throws InterruptedException, NoSuchPackageException {
     return builder.build();
   }
@@ -103,4 +114,16 @@ class WorkspaceFactoryTestHelper {
     assertThat(events.size()).isGreaterThan(0);
     return events.get(0).getMessage();
   }
+
+  protected void setSkylarkSemantics(String... options) throws Exception {
+    skylarkSemantics = parseSkylarkSemanticsOptions(options);
+  }
+
+  private static SkylarkSemantics parseSkylarkSemanticsOptions(String... options)
+      throws Exception {
+    OptionsParser parser = OptionsParser.newOptionsParser(SkylarkSemanticsOptions.class);
+    parser.parse(options);
+    return parser.getOptions(SkylarkSemanticsOptions.class).toSkylarkSemantics();
+  }
+
 }