From af0b670d9906e96ae41f55bb3b9b8698d425a1dc Mon Sep 17 00:00:00 2001 From: Ulf Adams Date: Wed, 18 Jan 2017 10:58:10 +0000 Subject: With interleaving now enabled, clean up our tests. Remove many calls from tests to SkyframeLabelVisitor.sync, which is generally no longer necessary. In particular, BuildViewTestCase.ensureTarget(s)Visited was calling it, so removing that, as well as simulateLoadingPhase. I moved some tests for TransitiveTargetFunction to the corresponding test class, where they belong. I also made sure that we have test coverage in the BuildViewTest for some cases that were previously only tested with TTF. I dropped some tests which no longer seem applicable / for which we have coverage elsewhere. Also, as a drive-by change, I fixed some warnings in BuildViewTestCase. Note that TTF is only used for genquery and in an experimental code path, but generally superseded by ConfiguredTargetFunction, which has taken over the responsibility for loading in all other cases (and interleaves loading and analysis). Some of the tests are not externally visible yet. -- PiperOrigin-RevId: 144813237 MOS_MIGRATED_REVID=144813237 --- .../devtools/build/lib/analysis/BuildViewTest.java | 39 ++++++++ .../build/lib/analysis/util/BuildViewTestCase.java | 107 ++++----------------- .../build/lib/pkgcache/IOExceptionsTest.java | 48 ++++----- .../build/lib/pkgcache/PackageCacheTest.java | 38 -------- .../devtools/build/lib/rules/cpp/CcCommonTest.java | 10 +- .../build/lib/skylark/SkylarkIntegrationTest.java | 30 +++--- 6 files changed, 95 insertions(+), 177 deletions(-) (limited to 'src/test/java/com/google/devtools') 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 63e7484e6d..d7be1ffaf4 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 @@ -166,6 +166,30 @@ public class BuildViewTest extends BuildViewTestBase { assertSame(FailAction.class, action.getClass()); } + @Test + public void testSyntaxErrorInDepPackage() throws Exception { + // Check that a loading error in a dependency is properly reported. + scratch.file("a/BUILD", + "genrule(name='x',", + " srcs = ['file.txt'],", + " outs = ['foo'],", + " cmd = 'echo')", + "@"); // syntax error + + scratch.file("b/BUILD", + "genrule(name= 'cc',", + " tools = ['//a:x'],", + " outs = ['bar'],", + " cmd = 'echo')"); + + reporter.removeHandler(failFastHandler); + EventBus eventBus = new EventBus(); + AnalysisResult result = update(eventBus, defaultFlags().with(Flag.KEEP_GOING), "//b:cc"); + + assertContainsEvent("invalid character: '@'"); + assertThat(result.hasError()).isTrue(); + } + @Test public void testReportsAnalysisRootCauses() throws Exception { scratch.file("private/BUILD", @@ -1284,6 +1308,21 @@ public class BuildViewTest extends BuildViewTestBase { assertThat(owners).containsExactly("//x:b", "//x:b"); } + @Test + public void testErrorMessageForMissingPackageGroup() throws Exception { + scratch.file( + "apple/BUILD", + "py_library(name='apple', visibility=['//non:existent'])"); + reporter.removeHandler(failFastHandler); + try { + update("//apple"); + fail(); + } catch (ViewCreationFailedException e) { + // Expected. + } + assertDoesNotContainEvent("implicitly depends upon"); + } + /** Runs the same test with the reduced loading phase. */ @TestSpec(size = Suite.SMALL_TESTS) @RunWith(JUnit4.class) diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java index f4c57c2cfd..476f90dd67 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java @@ -304,7 +304,6 @@ public abstract class BuildViewTestCase extends FoundationTestCase { optionsPolicyEnforcer.enforce(optionsParser); BuildOptions buildOptions = ruleClassProvider.createBuildOptions(optionsParser); - ensureTargetsVisited(buildOptions.getAllLabels().values()); skyframeExecutor.invalidateConfigurationCollection(); return skyframeExecutor.createConfigurations(reporter, configurationFactory, buildOptions, ImmutableSet.of(), false); @@ -441,7 +440,6 @@ public abstract class BuildViewTestCase extends FoundationTestCase { view.setArtifactRoots( ImmutableMap.of(PackageIdentifier.createInMainRepo(""), rootDirectory)); - simulateLoadingPhase(); } protected CachingAnalysisEnvironment getTestAnalysisEnvironment() { @@ -646,27 +644,10 @@ public abstract class BuildViewTestCase extends FoundationTestCase { Iterables.find(getFilesToBuild(target), artifactNamed(outputName))); } - protected void simulateLoadingPhase() { - try { - ensureTargetsVisited(targetConfig.getAllLabels().values()); - } catch (Exception e) { - throw new RuntimeException(e); - } - } - protected ActionsTestUtil actionsTestUtil() { return new ActionsTestUtil(getActionGraph()); } - private Set getTargets(Iterable