From cbc55814b13e6372ca10ae058211ab27a71d0d9e Mon Sep 17 00:00:00 2001 From: dacek Date: Mon, 25 Oct 2021 14:21:22 -0700 Subject: [PATCH] Add KeepGoingTest. PiperOrigin-RevId: 405489830 --- .../google/devtools/build/lib/buildtool/BUILD | 19 + .../build/lib/buildtool/KeepGoingTest.java | 349 ++++++++++++++++++ 2 files changed, 368 insertions(+) create mode 100644 src/test/java/com/google/devtools/build/lib/buildtool/KeepGoingTest.java diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/BUILD b/src/test/java/com/google/devtools/build/lib/buildtool/BUILD index 1656a4bafc0955..367e442eaa9bb5 100644 --- a/src/test/java/com/google/devtools/build/lib/buildtool/BUILD +++ b/src/test/java/com/google/devtools/build/lib/buildtool/BUILD @@ -310,6 +310,25 @@ java_test( ], ) +java_test( + name = "KeepGoingTest", + srcs = ["KeepGoingTest.java"], + tags = [ + "no_windows", + ], + deps = [ + "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/actions:artifacts", + "//src/main/java/com/google/devtools/build/lib/events", + "//src/test/java/com/google/devtools/build/lib/analysis/util", + "//src/test/java/com/google/devtools/build/lib/buildtool/util", + "//src/test/java/com/google/devtools/build/lib/packages:testutil", + "//third_party:guava", + "//third_party:junit4", + "//third_party:truth", + ], +) + java_test( name = "LabelCrossesPackageBoundaryTest", srcs = ["LabelCrossesPackageBoundaryTest.java"], diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/KeepGoingTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/KeepGoingTest.java new file mode 100644 index 00000000000000..3b5ddbd27a355f --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/buildtool/KeepGoingTest.java @@ -0,0 +1,349 @@ +// Copyright 2021 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.buildtool; + +// For debugging, uncomment these and the call to setupLogging() below. +// +// import com.google.devtools.build.lib.blaze.BlazeRuntime; +// import java.util.logging.Level; + +import static com.google.common.collect.Iterables.getOnlyElement; +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.fail; + +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.BuildFailedException; +import com.google.devtools.build.lib.analysis.util.AnalysisMock; +import com.google.devtools.build.lib.buildtool.util.BuildIntegrationTestCase; +import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.packages.util.MockGenruleSupport; +import java.io.IOException; +import java.util.Iterator; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** A test of the semantics of the keepGoing flag: continue as much as possible after an error. */ +@RunWith(JUnit4.class) +public class KeepGoingTest extends BuildIntegrationTestCase { + + @Before + public void stageEmbeddedTools() throws Exception { + AnalysisMock.get().setupMockToolsRepository(mockToolsConfig); + MockGenruleSupport.setup(mockToolsConfig); + } + + @Before + public final void addOptions() { + addOptions("--keep_going"); + } + + private static String genrule(String name, String deps, int succeeds) { + String out = name + ".out"; + String cmd = (succeeds != 0) ? "cp $(location in) $(location " + out + ")" : "exit 42"; + return "genrule(name='" + + name + + "', " + + " srcs=['in'," + + deps + + "], " + + " outs=['" + + out + + "'], " + + " cmd='" + + cmd + + "')\n"; + } + + private static final int A = 0x01; + private static final int B = 0x02; + private static final int C = 0x04; + private static final int D = 0x08; + private static final int E = 0x10; + + private static final String[] labels = { + "//keepgoing:A", "//keepgoing:B", "//keepgoing:C", "//keepgoing:D", "//keepgoing:E" + }; + + // "mask" is a bitmask of rules that succeed. + private void writeFiles(int mask) throws IOException { + // A --> B --> C + // | +---> D + // | + // +---> E + write( + "keepgoing/BUILD", + genrule("A", "'B','E'", mask & A) + + genrule("B", "'C','D'", mask & B) + + genrule("C", "", mask & C) + + genrule("D", "", mask & D) + + genrule("E", "", mask & E)); + + write("keepgoing/in", "(input)"); + } + + // "mask" is a bitmask of rules that succeed. + private void assertBuilt(int mask) throws Exception { + for (int ii = 0; ii < labels.length; ii++) { + assertOneBuilt(labels[ii], (mask & (1 << ii)) != 0); + } + } + + private void assertOneBuilt(String label, boolean shouldBeBuilt) throws Exception { + Iterable files = getArtifacts(label); + for (Artifact file : files) { + boolean isActuallyBuilt = file.getPath().exists(); + if (file.getPath().exists() != shouldBeBuilt) { + fail( + file.prettyPrint() + + ": shouldBeBuilt=" + + shouldBeBuilt + + ", isActuallyBuilt=" + + isActuallyBuilt); + } + } + } + + private void assertNoMoreEvents(Iterator events) { + boolean ok = true; + while (events.hasNext()) { + System.err.println(events.next()); + ok = false; + } + assertThat(ok).isTrue(); + } + + // Build //keepgoing:A, expecting failure. (The BuildResult instance is + // subsequently available via getRequest() for later assertions.) + private void buildA() throws Exception { + BuildFailedException e = + assertThrows(BuildFailedException.class, () -> buildTarget("//keepgoing:A")); + assertThat(e).hasMessageThat().isNull(); + } + + /******************************************************************** + * * + * Actual tests... * + * * + ********************************************************************/ + + @Test + public void testKeepGoingAfterCFails() throws Exception { + // C fails due to error (logged). + // B fails due to failed prereqs (logged). + // A fails due to failed prereqs (logged). + // D and E are built. + // Then a BuildFailedException is thrown. + writeFiles(A | B | D | E); + buildA(); + + Iterator errors = events.errors().iterator(); + assertThat(errors.next().getMessage()) + .containsMatch("Executing genrule //keepgoing:C failed: .*Exit 42.*"); + assertNoMoreEvents(errors); + + assertBuilt(D | E); + } + + @Test + public void testKeepGoingAfterDFails() throws Exception { + // D fails due to error (logged). + // B fails due to failed prereqs (logged). + // A fails due to failed prereqs (logged). + // C and E are built. + // Then a BuildFailedException is thrown. + writeFiles(A | B | C | E); + buildA(); + + Iterator errors = events.errors().iterator(); + assertThat(errors.next().getMessage()) + .containsMatch("Executing genrule //keepgoing:D failed: .*Exit 42.*"); + assertNoMoreEvents(errors); + + assertBuilt(C | E); + } + + @Test + public void testKeepGoingAfterCAndDFail() throws Exception { + // C and D fail due to error (logged). + // B fails due to failed prereqs (logged). + // A fails due to failed prereqs (logged). + // E is built. + // Then a BuildFailedException is thrown. + writeFiles(A | B | E); + buildA(); + + // C, D events are unordered: + Iterator errors = events.errors().iterator(); + assertThat(errors.next().getMessage()) + .containsMatch("Executing genrule //keepgoing:(C|D) failed: .*Exit 42.*"); + assertThat(errors.next().getMessage()) + .containsMatch("Executing genrule //keepgoing:(C|D) failed: .*Exit 42.*"); + + assertNoMoreEvents(errors); + + assertBuilt(E); + } + + @Test + public void testKeepGoingAfterEFails() throws Exception { + // E fails due to error (logged). + // A fails due to failed prereqs (logged). + // B,C,D are built. + // Then a BuildFailedException is thrown. + writeFiles(A | B | C | D); + buildA(); + + Iterator errors = events.errors().iterator(); + assertThat(errors.next().getMessage()) + .containsMatch("Executing genrule //keepgoing:E failed: .*Exit 42.*"); + assertNoMoreEvents(errors); + + assertBuilt(B | C | D); + } + + // Regression test for b/8826301, incremental builder does not correctly set root actions. + // Check that keep going works on second build. Note that this test failed non-deterministically + // in b/8826301, because it depended on HashSet iteration order. + @Test + public void testKeepGoingOnSecondBuild() throws Exception { + StringBuilder buildFile = new StringBuilder(); + buildFile + .append("genrule(name='topgen', tools=[':badgen'], outs=['top.out'], ") + .append("cmd='touch $@')\n") + .append("genrule(name='badgen', executable=1, srcs=['badsrc.sh'], ") + .append("outs=['bad.out'], cmd='bash $< > $@', tools = ["); + // Make graph large so incremental dependency checker does graph culling, if enabled. + for (int i = 0; i < 60; i++) { + buildFile.append("':gen" + i + "', "); + } + buildFile.append("])\n"); + for (int i = 0; i < 60; i++) { + buildFile + .append("genrule(name='gen") + .append(i) + .append("', " + "outs=['gen") + .append(i) + .append(".out'], executable=1, cmd = 'echo \"#!/bin/true\" > $@')\n"); + } + write("keepgoing/BUILD", buildFile.toString()); + write("keepgoing/badsrc.sh", "exit 0"); + buildTarget("//keepgoing:topgen"); + write("keepgoing/badsrc.sh", "exit 42"); + events.clear(); + BuildFailedException e = + assertThrows(BuildFailedException.class, () -> buildTarget("//keepgoing:topgen")); + assertThat(e).hasMessageThat().isNull(); + Iterator errors = events.errors().iterator(); + assertThat(errors.next().getMessage()) + .containsMatch("Executing genrule //keepgoing:badgen.* failed: .*Exit 42.*"); + assertNoMoreEvents(errors); + } + + @Test + public void testConfigurationErrorsAreToleratedWithKeepGoing() throws Exception { + write("a/BUILD", "cc_library(name='a', srcs=['missing.foo'])"); + write("b/BUILD", "cc_library(name='b')"); + + /** + * Regression coverage for bug 1191396: "blaze build -k exits zero if execution succeeds, even + * if there were analysis errors". + */ + assertBuildFailedExceptionFromBuilding( + "command succeeded, but not all targets were analyzed", "//a", "//b"); + events.assertContainsError( + "in srcs attribute of cc_library rule //a:a: " + "target '//a:missing.foo' does not exist"); + events.assertContainsInfo("Analysis succeeded for only 1 of 2 top-level targets"); + + assertSameConfiguredTarget("//b:b"); + } + + @Test + public void testKeepGoingAfterLoadingPhaseErrors() throws Exception { + write("a/BUILD", "cc_library(name='a')"); + write("b/BUILD", "cc_library(name='b', deps = ['//missing:lib'])"); + + assertBuildFailedExceptionFromBuilding( + "command succeeded, but not all targets were analyzed", "//a", "//b"); // + events.assertContainsError("no such package 'missing': BUILD file not found in any of the"); + + assertSameConfiguredTarget("//a:a"); + events.assertContainsInfo(" succeeded for only 1 of "); + } + + @Test + public void testKeepGoingAfterTargetParsingErrors() throws Exception { + write("a/BUILD", "cc_library(name='a', xyz)"); + write("b/BUILD", "cc_library(name='b', xyz)"); + write("b/b1/BUILD", "cc_library(name='b1')"); + write("b/b2/BUILD", "cc_library(name='b2', xyz)"); + + assertBuildFailedExceptionFromBuilding( + "command succeeded, but there were errors parsing the target pattern", "b/...", "//a"); + events.assertContainsWarning("Target pattern parsing failed."); + + assertSameConfiguredTarget("//b/b1"); + } + + @Test + public void testKeepGoingAfterSchedulingDependencyMiddlemanFailure() throws Exception { + write("foo/foo.cc", "int main() { return 0; }"); + write( + "foo/BUILD", + "cc_binary(name='foo', srcs=['foo.cc', 'gen.h'], malloc='system_malloc')", + "cc_library(name='system_malloc', linkstatic=1)", + "genrule(name='gen', srcs=[], outs=['gen.h'], cmd = 'exit 1')"); + + assertThrows(BuildFailedException.class, () -> buildTarget("//foo:foo")); + } + + private void assertSameConfiguredTarget(String label) throws Exception { + assertThat(getOnlyElement(getResult().getSuccessfulTargets())) + .isSameInstanceAs(getConfiguredTarget(label)); + } + + @Test + public void testKeepGoingAfterAnalysisFailure() throws Exception { + write( + "analysiserror/failer.bzl", + "def _failer_impl(ctx):", + " fail('BOOM!')", + "", + "failer = rule(implementation = _failer_impl)"); + write( + "analysiserror/BUILD", + "load(':failer.bzl', 'failer')", + "genrule(name='gen', srcs=[], outs=['gen.h'], cmd='exit 1')", + // The next line has an analysis failure: the xmb_lint rule is devoid of xmb files. + "failer(name='foo')", + "cc_library(name='bar')"); + + assertBuildFailedExceptionFromBuilding( + "command succeeded, but not all targets were analyzed", + "//analysiserror:foo", + "//analysiserror:bar"); + events.assertContainsError("Error in fail: BOOM!"); + + assertSameConfiguredTarget("//analysiserror:bar"); + } + + private void assertBuildFailedExceptionFromBuilding(String msg, String... targets) { + BuildFailedException e = assertThrows(BuildFailedException.class, () -> buildTarget(targets)); + assertThat(e).hasMessageThat().isEqualTo(msg); + assertThat(getResult().getSuccess()).isFalse(); + } +}