Enable label-based Skylark loading. In particular, such labels may reference files in external repositories.

In addition:

- Cleaned up and refactored some tests to reflect the new loading behavior.

Deferred to future CLs:

- Updating Bazel Skylark documentation to reflect the new load form.

- Enabling command-line loading of Aspects via labels.

RELNOTES: Skylark load statements may now reference .bzl files via build labels, in addition to paths. In particular, such labels can be used to reference Skylark files in external repositories; e.g., load("@my_external_repo//some_pkg:some_file.bzl", ...). Path-based loads are now deprecated and may be disabled in the future. Caveats: Skylark files currently do not respect package visibility; i.e., all Skylark files are effectively public. Also, loads may not reference the special //external package.

--
MOS_MIGRATED_REVID=110786452
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
index a9e9a18..ac99bc9 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
@@ -35,7 +35,6 @@
 import com.google.devtools.build.lib.testutil.TestUtils;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.vfs.Path;
-import com.google.devtools.build.lib.vfs.PathFragment;
 
 import java.io.IOException;
 
@@ -132,7 +131,7 @@
             ConstantRuleVisibility.PUBLIC,
             false,
             new MakeEnvironment.Builder(),
-            ImmutableMap.<PathFragment, Extension>of(),
+            ImmutableMap.<String, Extension>of(),
             ImmutableList.<Label>of());
     Package result = resultBuilder.build();
     Event.replayEventsOn(eventHandler, result.getEvents());
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
index f35577f..5a5d7cc 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
@@ -385,10 +385,11 @@
         getSkyframeExecutor(), skyKey, /*keepGoing=*/false, reporter);
     assertTrue(result.hasError());
     ErrorInfo errorInfo = result.getError(skyKey);
+    String expectedMsg = "error loading package 'test/skylark': "
+        + "Extension file not found. Unable to load file '//test/skylark:bad_extension.bzl': "
+        + "file doesn't exist or isn't a file";
     assertThat(errorInfo.getException())
-        .hasMessage("error loading package 'test/skylark': Extension file not found. "
-            + "Unable to load file '//test/skylark:bad_extension.bzl': "
-            + "file doesn't exist or isn't a file");
+        .hasMessage(expectedMsg);
   }
 
   @Test
@@ -406,9 +407,12 @@
     EvaluationResult<PackageValue> result = SkyframeExecutorTestUtils.evaluate(
         getSkyframeExecutor(), skyKey, /*keepGoing=*/false, reporter);
     assertTrue(result.hasError());
-    assertContainsEvent("Extension file not found. "
-        + "Unable to load file '//test/skylark:bad_extension.bzl': "
-        + "file doesn't exist or isn't a file");
+    ErrorInfo errorInfo = result.getError(skyKey);
+    String expectedMsg = "error loading package 'test/skylark': "
+        + "Extension file not found. Unable to load file '//test/skylark:bad_extension.bzl': "
+        + "file doesn't exist or isn't a file";
+    assertThat(errorInfo.getException())
+        .hasMessage(expectedMsg);
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java
index 3f8d1d1..b174296 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java
@@ -58,22 +58,22 @@
   public void testSkylarkImportLabels() throws Exception {
     scratch.file("pkg1/BUILD");
     scratch.file("pkg1/ext.bzl");
-    checkLabel("//pkg1:ext.bzl", "//pkg1:ext.bzl");
+    checkSuccessfulLookup("//pkg1:ext.bzl");
 
     scratch.file("pkg2/BUILD");
     scratch.file("pkg2/dir/ext.bzl");
-    checkLabel("//pkg2:dir/ext.bzl", "//pkg2:dir/ext.bzl");
+    checkSuccessfulLookup("//pkg2:dir/ext.bzl");
 
     scratch.file("dir/pkg3/BUILD");
     scratch.file("dir/pkg3/dir/ext.bzl");
-    checkLabel("//dir/pkg3:dir/ext.bzl", "//dir/pkg3:dir/ext.bzl");
+    checkSuccessfulLookup("//dir/pkg3:dir/ext.bzl");
   }
 
   @Test
   public void testSkylarkImportLabelsAlternativeRoot() throws Exception {
     scratch.file("/root_2/pkg4/BUILD");
     scratch.file("/root_2/pkg4/ext.bzl");
-    checkLabel("//pkg4:ext.bzl", "//pkg4:ext.bzl");
+    checkSuccessfulLookup("//pkg4:ext.bzl");
   }
 
   @Test
@@ -81,7 +81,23 @@
     scratch.file("dir1/BUILD");
     scratch.file("dir1/dir2/BUILD");
     scratch.file("dir1/dir2/ext.bzl");
-    checkLabel("//dir1/dir2:ext.bzl", "//dir1/dir2:ext.bzl");
+    checkSuccessfulLookup("//dir1/dir2:ext.bzl");
+  }
+
+  @Test
+  public void testLoadFromSkylarkFileInRemoteRepo() throws Exception {
+    scratch.deleteFile("tools/build_rules/prelude_blaze");
+    scratch.overwriteFile("WORKSPACE",
+        "local_repository(",
+        "    name = 'a_remote_repo',",
+        "    path = '/a_remote_repo'",
+        ")");
+    scratch.file("/a_remote_repo/remote_pkg/BUILD");
+    scratch.file("/a_remote_repo/remote_pkg/ext1.bzl",
+        "load(':ext2.bzl', 'CONST')");
+    scratch.file("/a_remote_repo/remote_pkg/ext2.bzl",
+        "CONST = 17");
+    checkSuccessfulLookup("@a_remote_repo//remote_pkg:ext1.bzl");
   }
 
   @Test
@@ -141,11 +157,13 @@
     return SkylarkImportLookupValue.key(Label.parseAbsoluteUnchecked(label), false);
   }
 
-  private void checkLabel(String labelRequested, String labelFound) throws Exception {
-    SkyKey skylarkImportLookupKey = key(labelRequested);
+  // Ensures that a Skylark file has been successfully processed by checking that the
+  // the label in its dependency set corresponds to the requested label.
+  private void checkSuccessfulLookup(String label) throws Exception {
+    SkyKey skylarkImportLookupKey = key(label);
     EvaluationResult<SkylarkImportLookupValue> result = get(skylarkImportLookupKey);
-    assertEquals(labelFound,
-        result.get(skylarkImportLookupKey).getDependency().getLabel().toString());
+    assertEquals(result.get(skylarkImportLookupKey).getDependency().getLabel().toString(),
+        label);
   }
 
   @Test
@@ -195,21 +213,4 @@
     assertEquals("invalid target name 'oops<?>.bzl': "
         + "target names may not contain non-printable characters: '\\x00'", errorMessage);
   }
-
-  @Test
-  public void testSkylarkRelativeImportFilenameWithControlChars() throws Exception {
-    scratch.file("pkg/BUILD", "");
-    scratch.file("pkg/ext.bzl", "load('oops\u0000', 'a')");
-    SkyKey skylarkImportLookupKey =
-        SkylarkImportLookupValue.key(Label.parseAbsoluteUnchecked("//pkg:ext.bzl"), false);
-    EvaluationResult<SkylarkImportLookupValue> result =
-        SkyframeExecutorTestUtils.evaluate(
-            getSkyframeExecutor(), skylarkImportLookupKey, /*keepGoing=*/ false, reporter);
-    assertTrue(result.hasError());
-    ErrorInfo errorInfo = result.getError(skylarkImportLookupKey);
-    String errorMessage = errorInfo.getException().getMessage();
-    assertEquals("invalid target name 'oops<?>.bzl': "
-        + "target names may not contain non-printable characters: '\\x00'", errorMessage);
-  }
-
 }
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
index de81e4e..2985fd1 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
@@ -22,10 +22,12 @@
 
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.syntax.Argument.Passed;
 import com.google.devtools.build.lib.syntax.DictionaryLiteral.DictionaryEntryLiteral;
 import com.google.devtools.build.lib.syntax.util.EvaluationTestCase;
+import com.google.devtools.build.lib.vfs.PathFragment;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -1079,26 +1081,135 @@
     assertContainsError("syntax error");
   }
 
-  private static final String DOUBLE_SLASH_LOAD = "load('//foo/bar/file', 'test')\n";
-  private static final String DOUBLE_SLASH_ERROR =
-      "First argument of load() is a path, not a label. It should start with a "
-      + "single slash if it is an absolute path.";
-
   @Test
-  public void testLoadDoubleSlashBuild() throws Exception {
+  public void testValidAbsoluteImportPath() {
+    List<Statement> statements =
+        parseFileForSkylark("load('/some/skylark/file', 'fun_test')\n");
+    LoadStatement stmt = (LoadStatement) statements.get(0);
+    SkylarkImport imp = stmt.getImport();
+    assertThat(imp.getImportString()).isEqualTo("/some/skylark/file");
+    assertThat(imp.hasAbsolutePath()).isTrue();
+    assertThat(imp.getAbsolutePath()).isEqualTo(new PathFragment("/some/skylark/file.bzl"));
+  }
+
+  private void validNonAbsoluteImportTest(String importString, String containingFileLabelString,
+      String expectedLabelString) {
+    List<Statement> statements =
+        parseFileForSkylark("load('" + importString + "', 'fun_test')\n");
+    LoadStatement stmt = (LoadStatement) statements.get(0);
+    SkylarkImport imp = stmt.getImport();
+    assertThat(imp.getImportString()).isEqualTo(importString);
+    assertThat(imp.hasAbsolutePath()).isFalse();
+    Label containingFileLabel = Label.parseAbsoluteUnchecked(containingFileLabelString);
+    assertThat(imp.getLabel(containingFileLabel))
+    .isEqualTo(Label.parseAbsoluteUnchecked(expectedLabelString));      
+  }
+
+  private void invalidImportTest(String importString, String expectedMsg) {
     setFailFast(false);
-    parseFile(DOUBLE_SLASH_LOAD);
-    assertContainsError(DOUBLE_SLASH_ERROR);
+    parseFileForSkylark("load('" + importString + "', 'fun_test')\n"); 
+    assertContainsError(expectedMsg);    
   }
 
   @Test
-  public void testLoadDoubleSlashSkylark() throws Exception {
-    setFailFast(false);
-    parseFileForSkylark(DOUBLE_SLASH_LOAD);
-    assertContainsError(DOUBLE_SLASH_ERROR);
+  public void testValidRelativeImportPathInPackageDir() throws Exception {
+    validNonAbsoluteImportTest("file", /*containing*/ "//some/skylark:BUILD",
+        /*expected*/ "//some/skylark:file.bzl");
   }
 
   @Test
+  public void testValidRelativeImportPathInPackageSubdir() throws Exception {
+    validNonAbsoluteImportTest("file", /*containing*/ "//some/path/to:skylark/parent.bzl",
+        /*expected*/ "//some/path/to:skylark/file.bzl");
+  }
+
+  @Test
+  public void testInvalidRelativePathBzlExtImplicit() throws Exception {
+    invalidImportTest("file.bzl", SkylarkImports.BZL_EXT_IMPLICIT_MSG);
+  }
+
+  @Test
+  public void testInvalidRelativePathNoSubdirs() throws Exception {
+    invalidImportTest("path/to/file", SkylarkImports.RELATIVE_PATH_NO_SUBDIRS_MSG);
+  }
+
+  @Test
+  public void testInvalidRelativePathInvalidFilename() throws Exception {
+    invalidImportTest("\tfile", SkylarkImports.INVALID_FILENAME_PREFIX);
+  }
+
+  private void validAbsoluteImportLabelTest(String importString) {
+    validNonAbsoluteImportTest(importString, /*irrelevant*/ "//another/path:BUILD",
+        /*expected*/ importString);
+  }
+
+  @Test
+  public void testValidAbsoluteImportLabel() throws Exception {
+    validAbsoluteImportLabelTest("//some/skylark:file.bzl");
+  }
+
+  @Test
+  public void testValidAbsoluteImportLabelWithRepo() throws Exception {
+    validAbsoluteImportLabelTest("@my_repo//some/skylark:file.bzl");
+  }
+
+  @Test
+  public void testInvalidAbsoluteImportLabel() throws Exception {
+    invalidImportTest("//some/skylark/:file.bzl", SkylarkImports.INVALID_LABEL_PREFIX);
+  }
+
+  @Test
+  public void testInvalidAbsoluteImportLabelWithRepo() throws Exception {
+    invalidImportTest("@my_repo//some/skylark/:file.bzl",
+        SkylarkImports.INVALID_LABEL_PREFIX);
+  }
+
+  @Test
+  public void testInvalidAbsoluteImportLabelMissingBzlExt() throws Exception {
+    invalidImportTest("//some/skylark:file", SkylarkImports.MUST_HAVE_BZL_EXT_MSG);
+  }
+
+  @Test
+  public void testInvalidAbsoluteImportReferencesExternalPkg() throws Exception {
+    invalidImportTest("//external:file.bzl", SkylarkImports.EXTERNAL_PKG_NOT_ALLOWED_MSG);
+  }
+
+  @Test
+  public void testValidRelativeImportSimpleLabelInPackageDir() throws Exception {
+    validNonAbsoluteImportTest(":file.bzl", /*containing*/ "//some/skylark:BUILD",
+        /*expected*/ "//some/skylark:file.bzl");
+  }
+
+  @Test
+  public void testValidRelativeImportSimpleLabelInPackageSubdir() throws Exception {
+    validNonAbsoluteImportTest(":file.bzl", /*containing*/ "//some/path/to:skylark/parent.bzl",
+        /*expected*/ "//some/path/to:file.bzl");
+  }
+
+  @Test
+  public void testValidRelativeImportComplexLabelInPackageDir() throws Exception {
+    validNonAbsoluteImportTest(":subdir/containing/file.bzl", /*containing*/ "//some/skylark:BUILD",
+        /*expected*/ "//some/skylark:subdir/containing/file.bzl");
+  }
+
+  @Test
+  public void testValidRelativeImportComplexLabelInPackageSubdir() throws Exception {
+    validNonAbsoluteImportTest(":subdir/containing/file.bzl",
+        /*containing*/ "//some/path/to:skylark/parent.bzl",
+        /*expected*/ "//some/path/to:subdir/containing/file.bzl");
+  }
+
+  @Test
+  public void testInvalidRelativeImportLabelMissingBzlExt() throws Exception {
+    invalidImportTest(":file", SkylarkImports.MUST_HAVE_BZL_EXT_MSG);
+  }
+
+  @Test
+  public void testInvalidRelativeImportLabelSyntax() throws Exception {
+    invalidImportTest("::file.bzl", SkylarkImports.INVALID_TARGET_PREFIX);
+  }
+
+ @Test
   public void testLoadNoSymbol() throws Exception {
     setFailFast(false);
     parseFileForSkylark("load('/foo/bar/file')\n");
@@ -1110,7 +1221,7 @@
     List<Statement> statements = parseFileForSkylark(
         "load('/foo/bar/file', 'fun_test')\n");
     LoadStatement stmt = (LoadStatement) statements.get(0);
-    assertEquals("/foo/bar/file.bzl", stmt.getImportPath().toString());
+    assertEquals("/foo/bar/file", stmt.getImport().getImportString());
     assertThat(stmt.getSymbols()).hasSize(1);
   }
 
@@ -1119,7 +1230,7 @@
     List<Statement> statements = parseFileForSkylark(
         "load('/foo/bar/file', 'fun_test',)\n");
     LoadStatement stmt = (LoadStatement) statements.get(0);
-    assertEquals("/foo/bar/file.bzl", stmt.getImportPath().toString());
+    assertEquals("/foo/bar/file", stmt.getImport().getImportString());
     assertThat(stmt.getSymbols()).hasSize(1);
   }
 
@@ -1128,7 +1239,7 @@
     List<Statement> statements = parseFileForSkylark(
         "load('file', 'foo', 'bar')\n");
     LoadStatement stmt = (LoadStatement) statements.get(0);
-    assertEquals("file.bzl", stmt.getImportPath().toString());
+    assertEquals("file", stmt.getImport().getImportString());
     assertThat(stmt.getSymbols()).hasSize(2);
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkImportTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkImportTest.java
index c233a59..de3452f 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkImportTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkImportTest.java
@@ -34,35 +34,28 @@
   @Rule
   public ExpectedException thrown = ExpectedException.none();
 
-  @Test
-  public void testValidAbsoluteLabel() throws Exception {
-    String labelToTest = "//some/skylark:file.bzl";
-    SkylarkImport importForLabel = SkylarkImports.create(labelToTest);
+  private void validAbsoluteLabelTest(String labelString) throws Exception {
+    SkylarkImport importForLabel = SkylarkImports.create(labelString);
 
     assertThat(importForLabel.hasAbsolutePath()).isFalse();
+    assertThat(importForLabel.getImportString()).isEqualTo(labelString);
 
     Label irrelevantContainingFile = Label.parseAbsoluteUnchecked("//another/path:BUILD");
     assertThat(importForLabel.getLabel(irrelevantContainingFile))
-        .isEqualTo(Label.parseAbsoluteUnchecked("//some/skylark:file.bzl"));
+        .isEqualTo(Label.parseAbsoluteUnchecked(labelString));
 
     thrown.expect(IllegalStateException.class);
-    importForLabel.getAbsolutePath();
+    importForLabel.getAbsolutePath();   
   }
 
+  @Test
+  public void testValidAbsoluteLabel() throws Exception {
+    validAbsoluteLabelTest("//some/skylark:file.bzl");
+  }
 
   @Test
   public void testValidAbsoluteLabelWithRepo() throws Exception {
-    String labelToTest = "@my_repo//some/skylark:file.bzl";
-    SkylarkImport importForLabel = SkylarkImports.create(labelToTest);
-
-    assertThat(importForLabel.hasAbsolutePath()).isFalse();
-
-    Label irrelevantContainingFile = Label.parseAbsoluteUnchecked("//another/path:BUILD");
-    assertThat(importForLabel.getLabel(irrelevantContainingFile))
-        .isEqualTo(Label.parseAbsoluteUnchecked("@my_repo//some/skylark:file.bzl"));
-
-    thrown.expect(IllegalStateException.class);
-    importForLabel.getAbsolutePath();
+    validAbsoluteLabelTest("@my_repo//some/skylark:file.bzl");
   }
 
   @Test
@@ -71,197 +64,141 @@
     SkylarkImport importForPath = SkylarkImports.create(pathToTest);
 
     assertThat(importForPath.hasAbsolutePath()).isTrue();
+    assertThat(importForPath.getImportString()).isEqualTo(pathToTest);
 
     Label irrelevantContainingFile = Label.parseAbsoluteUnchecked("//another/path:BUILD");
-    assertThat(importForPath.getAbsolutePath()).isEqualTo(new PathFragment(pathToTest));
+    assertThat(importForPath.getAbsolutePath()).isEqualTo(new PathFragment(pathToTest + ".bzl"));
 
     thrown.expect(IllegalStateException.class);
     importForPath.getLabel(irrelevantContainingFile);
   }
 
-  @Test
-  public void testValidRelativeSimpleLabelInPackageDir() throws Exception {
-    String labelToTest = ":file.bzl";
-    SkylarkImport importForLabel = SkylarkImports.create(labelToTest);
+  private void validRelativeLabelTest(String labelString,
+      String containingLabelString, String expectedLabelString) throws Exception {
+    SkylarkImport importForLabel = SkylarkImports.create(labelString);
 
     assertThat(importForLabel.hasAbsolutePath()).isFalse();
+    assertThat(importForLabel.getImportString()).isEqualTo(labelString);
 
     // The import label is relative to the parent's package, not the parent's directory.
-    Label containingFile = Label.parseAbsolute("//some/skylark:BUILD");
-    assertThat(importForLabel.getLabel(containingFile))
-        .isEqualTo(Label.parseAbsolute("//some/skylark:file.bzl"));
+    Label containingLabel = Label.parseAbsolute(containingLabelString);
+    assertThat(importForLabel.getLabel(containingLabel))
+    .isEqualTo(Label.parseAbsolute(expectedLabelString));
 
     thrown.expect(IllegalStateException.class);
     importForLabel.getAbsolutePath();
   }
 
   @Test
+  public void testValidRelativeSimpleLabelInPackageDir() throws Exception {
+    validRelativeLabelTest(":file.bzl", /*containing*/ "//some/skylark:BUILD",
+        /*expected*/ "//some/skylark:file.bzl");
+  }
+
+  @Test
   public void testValidRelativeSimpleLabelInPackageSubdir() throws Exception {
-    String labelToTest = ":file.bzl";
-    SkylarkImport importForLabel = SkylarkImports.create(labelToTest);
-
-    assertThat(importForLabel.hasAbsolutePath()).isFalse();
-
-    // The import label is relative to the parent's package, not the parent's directory.
-    Label containingFile = Label.parseAbsolute("//some/path/to:skylark/parent.bzl");
-    assertThat(importForLabel.getLabel(containingFile))
-        .isEqualTo(Label.parseAbsolute("//some/path/to:file.bzl"));
-
-    thrown.expect(IllegalStateException.class);
-    importForLabel.getAbsolutePath();
+    validRelativeLabelTest(":file.bzl", /*containing*/ "//some/path/to:skylark/parent.bzl",
+        /*expected*/ "//some/path/to:file.bzl");
   }
 
   @Test
   public void testValidRelativeComplexLabelInPackageDir() throws Exception {
-    String labelToTest = ":subdir/containing/file.bzl";
-    SkylarkImport importForLabel = SkylarkImports.create(labelToTest);
-
-    assertThat(importForLabel.hasAbsolutePath()).isFalse();
-
-    // The import label is relative to the parent's package, not the parent's directory.
-    Label containingFile = Label.parseAbsolute("//some/skylark:BUILD");
-    assertThat(importForLabel.getLabel(containingFile))
-        .isEqualTo(Label.parseAbsolute("//some/skylark:subdir/containing/file.bzl"));
-
-    thrown.expect(IllegalStateException.class);
-    importForLabel.getAbsolutePath();
+    validRelativeLabelTest(":subdir/containing/file.bzl",
+        /*containing*/ "//some/skylark:BUILD",
+        /*expected*/ "//some/skylark:subdir/containing/file.bzl");
   }
 
   @Test
   public void testValidRelativeComplexLabelInPackageSubdir() throws Exception {
-    String labelToTest = ":subdir/containing/file.bzl";
-    SkylarkImport importForLabel = SkylarkImports.create(labelToTest);
+    validRelativeLabelTest(":subdir/containing/file.bzl",
+        /*containing*/ "//some/path/to:skylark/parent.bzl",
+        /*expected*/ "//some/path/to:subdir/containing/file.bzl");
+  }
 
-    assertThat(importForLabel.hasAbsolutePath()).isFalse();
+  private void validRelativePathTest(String pathString, String containingLabelString,
+      String expectedLabelString) throws Exception {
+    SkylarkImport importForPath = SkylarkImports.create(pathString);
 
-    // The import label is relative to the parent's package, not the parent's directory.
-    Label containingFile = Label.parseAbsolute("//some/path/to:skylark/parent.bzl");
-    assertThat(importForLabel.getLabel(containingFile))
-        .isEqualTo(Label.parseAbsolute("//some/path/to:subdir/containing/file.bzl"));
+    assertThat(importForPath.hasAbsolutePath()).isFalse();
+
+    // The import label is relative to the parent's directory not the parent's package.
+    Label containingLabel = Label.parseAbsolute(containingLabelString);
+    assertThat(importForPath.getLabel(containingLabel))
+    .isEqualTo(Label.parseAbsolute(expectedLabelString));
 
     thrown.expect(IllegalStateException.class);
-    importForLabel.getAbsolutePath();
+    importForPath.getAbsolutePath();
   }
 
   @Test
   public void testValidRelativePathInPackageDir() throws Exception {
-    String pathToTest = "file";
-    SkylarkImport importForPath = SkylarkImports.create(pathToTest);
-
-    assertThat(importForPath.hasAbsolutePath()).isFalse();
-
-    // The import label is relative to the parent's directory not the parent's package.
-    Label containingFile = Label.parseAbsolute("//some/skylark:BUILD");
-    assertThat(importForPath.getLabel(containingFile))
-        .isEqualTo(Label.parseAbsolute("//some/skylark:file.bzl"));
-
-    thrown.expect(IllegalStateException.class);
-    importForPath.getAbsolutePath();
+    validRelativePathTest("file", /*containing*/ "//some/skylark:BUILD",
+        /*expected*/ "//some/skylark:file.bzl");
   }
 
   @Test
   public void testValidRelativePathInPackageSubdir() throws Exception {
-    String pathToTest = "file";
-    SkylarkImport importForPath = SkylarkImports.create(pathToTest);
-    assertThat(importForPath.hasAbsolutePath()).isFalse();
+    validRelativePathTest("file", /*containing*/ "//some/path/to:skylark/parent.bzl",
+        /*expected*/ "//some/path/to:skylark/file.bzl");
+  }
 
-    // The import label is relative to the parent's directory not the parent's package.
-    Label containingFile = Label.parseAbsolute("//some/path/to:skylark/parent.bzl");
-    assertThat(importForPath.getLabel(containingFile))
-        .isEqualTo(Label.parseAbsolute("//some/path/to:skylark/file.bzl"));
-
-    thrown.expect(IllegalStateException.class);
-    importForPath.getAbsolutePath();
+  private void invalidImportTest(String importString, String expectedMsgPrefix) throws Exception {
+    thrown.expect(SkylarkImportSyntaxException.class);
+    thrown.expectMessage(startsWith(expectedMsgPrefix));
+    SkylarkImports.create(importString);   
   }
 
   @Test
   public void testInvalidAbsoluteLabelSyntax() throws Exception {
-    String labelToTest = "//some/skylark/:file.bzl"; // final '/' is illegal
-    
-    thrown.expect(SkylarkImportSyntaxException.class);
-    thrown.expectMessage(startsWith(SkylarkImports.INVALID_LABEL_PREFIX));
-    SkylarkImports.create(labelToTest);
+    // final '/' is illegal
+    invalidImportTest("//some/skylark/:file.bzl", SkylarkImports.INVALID_LABEL_PREFIX);
   }
 
   @Test
   public void testInvalidAbsoluteLabelSyntaxWithRepo() throws Exception {
-    String labelToTest = "@my_repo//some/skylark/:file.bzl"; // final '/' is illegal
-    
-    thrown.expect(SkylarkImportSyntaxException.class);
-    thrown.expectMessage(startsWith(SkylarkImports.INVALID_LABEL_PREFIX));
-    SkylarkImports.create(labelToTest);
+    // final '/' is illegal
+    invalidImportTest("@my_repo//some/skylark/:file.bzl", SkylarkImports.INVALID_LABEL_PREFIX);
   }
 
   @Test
   public void tesInvalidAbsoluteLabelMissingBzlExt() throws Exception {
-    String labelToTest = "//some/skylark:file";
-    
-    thrown.expect(SkylarkImportSyntaxException.class);
-    thrown.expectMessage(SkylarkImports.MUST_HAVE_BZL_EXT_MSG);
-    SkylarkImports.create(labelToTest);
+    invalidImportTest("//some/skylark:file", SkylarkImports.MUST_HAVE_BZL_EXT_MSG);
   }
 
   @Test
   public void tesInvalidAbsoluteLabelReferencesExternalPkg() throws Exception {
-    String labelToTest = "//external:file.bzl";
-    
-    thrown.expect(SkylarkImportSyntaxException.class);
-    thrown.expectMessage(SkylarkImports.EXTERNAL_PKG_NOT_ALLOWED_MSG);
-    SkylarkImports.create(labelToTest);
+    invalidImportTest("//external:file.bzl", SkylarkImports.EXTERNAL_PKG_NOT_ALLOWED_MSG);
   }
 
- @Test
+  @Test
   public void tesInvalidAbsolutePathBzlExtImplicit() throws Exception {
-    String labelToTest = "/some/skylark/file.bzl";
-    
-    thrown.expect(SkylarkImportSyntaxException.class);
-    thrown.expectMessage(SkylarkImports.BZL_EXT_IMPLICIT_MSG);
-    SkylarkImports.create(labelToTest);
+    invalidImportTest("/some/skylark/file.bzl", SkylarkImports.BZL_EXT_IMPLICIT_MSG);
   }
 
   @Test
   public void testInvalidRelativeLabelMissingBzlExt() throws Exception {
-    String labelToTest = ":file";
-    
-    thrown.expect(SkylarkImportSyntaxException.class);
-    thrown.expectMessage(SkylarkImports.MUST_HAVE_BZL_EXT_MSG);
-    SkylarkImports.create(labelToTest);
+    invalidImportTest(":file", SkylarkImports.MUST_HAVE_BZL_EXT_MSG);
   }
 
   @Test
   public void testInvalidRelativeLabelSyntax() throws Exception {
-    String labelToTest = "::file.bzl";
-    
-    thrown.expect(SkylarkImportSyntaxException.class);
-    thrown.expectMessage(startsWith(SkylarkImports.INVALID_TARGET_PREFIX));
-    SkylarkImports.create(labelToTest);
+    invalidImportTest("::file.bzl", SkylarkImports.INVALID_TARGET_PREFIX);
   }
 
   @Test
   public void testInvalidRelativePathBzlExtImplicit() throws Exception {
-    String labelToTest = "file.bzl";
-    
-    thrown.expect(SkylarkImportSyntaxException.class);
-    thrown.expectMessage(SkylarkImports.BZL_EXT_IMPLICIT_MSG);
-    SkylarkImports.create(labelToTest);
+    invalidImportTest("file.bzl", SkylarkImports.BZL_EXT_IMPLICIT_MSG);
   }
 
   @Test
   public void testInvalidRelativePathNoSubdirs() throws Exception {
-    String labelToTest = "path/to/file";
-    
-    thrown.expect(SkylarkImportSyntaxException.class);
-    thrown.expectMessage(SkylarkImports.RELATIVE_PATH_NO_SUBDIRS_MSG);
-    SkylarkImports.create(labelToTest);
+    invalidImportTest("path/to/file", SkylarkImports.RELATIVE_PATH_NO_SUBDIRS_MSG);
   }
 
   @Test
   public void testInvalidRelativePathInvalidFilename() throws Exception {
-    String labelToTest = "\tfile"; // tab character is invalid
-    
-    thrown.expect(SkylarkImportSyntaxException.class);
-    thrown.expectMessage(startsWith(SkylarkImports.INVALID_FILENAME_PREFIX));
-    SkylarkImports.create(labelToTest);
+    // tab character is invalid
+    invalidImportTest("\tfile", SkylarkImports.INVALID_FILENAME_PREFIX);
   }
 }
 
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
index 5adba93..4c6eb6a 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
@@ -270,24 +270,6 @@
   }
 
   @Test
-  public void testLoadRelativePathOneSegment() throws Exception {
-    parse("load('extension', 'a')\n");
-  }
-
-  @Test
-  public void testLoadAbsolutePathMultipleSegments() throws Exception {
-    parse("load('/pkg/extension', 'a')\n");
-  }
-
-  @Test
-  public void testLoadRelativePathMultipleSegments() throws Exception {
-    checkError(
-        "Path 'pkg/extension.bzl' is not valid. It should either start with "
-            + "a slash or refer to a file in the current directory.",
-        "load('pkg/extension', 'a')\n");
-  }
-
-  @Test
   public void testDollarErrorDoesNotLeak() throws Exception {
     setFailFast(false);
     parseFile(