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");