Roll forward of commit 500175fcfb37953f50cf0869df164902755807f2: Don't include absolute paths in blaze IDE
artifacts
NEW: Fixed bazel java_test rules
Rollback of commit a4ba24e286b7e9a7a09e333d8eb64a8cf7e81895.
*** Reason for rollback ***
Rollforward, fixing bazel java_test rules
*** Original change description ***
Automated [] rollback of commit 500175fcfb37953f50cf0869df164902755807f2.
*** Reason for rollback ***
Breaks Bazel Build
http://ci.bazel.io/job/Bazel/JAVA_VERSION=1.7,PLATFORM_NAME=linux-x86_64/356/console
*** Original change description ***
Don't include absolute paths in blaze IDE artifacts
RELNOTES: Don't include absolute paths in blaze IDE artifacts
--
MOS_MIGRATED_REVID=114751236
diff --git a/src/test/java/com/google/devtools/build/android/ideinfo/ArtifactLocationConverterTest.java b/src/test/java/com/google/devtools/build/android/ideinfo/ArtifactLocationConverterTest.java
new file mode 100644
index 0000000..0ac6311
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/android/ideinfo/ArtifactLocationConverterTest.java
@@ -0,0 +1,96 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.android.ideinfo;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.fail;
+
+import com.google.common.base.Joiner;
+import com.google.devtools.build.lib.ideinfo.androidstudio.PackageManifestOuterClass.ArtifactLocation;
+import com.google.devtools.common.options.OptionsParsingException;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Tests {@link ArtifactLocationConverter}.
+ */
+@RunWith(JUnit4.class)
+public class ArtifactLocationConverterTest {
+
+ private ArtifactLocationConverter converter;
+
+ @Before
+ public final void init() throws Exception {
+ converter = new ArtifactLocationConverter();
+ }
+
+ @Test
+ public void testConverterSourceArtifact() throws Exception {
+ ArtifactLocation parsed = converter.convert(
+ Joiner.on(",").join("", "test.java", "/usr/local/code")
+ );
+ assertThat(parsed).isEqualTo(
+ ArtifactLocation.newBuilder()
+ .setRootPath("/usr/local/code")
+ .setRelativePath("test.java")
+ .setIsSource(true)
+ .build());
+ }
+
+ @Test
+ public void testConverterDerivedArtifact() throws Exception {
+ ArtifactLocation parsed = converter.convert(
+ Joiner.on(",").join("bin", "java/com/test.java", "/usr/local/_tmp/code/bin")
+ );
+ assertThat(parsed).isEqualTo(
+ ArtifactLocation.newBuilder()
+ .setRootPath("/usr/local/_tmp/code/bin")
+ .setRootExecutionPathFragment("bin")
+ .setRelativePath("java/com/test.java")
+ .setIsSource(false)
+ .build());
+ }
+
+ @Test
+ public void testInvalidFormatFails() throws Exception {
+ assertFails("/root", "Expected either 2 or 3 comma-separated paths");
+ assertFails("/root,exec,rel,extra", "Expected either 2 or 3 comma-separated paths");
+ }
+
+ @Test
+ public void testFutureFormat() throws Exception {
+ ArtifactLocation parsed = converter.convert("bin/out,java/com/test.java");
+ assertThat(parsed).isEqualTo(
+ ArtifactLocation.newBuilder()
+ .setRootExecutionPathFragment("bin/out")
+ .setRelativePath("java/com/test.java")
+ .setIsSource(false)
+ .build());
+ }
+
+
+ private void assertFails(String input, String expectedError) {
+ try {
+ new ArtifactLocationConverter().convert(input);
+ fail();
+ } catch (OptionsParsingException e) {
+ assertThat(e).hasMessage(expectedError);
+ }
+ }
+}
+
diff --git a/src/test/java/com/google/devtools/build/android/ideinfo/BUILD b/src/test/java/com/google/devtools/build/android/ideinfo/BUILD
index 79f75dd..f268b1e 100644
--- a/src/test/java/com/google/devtools/build/android/ideinfo/BUILD
+++ b/src/test/java/com/google/devtools/build/android/ideinfo/BUILD
@@ -1,8 +1,9 @@
java_test(
name = "PackageParserTest",
size = "small",
- srcs = glob(["*.java"]),
+ srcs = ["PackageParserTest.java"],
deps = [
+ "//src/main/java/com/google/devtools/common/options",
"//src/main/protobuf:package_manifest_proto",
"//src/tools/android/java/com/google/devtools/build/android/ideinfo:package_parser_lib",
"//third_party:guava",
@@ -12,3 +13,18 @@
"//third_party:truth",
],
)
+
+java_test(
+ name = "ArtifactLocationConverterTest",
+ size = "small",
+ srcs = ["ArtifactLocationConverterTest.java"],
+ deps = [
+ "//src/main/java/com/google/devtools/common/options",
+ "//src/main/protobuf:package_manifest_proto",
+ "//src/tools/android/java/com/google/devtools/build/android/ideinfo:package_parser_lib",
+ "//third_party:guava",
+ "//third_party:junit4",
+ "//third_party:protobuf",
+ "//third_party:truth",
+ ],
+)
diff --git a/src/test/java/com/google/devtools/build/android/ideinfo/PackageParserTest.java b/src/test/java/com/google/devtools/build/android/ideinfo/PackageParserTest.java
index a02ac30..3f466d6 100644
--- a/src/test/java/com/google/devtools/build/android/ideinfo/PackageParserTest.java
+++ b/src/test/java/com/google/devtools/build/android/ideinfo/PackageParserTest.java
@@ -17,9 +17,11 @@
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.fail;
+import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
+import com.google.devtools.build.lib.ideinfo.androidstudio.PackageManifestOuterClass.ArtifactLocation;
import com.google.protobuf.MessageLite;
import org.junit.Before;
@@ -35,6 +37,7 @@
import java.io.StringWriter;
import java.io.UnsupportedEncodingException;
import java.nio.charset.StandardCharsets;
+import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
@@ -50,12 +53,16 @@
private static class MockPackageParserIoProvider extends PackageParserIoProvider {
private final Map<Path, InputStream> sources = Maps.newHashMap();
+ private final List<ArtifactLocation> sourceLocations = Lists.newArrayList();
private StringWriter writer = new StringWriter();
- public MockPackageParserIoProvider addSource(String filePath, String javaSrc) {
+ public MockPackageParserIoProvider addSource(ArtifactLocation source, String javaSrc) {
try {
- sources.put(Paths.get(filePath), new ByteArrayInputStream(javaSrc.getBytes("UTF-8")));
- } catch (UnsupportedEncodingException e) {
+ Path path = Paths.get(source.getRootExecutionPathFragment(), source.getRelativePath());
+ sources.put(path, new ByteArrayInputStream(javaSrc.getBytes("UTF-8")));
+ sourceLocations.add(source);
+
+ } catch (UnsupportedEncodingException | InvalidPathException e) {
fail(e.getMessage());
}
return this;
@@ -63,11 +70,12 @@
public void reset() {
sources.clear();
+ sourceLocations.clear();
writer = new StringWriter();
}
- public List<Path> getPaths() {
- return Lists.newArrayList(sources.keySet());
+ public List<ArtifactLocation> getSourceLocations() {
+ return Lists.newArrayList(sourceLocations);
}
@Nonnull
@@ -83,6 +91,21 @@
}
}
+ private static final ArtifactLocation DUMMY_SOURCE_ARTIFACT =
+ ArtifactLocation.newBuilder()
+ .setRootPath("/usr/local/google/code")
+ .setRelativePath("java/com/google/Foo.java")
+ .setIsSource(true)
+ .build();
+
+ private static final ArtifactLocation DUMMY_DERIVED_ARTIFACT =
+ ArtifactLocation.newBuilder()
+ .setRootPath("/usr/local/_tmp/code/bin")
+ .setRootExecutionPathFragment("bin")
+ .setRelativePath("java/com/google/Bla.java")
+ .setIsSource(false)
+ .build();
+
private MockPackageParserIoProvider mockIoProvider;
private PackageParser parser;
@@ -92,9 +115,9 @@
parser = new PackageParser(mockIoProvider);
}
- private Map<Path, String> parsePackageStrings() throws Exception {
- List<Path> paths = mockIoProvider.getPaths();
- return parser.parsePackageStrings(paths, paths);
+ private Map<ArtifactLocation, String> parsePackageStrings() throws Exception {
+ List<ArtifactLocation> sources = mockIoProvider.getSourceLocations();
+ return parser.parsePackageStrings(sources);
}
@Test
@@ -102,91 +125,205 @@
String[] args = new String[] {
"--output_manifest",
"/tmp/out.manifest",
- "--sources_absolute_paths",
- "/path/test1.java:/path/test2.java",
- "--sources_execution_paths",
- "/path/test1.java:/path/test2.java"
- };
+ "--sources",
+ Joiner.on(":").join(
+ ",java/com/google/Foo.java,/usr/local/google/code",
+ "bin,java/com/google/Bla.java,/usr/local/_tmp/code/bin"
+ )};
PackageParser.PackageParserOptions options = PackageParser.parseArgs(args);
assertThat(options.outputManifest.toString()).isEqualTo("/tmp/out.manifest");
- assertThat(options.sourcesAbsolutePaths).hasSize(2);
- assertThat(options.sourcesExecutionPaths).hasSize(2);
- assertThat(options.sourcesAbsolutePaths.get(0).toString()).isEqualTo("/path/test1.java");
- assertThat(options.sourcesAbsolutePaths.get(1).toString()).isEqualTo("/path/test2.java");
+ assertThat(options.sources).hasSize(2);
+ assertThat(options.sources.get(0)).isEqualTo(
+ ArtifactLocation.newBuilder()
+ .setRootPath("/usr/local/google/code")
+ .setRelativePath("java/com/google/Foo.java")
+ .setIsSource(true)
+ .build());
+ assertThat(options.sources.get(1)).isEqualTo(
+ ArtifactLocation.newBuilder()
+ .setRootPath("/usr/local/_tmp/code/bin")
+ .setRootExecutionPathFragment("bin")
+ .setRelativePath("java/com/google/Bla.java")
+ .setIsSource(false)
+ .build());
}
@Test
public void testReadNoSources() throws Exception {
- Map<Path, String> map = parsePackageStrings();
+ Map<ArtifactLocation, String> map = parsePackageStrings();
assertThat(map).isEmpty();
}
@Test
public void testSingleRead() throws Exception {
mockIoProvider
- .addSource("java/com/google/Bla.java",
- "package com.test;\n public class Bla {}\"");
- Map<Path, String> map = parsePackageStrings();
+ .addSource(
+ DUMMY_SOURCE_ARTIFACT,
+ "package com.google;\n public class Bla {}\"");
+ Map<ArtifactLocation, String> map = parsePackageStrings();
assertThat(map).hasSize(1);
- assertThat(map).containsEntry(Paths.get("java/com/google/Bla.java"), "com.test");
+ assertThat(map).containsEntry(
+ DUMMY_SOURCE_ARTIFACT,
+ "com.google");
}
@Test
public void testMultiRead() throws Exception {
mockIoProvider
- .addSource("java/com/google/Bla.java",
+ .addSource(
+ DUMMY_SOURCE_ARTIFACT,
"package com.test;\n public class Bla {}\"")
- .addSource("java/com/other/Foo.java",
+ .addSource(
+ DUMMY_DERIVED_ARTIFACT,
"package com.other;\n public class Foo {}\"");
- Map<Path, String> map = parsePackageStrings();
+ Map<ArtifactLocation, String> map = parsePackageStrings();
assertThat(map).hasSize(2);
- assertThat(map).containsEntry(Paths.get("java/com/google/Bla.java"), "com.test");
- assertThat(map).containsEntry(Paths.get("java/com/other/Foo.java"), "com.other");
+ assertThat(map).containsEntry(
+ DUMMY_SOURCE_ARTIFACT,
+ "com.test");
+ assertThat(map).containsEntry(
+ DUMMY_DERIVED_ARTIFACT,
+ "com.other");
}
@Test
public void testReadSomeInvalid() throws Exception {
mockIoProvider
- .addSource("java/com/google/Bla.java",
+ .addSource(
+ DUMMY_SOURCE_ARTIFACT,
"package %com.test;\n public class Bla {}\"")
- .addSource("java/com/other/Foo.java",
+ .addSource(
+ DUMMY_DERIVED_ARTIFACT,
"package com.other;\n public class Foo {}\"");
- Map<Path, String> map = parsePackageStrings();
+ Map<ArtifactLocation, String> map = parsePackageStrings();
assertThat(map).hasSize(1);
- assertThat(map).containsEntry(Paths.get("java/com/other/Foo.java"), "com.other");
+ assertThat(map).containsEntry(
+ DUMMY_DERIVED_ARTIFACT,
+ "com.other");
}
@Test
public void testReadAllInvalid() throws Exception {
mockIoProvider
- .addSource("java/com/google/Bla.java",
+ .addSource(
+ DUMMY_SOURCE_ARTIFACT,
"#package com.test;\n public class Bla {}\"")
- .addSource("java/com/other/Foo.java",
+ .addSource(
+ DUMMY_DERIVED_ARTIFACT,
"package com.other\n public class Foo {}\"");
- Map<Path, String> map = parsePackageStrings();
+ Map<ArtifactLocation, String> map = parsePackageStrings();
assertThat(map).isEmpty();
}
@Test
public void testWriteEmptyMap() throws Exception {
parser.writeManifest(
- Maps.<Path, String> newHashMap(), Paths.get("/java/com/google/test.manifest"));
+ Maps.<ArtifactLocation, String> newHashMap(),
+ Paths.get("/java/com/google/test.manifest"),
+ false);
assertThat(mockIoProvider.writer.toString()).isEmpty();
}
@Test
public void testWriteMap() throws Exception {
- Map<Path, String> map = ImmutableMap.of(
- Paths.get("/java/com/google/Bla.java"), "com.google",
- Paths.get("/java/com/other/Foo.java"), "com.other"
+ Map<ArtifactLocation, String> map = ImmutableMap.of(
+ DUMMY_SOURCE_ARTIFACT,
+ "com.google",
+ DUMMY_DERIVED_ARTIFACT,
+ "com.other"
);
- parser.writeManifest(map, Paths.get("/java/com/google/test.manifest"));
+ parser.writeManifest(map, Paths.get("/java/com/google/test.manifest"), false);
String writtenString = mockIoProvider.writer.toString();
- assertThat(writtenString).contains("absolute_path: \"/java/com/google/Bla.java\"");
+ assertThat(writtenString).contains(String.format(
+ "root_path: \"%s\"",
+ DUMMY_SOURCE_ARTIFACT.getRootPath()));
+ assertThat(writtenString).contains(String.format(
+ "relative_path: \"%s\"",
+ DUMMY_SOURCE_ARTIFACT.getRelativePath()));
assertThat(writtenString).contains("package_string: \"com.google\"");
- assertThat(writtenString).contains("absolute_path: \"/java/com/other/Foo.java\"");
+
+ assertThat(writtenString).contains(String.format(
+ "root_path: \"%s\"",
+ DUMMY_DERIVED_ARTIFACT.getRootPath()));
+ assertThat(writtenString).contains(String.format(
+ "root_execution_path_fragment: \"%s\"",
+ DUMMY_DERIVED_ARTIFACT.getRootExecutionPathFragment()));
+ assertThat(writtenString).contains(String.format(
+ "relative_path: \"%s\"",
+ DUMMY_DERIVED_ARTIFACT.getRelativePath()));
assertThat(writtenString).contains("package_string: \"com.other\"");
}
+ @Test
+ public void testHandlesOldFormat() throws Exception {
+ String[] args = new String[] {
+ "--output_manifest",
+ "/tmp/out.manifest",
+ "--sources_absolute_paths",
+ "/usr/local/code/java/com/google/Foo.java:/usr/local/code/java/com/google/Bla.java",
+ "--sources_execution_paths",
+ "java/com/google/Foo.java:java/com/google/Bla.java"
+ };
+ PackageParser.PackageParserOptions options = PackageParser.parseArgs(args);
+ assertThat(options.outputManifest.toString()).isEqualTo("/tmp/out.manifest");
+ assertThat(options.sourcesAbsolutePaths.get(0).toString())
+ .isEqualTo("/usr/local/code/java/com/google/Foo.java");
+ assertThat(options.sourcesAbsolutePaths.get(1).toString())
+ .isEqualTo("/usr/local/code/java/com/google/Bla.java");
+
+ PackageParser.convertFromOldFormat(options);
+ assertThat(options.sources).hasSize(2);
+ assertThat(options.sources.get(0)).isEqualTo(
+ ArtifactLocation.newBuilder()
+ .setRootPath("/usr/local/code/")
+ .setRelativePath("java/com/google/Foo.java")
+ .build());
+
+ mockIoProvider
+ .addSource(options.sources.get(0),
+ "package com.test;\n public class Bla {}\"")
+ .addSource(options.sources.get(1),
+ "package com.other;\n public class Foo {}\"");
+
+ Map<ArtifactLocation, String> map = parser.parsePackageStrings(options.sources);
+ assertThat(map).hasSize(2);
+ assertThat(map).containsEntry(options.sources.get(0), "com.test");
+ assertThat(map).containsEntry(options.sources.get(1), "com.other");
+
+ parser.writeManifest(map, options.outputManifest, true);
+
+ String writtenString = mockIoProvider.writer.toString();
+ assertThat(writtenString).doesNotContain("location");
+ assertThat(writtenString).contains(
+ "absolute_path: \"/usr/local/code/java/com/google/Foo.java\"");
+ assertThat(writtenString).contains(
+ "absolute_path: \"/usr/local/code/java/com/google/Bla.java\"");
+ }
+
+ @Test
+ public void testHandlesFutureFormat() throws Exception {
+ String[] args = new String[] {
+ "--output_manifest",
+ "/tmp/out.manifest",
+ "--sources",
+ ",java/com/google/Foo.java:"
+ + "bin/out,java/com/google/Bla.java",
+ };
+ PackageParser.PackageParserOptions options = PackageParser.parseArgs(args);
+ assertThat(options.outputManifest.toString()).isEqualTo("/tmp/out.manifest");
+ assertThat(options.sources).hasSize(2);
+ assertThat(options.sources.get(0)).isEqualTo(
+ ArtifactLocation.newBuilder()
+ .setRelativePath("java/com/google/Foo.java")
+ .setIsSource(true)
+ .build());
+ assertThat(options.sources.get(1)).isEqualTo(
+ ArtifactLocation.newBuilder()
+ .setRootExecutionPathFragment("bin/out")
+ .setRelativePath("java/com/google/Bla.java")
+ .setIsSource(false)
+ .build());
+ }
+
}
diff --git a/src/test/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspectTest.java b/src/test/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspectTest.java
index 6810d5d..22416fc 100644
--- a/src/test/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspectTest.java
+++ b/src/test/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspectTest.java
@@ -43,6 +43,7 @@
import java.io.InputStream;
import java.io.InputStreamReader;
import java.nio.charset.StandardCharsets;
+import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
@@ -69,6 +70,10 @@
"//com/google/example:simple", ruleIdeInfos);
if (isNativeTest()) {
assertThat(ruleIdeInfo.getBuildFile()).isEqualTo(buildFilePath.toString());
+ ArtifactLocation location = ruleIdeInfo.getBuildFileArtifactLocation();
+ assertThat(Paths.get(location.getRootPath(), location.getRelativePath()).toString())
+ .isEqualTo(buildFilePath.toString());
+ assertThat(location.getRelativePath()).isEqualTo("com/google/example/BUILD");
}
assertThat(ruleIdeInfo.getKind()).isEqualTo(Kind.JAVA_LIBRARY);
assertThat(ruleIdeInfo.getDependenciesCount()).isEqualTo(0);
@@ -907,26 +912,21 @@
")");
Map<String, RuleIdeInfo> ruleIdeInfos = buildRuleIdeInfo("//com/google/example:lib");
RuleIdeInfo ruleIdeInfo = getRuleInfoAndVerifyLabel("//com/google/example:lib", ruleIdeInfos);
- assertThat(ruleIdeInfo.getJavaRuleIdeInfo().getSourcesList())
- .containsExactly(
- expectedArtifactLocationWithRootPath(
- targetConfig.getGenfilesDirectory().getPath().getPathString())
- .setRelativePath("com/google/example/gen.java")
- .setIsSource(false)
- .build(),
- expectedArtifactLocationWithRootPath(directories.getWorkspace().getPathString())
- .setRelativePath("com/google/example/Test.java")
- .setIsSource(true)
- .build());
- }
-
- private ArtifactLocation.Builder expectedArtifactLocationWithRootPath(String pathString) {
- if (isNativeTest()) {
- return ArtifactLocation.newBuilder().setRootPath(pathString);
- } else {
- // todo(dslomov): Skylark aspect implementation does not yet return a correct root path.
- return ArtifactLocation.newBuilder();
- }
+ // todo(dslomov): Skylark aspect implementation does not yet return a correct root path.
+ assertThat(ruleIdeInfo.getJavaRuleIdeInfo().getSourcesList()).containsExactly(
+ ArtifactLocation.newBuilder()
+ .setRootPath(
+ isNativeTest() ? targetConfig.getGenfilesDirectory().getPath().getPathString() : "")
+ .setRootExecutionPathFragment(
+ isNativeTest() ? targetConfig.getGenfilesDirectory().getExecPathString() : "")
+ .setRelativePath("com/google/example/gen.java")
+ .setIsSource(false)
+ .build(),
+ ArtifactLocation.newBuilder()
+ .setRootPath(isNativeTest() ? directories.getWorkspace().getPathString() : "")
+ .setRelativePath("com/google/example/Test.java")
+ .setIsSource(true)
+ .build());
}
@Test
