Add test coverage that failures and printed errors are consistent.

Apparently, there's at least one case where errors are printed, but we exit
with a zero exit code. How's that.

--
MOS_MIGRATED_REVID=112950105
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
index 0a92f1f..64dd49d 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
@@ -36,6 +36,7 @@
 import com.google.devtools.build.lib.actions.Actions;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.FailAction;
+import com.google.devtools.build.lib.analysis.BuildView.AnalysisResult;
 import com.google.devtools.build.lib.analysis.DependencyResolver.Dependency;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
@@ -178,7 +179,8 @@
     EventBus eventBus = new EventBus();
     AnalysisFailureRecorder recorder = new AnalysisFailureRecorder();
     eventBus.register(recorder);
-    update(eventBus, defaultFlags().with(Flag.KEEP_GOING), "//foo");
+    AnalysisResult result = update(eventBus, defaultFlags().with(Flag.KEEP_GOING), "//foo");
+    assertThat(result.hasError()).isTrue();
     assertThat(recorder.events).hasSize(1);
     AnalysisFailureEvent event = recorder.events.get(0);
     assertEquals("//foo:bar", event.getFailureReason().toString());
@@ -197,7 +199,8 @@
     LoadingFailureRecorder recorder = new LoadingFailureRecorder();
     eventBus.register(recorder);
     // Note: no need to run analysis for a loading failure.
-    update(eventBus, defaultFlags().with(Flag.KEEP_GOING), "//pkg:foo");
+    AnalysisResult result = update(eventBus, defaultFlags().with(Flag.KEEP_GOING), "//pkg:foo");
+    assertThat(result.hasError()).isTrue();
     assertThat(recorder.events)
         .contains(
             Pair.of(Label.parseAbsolute("//pkg:foo"), Label.parseAbsolute("//nopackage:missing")));
@@ -229,8 +232,9 @@
     LoadingFailureRecorder recorder = new LoadingFailureRecorder();
     eventBus.register(recorder);
     // Note: no need to run analysis for a loading failure.
-    update(eventBus, defaultFlags().with(Flag.KEEP_GOING),
+    AnalysisResult result = update(eventBus, defaultFlags().with(Flag.KEEP_GOING),
         "//third_party/first", "//third_party/third");
+    assertThat(result.hasError()).isTrue();
     assertThat(recorder.events).hasSize(2);
     assertTrue(recorder.events.toString(), recorder.events.contains(
         Pair.of(Label.parseAbsolute("//third_party/first"),
@@ -253,7 +257,8 @@
     EventBus eventBus = new EventBus();
     LoadingFailureRecorder recorder = new LoadingFailureRecorder();
     eventBus.register(recorder);
-    update(eventBus, defaultFlags().with(Flag.KEEP_GOING), "//gp");
+    AnalysisResult result = update(eventBus, defaultFlags().with(Flag.KEEP_GOING), "//gp");
+    assertThat(result.hasError()).isTrue();
     assertThat(recorder.events).hasSize(2);
     assertTrue(recorder.events.toString(), recorder.events.contains(
         Pair.of(Label.parseAbsolute("//gp"),
@@ -397,7 +402,8 @@
   public void testAnalysisErrorMessageWithKeepGoing() throws Exception {
     scratch.file("a/BUILD", "sh_binary(name='a', srcs=['a1.sh', 'a2.sh'])");
     reporter.removeHandler(failFastHandler);
-    update(defaultFlags().with(Flag.KEEP_GOING), "//a");
+    AnalysisResult result = update(defaultFlags().with(Flag.KEEP_GOING), "//a");
+    assertThat(result.hasError()).isTrue();
     assertContainsEvent("errors encountered while analyzing target '//a:a'");
   }
 
@@ -414,7 +420,9 @@
         "sh_library(name = 'rec2', srcs = ['rec2.sh'], deps = [':rec1'])"
     );
     reporter.removeHandler(failFastHandler);
-    update(defaultFlags().with(Flag.KEEP_GOING), "//foo:top1", "//foo:top2");
+    AnalysisResult result =
+        update(defaultFlags().with(Flag.KEEP_GOING), "//foo:top1", "//foo:top2");
+    assertThat(result.hasError()).isTrue();
     assertContainsEvent("in sh_library rule //foo:rec1: cycle in dependency graph:\n");
     assertContainsEvent("in sh_library rule //foo:top");
   }
@@ -470,7 +478,8 @@
     scratch.file("y/BUILD",
         "cc_library(name='y', srcs=['y.cc'], deps=['//x:z'])");
 
-    update(defaultFlags().with(Flag.KEEP_GOING), "//y:y");
+    AnalysisResult result = update(defaultFlags().with(Flag.KEEP_GOING), "//y:y");
+    assertThat(result.hasError()).isTrue();
     assertContainsEvent("no such target '//x:z': "
         + "target 'z' not declared in package 'x' "
         + "defined by /workspace/x/BUILD and referenced by '//y:y'");
@@ -708,7 +717,8 @@
     EventBus eventBus = new EventBus();
     LoadingFailureRecorder recorder = new LoadingFailureRecorder();
     eventBus.register(recorder);
-    update(eventBus, defaultFlags().with(Flag.KEEP_GOING), "//gp");
+    AnalysisResult result = update(eventBus, defaultFlags().with(Flag.KEEP_GOING), "//gp");
+    assertThat(result.hasError()).isTrue();
     assertThat(recorder.events).hasSize(2);
     assertTrue(recorder.events.toString(), recorder.events.contains(
         Pair.of(Label.parseAbsolute("//gp"), Label.parseAbsolute("//cycles1"))));
@@ -816,8 +826,12 @@
         "config_setting(name = 'a', values = {'test_arg': 'a'})",
         "cc_library(name='x', srcs=select({':a': ['a.cc'], '//conditions:default': ['foo.cc']}))",
         "cc_binary(name='_objs/x/conflict/foo.pic.o', srcs=['bar.cc'])");
-    update(defaultFlags().with(Flag.KEEP_GOING), "//conflict:_objs/x/conflict/foo.pic.o",
+    AnalysisResult result = update(
+        defaultFlags().with(Flag.KEEP_GOING),
+        "//conflict:_objs/x/conflict/foo.pic.o",
         "//conflict:x");
+    // TODO(ulfjack): We print an error message but don't return an error, apparently.
+    //assertThat(result.hasError()).isTrue();
     // Expect to reach this line without a Precondition-triggered NullPointerException.
     assertContainsEvent(
         "file 'conflict/_objs/x/conflict/foo.pic.o' is generated by these conflicting actions");