From af0b670d9906e96ae41f55bb3b9b8698d425a1dc Mon Sep 17 00:00:00 2001 From: Ulf Adams Date: Wed, 18 Jan 2017 10:58:10 +0000 Subject: [PATCH] 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 --- .../build/lib/analysis/BuildViewTest.java | 39 +++++++ .../lib/analysis/util/BuildViewTestCase.java | 107 ++++-------------- .../build/lib/pkgcache/IOExceptionsTest.java | 48 ++++---- .../build/lib/pkgcache/PackageCacheTest.java | 38 ------- .../build/lib/rules/cpp/CcCommonTest.java | 10 +- .../lib/skylark/SkylarkIntegrationTest.java | 30 ++--- 6 files changed, 95 insertions(+), 177 deletions(-) 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 63e7484e6d03d8..d7be1ffaf465f5 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 void testGeneratedArtifact() throws Exception { 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 void ruleExtraActionsDontHideAspectExtraActions() throws Exception { 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 f4c57c2cfdde54..476f90dd6740b1 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 @@ protected final BuildConfigurationCollection createConfigurations(String... args 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 @@ protected final void createBuildView() throws Exception { view.setArtifactRoots( ImmutableMap.of(PackageIdentifier.createInMainRepo(""), rootDirectory)); - simulateLoadingPhase(); } protected CachingAnalysisEnvironment getTestAnalysisEnvironment() { @@ -646,27 +644,10 @@ protected SpawnAction getGeneratingSpawnAction(ConfiguredTarget target, String o 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