Don't print absolute paths when:
(1) Printing the "location" of ERROR/INFO/etc messages. Instead, print a path relative to the workspace or a --package_path root. Behavior is controlled by flag --experimental_ui_attempt_to_print_relative_paths).
(2) Printing the absolute path to a test log. Instead print a relative path under the "testlogs" convenience symlink. Behavior is controlled by flag --print_relative_test_log_paths.
There are drawbacks to both these new features (discussed below and also in the flag help strings).
Out of scope (these can be addressed in followup feature requests, as necessary):
(3) Arbitrary strings in messages.
(4) Non-usefulness of relative paths when running Blaze underneath the workspace directory.
(5) Non-usefulness of the a path that contains the "testlogs" convenience symlink when changing configurations in a subsequent invocation.
(6) Other things besides error message prefixes and test log paths in test summaries.
RELNOTES: None
PiperOrigin-RevId: 218762875
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/ExperimentalEventHandlerTest.java b/src/test/java/com/google/devtools/build/lib/runtime/ExperimentalEventHandlerTest.java
new file mode 100644
index 0000000..b2ace7c
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/runtime/ExperimentalEventHandlerTest.java
@@ -0,0 +1,101 @@
+// Copyright 2018 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.lib.runtime;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.events.Location.LineAndColumn;
+import com.google.devtools.build.lib.vfs.FileSystem;
+import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.vfs.Root;
+import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@link ExperimentalEventHandler} static methods. */
+@RunWith(JUnit4.class)
+public class ExperimentalEventHandlerTest {
+ private final FileSystem fileSystem = new InMemoryFileSystem();
+
+ @Test
+ public void getRelativeLocationString_PathIsAlreadyRelative() {
+ assertThat(
+ ExperimentalEventHandler.getRelativeLocationString(
+ Location.fromPathAndStartColumn(
+ PathFragment.create("relative/path"), 0, 0, new LineAndColumn(4, 2)),
+ PathFragment.create("/this/is/the/workspace"),
+ ImmutableList.of(
+ Root.fromPath(fileSystem.getPath("/this/is/a/package/path/root")))))
+ .isEqualTo("relative/path:4:2");
+ }
+
+ @Test
+ public void getRelativeLocationString_PathIsAbsoluteAndWorkspaceIsNull() {
+ assertThat(
+ ExperimentalEventHandler.getRelativeLocationString(
+ Location.fromPathAndStartColumn(
+ PathFragment.create("/absolute/path"), 0, 0, new LineAndColumn(4, 2)),
+ null,
+ ImmutableList.of(
+ Root.fromPath(fileSystem.getPath("/this/is/a/package/path/root")))))
+ .isEqualTo("/absolute/path:4:2");
+ }
+
+ @Test
+ public void getRelativeLocationString_PathIsAbsoluteButNotUnderWorkspaceOrPackagePathRoots() {
+ assertThat(
+ ExperimentalEventHandler.getRelativeLocationString(
+ Location.fromPathAndStartColumn(
+ PathFragment.create("/absolute/path"), 0, 0, new LineAndColumn(4, 2)),
+ PathFragment.create("/this/is/the/workspace"),
+ ImmutableList.of(
+ Root.fromPath(fileSystem.getPath("/this/is/a/package/path/root")))))
+ .isEqualTo("/absolute/path:4:2");
+ }
+
+ @Test
+ public void getRelativeLocationString_PathIsAbsoluteAndUnderWorkspace() {
+ assertThat(
+ ExperimentalEventHandler.getRelativeLocationString(
+ Location.fromPathAndStartColumn(
+ PathFragment.create("/this/is/the/workspace/blah.txt"),
+ 0,
+ 0,
+ new LineAndColumn(4, 2)),
+ PathFragment.create("/this/is/the/workspace"),
+ ImmutableList.of(
+ Root.fromPath(fileSystem.getPath("/this/is/a/package/path/root")))))
+ .isEqualTo("blah.txt:4:2");
+ }
+
+ @Test
+ public void getRelativeLocationString_PathIsAbsoluteAndUnderPackagePathRoot() {
+ assertThat(
+ ExperimentalEventHandler.getRelativeLocationString(
+ Location.fromPathAndStartColumn(
+ PathFragment.create("/this/is/a/package/path/root3/blah.txt"),
+ 0,
+ 0,
+ new LineAndColumn(4, 2)),
+ PathFragment.create("/this/is/the/workspace"),
+ ImmutableList.of(
+ Root.fromPath(fileSystem.getPath("/this/is/a/package/path/root1")),
+ Root.fromPath(fileSystem.getPath("/this/is/a/package/path/root2")),
+ Root.fromPath(fileSystem.getPath("/this/is/a/package/path/root3")))))
+ .isEqualTo("blah.txt:4:2");
+ }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifierTest.java b/src/test/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifierTest.java
index af2e211..7ab653b 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifierTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifierTest.java
@@ -24,6 +24,7 @@
import com.google.devtools.build.lib.exec.TestStrategy.TestSummaryFormat;
import com.google.devtools.build.lib.runtime.TerminalTestResultNotifier.TestSummaryOptions;
import com.google.devtools.build.lib.util.io.AnsiTerminalPrinter;
+import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.view.test.TestStatus.BlazeTestStatus;
import com.google.devtools.build.lib.view.test.TestStatus.TestCase;
import com.google.devtools.build.lib.view.test.TestStatus.TestCase.Status;
@@ -61,8 +62,8 @@
int numOfFailedCases = random.nextInt(numOfTotalTestCases);
int numOfSuccessfulTestCases = numOfTotalTestCases - numOfFailedCases;
- TerminalTestResultNotifier terminalTestResultNotifier =
- new TerminalTestResultNotifier(ansiTerminalPrinter, optionsParsingResult);
+ TerminalTestResultNotifier terminalTestResultNotifier = new TerminalTestResultNotifier(
+ ansiTerminalPrinter, Path::getPathString, optionsParsingResult);
TestSummary testSummary = Mockito.mock(TestSummary.class);
when(testSummary.getTotalTestCases()).thenReturn(numOfTotalTestCases);
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/TestSummaryTest.java b/src/test/java/com/google/devtools/build/lib/runtime/TestSummaryTest.java
index 6bee1df..3a885d2 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/TestSummaryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/TestSummaryTest.java
@@ -96,7 +96,7 @@
AnsiTerminalPrinter terminalPrinter = Mockito.mock(AnsiTerminalPrinter.class);
TestSummary summaryStatus = createTestSummary(target, BlazeTestStatus.PASSED, CACHED);
- TestSummaryPrinter.print(summaryStatus, terminalPrinter, true, false);
+ TestSummaryPrinter.print(summaryStatus, terminalPrinter, Path::getPathString, true, false);
terminalPrinter.print(find(expectedString));
}
@@ -106,7 +106,7 @@
AnsiTerminalPrinter terminalPrinter = Mockito.mock(AnsiTerminalPrinter.class);
TestSummary summary = createTestSummary(stubTarget, BlazeTestStatus.PASSED, NOT_CACHED);
- TestSummaryPrinter.print(summary, terminalPrinter, true, false);
+ TestSummaryPrinter.print(summary, terminalPrinter, Path::getPathString, true, false);
verify(terminalPrinter).print(find(expectedString));
}
@@ -118,7 +118,7 @@
TestSummary summary = createTestSummary(stubTarget, BlazeTestStatus.FAILED, NOT_CACHED);
- TestSummaryPrinter.print(summary, terminalPrinter, true, false);
+ TestSummaryPrinter.print(summary, terminalPrinter, Path::getPathString, true, false);
terminalPrinter.print(find(expectedString));
}
@@ -126,7 +126,11 @@
private void assertShouldNotPrint(BlazeTestStatus status) throws Exception {
AnsiTerminalPrinter terminalPrinter = Mockito.mock(AnsiTerminalPrinter.class);
TestSummaryPrinter.print(
- createTestSummary(stubTarget, status, NOT_CACHED), terminalPrinter, true, false);
+ createTestSummary(stubTarget, status, NOT_CACHED),
+ terminalPrinter,
+ Path::getPathString,
+ true,
+ false);
verify(terminalPrinter, never()).print(anyString());
}
@@ -147,7 +151,7 @@
TestSummary summary = createTestSummary(stubTarget, BlazeTestStatus.PASSED, CACHED);
- TestSummaryPrinter.print(summary, terminalPrinter, true, false);
+ TestSummaryPrinter.print(summary, terminalPrinter, Path::getPathString, true, false);
terminalPrinter.print(find(expectedString));
}
@@ -158,7 +162,7 @@
AnsiTerminalPrinter terminalPrinter = Mockito.mock(AnsiTerminalPrinter.class);
TestSummary summary = createTestSummary(stubTarget, BlazeTestStatus.PASSED, CACHED - 1);
- TestSummaryPrinter.print(summary, terminalPrinter, true, false);
+ TestSummaryPrinter.print(summary, terminalPrinter, Path::getPathString, true, false);
terminalPrinter.print(find(expectedString));
}
@@ -166,7 +170,7 @@
public void testIncompleteCached() throws Exception {
AnsiTerminalPrinter terminalPrinter = Mockito.mock(AnsiTerminalPrinter.class);
TestSummary summary = createTestSummary(stubTarget, BlazeTestStatus.INCOMPLETE, CACHED - 1);
- TestSummaryPrinter.print(summary, terminalPrinter, true, false);
+ TestSummaryPrinter.print(summary, terminalPrinter, Path::getPathString, true, false);
verify(terminalPrinter).print(not(contains("cached")));
}
@@ -174,7 +178,7 @@
public void testShouldPrintUncachedStatus() throws Exception {
AnsiTerminalPrinter terminalPrinter = Mockito.mock(AnsiTerminalPrinter.class);
TestSummary summary = createTestSummary(stubTarget, BlazeTestStatus.PASSED, NOT_CACHED);
- TestSummaryPrinter.print(summary, terminalPrinter, true, false);
+ TestSummaryPrinter.print(summary, terminalPrinter, Path::getPathString, true, false);
verify(terminalPrinter).print(not(contains("cached")));
}
@@ -185,7 +189,7 @@
TestSummary summary = createTestSummary(stubTarget, BlazeTestStatus.PASSED, NOT_CACHED);
- TestSummaryPrinter.print(summary, terminalPrinter, true, false);
+ TestSummaryPrinter.print(summary, terminalPrinter, Path::getPathString, true, false);
terminalPrinter.print(find(expectedString));
}
@@ -234,7 +238,7 @@
AnsiTerminalPrinter terminalPrinter = Mockito.mock(AnsiTerminalPrinter.class);
TestSummary summary = basicBuilder.addTestTimes(ImmutableList.of(3412L)).build();
- TestSummaryPrinter.print(summary, terminalPrinter, true, false);
+ TestSummaryPrinter.print(summary, terminalPrinter, Path::getPathString, true, false);
terminalPrinter.print(find(expectedString));
}
@@ -245,7 +249,7 @@
AnsiTerminalPrinter terminalPrinter = Mockito.mock(AnsiTerminalPrinter.class);
TestSummary summary = basicBuilder.addTestTimes(ImmutableList.of(3412L)).build();
- TestSummaryPrinter.print(summary, terminalPrinter, false, false);
+ TestSummaryPrinter.print(summary, terminalPrinter, Path::getPathString, false, false);
terminalPrinter.print(find(expectedString));
}
@@ -258,7 +262,7 @@
TestSummary summary = basicBuilder
.addTestTimes(ImmutableList.of(1000L, 2000L, 3000L))
.build();
- TestSummaryPrinter.print(summary, terminalPrinter, true, false);
+ TestSummaryPrinter.print(summary, terminalPrinter, Path::getPathString, true, false);
terminalPrinter.print(find(expectedString));
}
@@ -271,7 +275,7 @@
TestSummary summary = basicBuilder.addCoverageFiles(paths).build();
AnsiTerminalPrinter terminalPrinter = Mockito.mock(AnsiTerminalPrinter.class);
- TestSummaryPrinter.print(summary, terminalPrinter, true, false);
+ TestSummaryPrinter.print(summary, terminalPrinter, Path::getPathString, true, false);
verify(terminalPrinter).print(find(ANY_STRING + "INFO" + ANY_STRING + BlazeTestStatus.PASSED));
verify(terminalPrinter).print(find(" /cov2.dat"));
verify(terminalPrinter).print(find(" /cov4.dat"));
@@ -288,7 +292,7 @@
.addPassedLogs(getPathList("/a"))
.addFailedLogs(getPathList("/b", "/c"))
.build();
- TestSummaryPrinter.print(summary, terminalPrinter, true, false);
+ TestSummaryPrinter.print(summary, terminalPrinter, Path::getPathString, true, false);
terminalPrinter.print(find(expectedString));
}
@@ -303,7 +307,7 @@
.addPassedLogs(getPathList("/a"))
.addFailedLogs(getPathList("/b", "/c"))
.build();
- TestSummaryPrinter.print(summary, terminalPrinter, true, false);
+ TestSummaryPrinter.print(summary, terminalPrinter, Path::getPathString, true, false);
terminalPrinter.print(find(expectedString));
}
@@ -320,7 +324,7 @@
// Check that only //package:name is printed.
AnsiTerminalPrinter printer = Mockito.mock(AnsiTerminalPrinter.class);
- TestSummaryPrinter.print(summary, printer, true, true);
+ TestSummaryPrinter.print(summary, printer, Path::getPathString, true, true);
verify(printer).print(contains("//package:name"));
}
@@ -331,7 +335,7 @@
BlazeTestStatus.FAILED, emptyList, FailedTestCasesStatus.NOT_AVAILABLE);
AnsiTerminalPrinter printer = Mockito.mock(AnsiTerminalPrinter.class);
- TestSummaryPrinter.print(summary, printer, true, true);
+ TestSummaryPrinter.print(summary, printer, Path::getPathString, true, true);
verify(printer).print(contains("//package:name"));
verify(printer).print(contains("not available"));
}
@@ -344,7 +348,7 @@
FailedTestCasesStatus.PARTIAL);
AnsiTerminalPrinter printer = Mockito.mock(AnsiTerminalPrinter.class);
- TestSummaryPrinter.print(summary, printer, true, true);
+ TestSummaryPrinter.print(summary, printer, Path::getPathString, true, true);
verify(printer).print(contains("//package:name"));
verify(printer).print(find("FAILED.*orange"));
verify(printer).print(contains("incomplete"));
@@ -372,11 +376,11 @@
assertThat(summaryFailed.getStatus()).isEqualTo(BlazeTestStatus.FAILED);
AnsiTerminalPrinter printerPassed = Mockito.mock(AnsiTerminalPrinter.class);
- TestSummaryPrinter.print(summaryPassed, printerPassed, true, true);
+ TestSummaryPrinter.print(summaryPassed, printerPassed, Path::getPathString, true, true);
verify(printerPassed).print(contains("//package:name"));
AnsiTerminalPrinter printerFailed = Mockito.mock(AnsiTerminalPrinter.class);
- TestSummaryPrinter.print(summaryFailed, printerFailed, true, true);
+ TestSummaryPrinter.print(summaryFailed, printerFailed, Path::getPathString, true, true);
verify(printerFailed).print(contains("//package:name"));
verify(printerFailed).print(find("FAILED.*orange *\\(1\\.5"));
}
@@ -411,7 +415,7 @@
// A mock that checks the ordering of method calls
AnsiTerminalPrinter printer = Mockito.mock(AnsiTerminalPrinter.class);
- TestSummaryPrinter.print(summary, printer, true, true);
+ TestSummaryPrinter.print(summary, printer, Path::getPathString, true, true);
InOrder order = Mockito.inOrder(printer);
order.verify(printer).print(contains("//package:name"));
order.verify(printer).print(find("FAILED.*apple"));
@@ -450,7 +454,7 @@
.build();
AnsiTerminalPrinter printer = Mockito.mock(AnsiTerminalPrinter.class);
- TestSummaryPrinter.print(summary, printer, true, true);
+ TestSummaryPrinter.print(summary, printer, Path::getPathString, true, true);
verify(printer).print(contains("//package:name"));
verify(printer).print(find("FAILED.*apple"));
verify(printer).print(find("ERROR.*cherry"));
