From de62b4e9cd0f56473278811774ec191a4e0fafcd Mon Sep 17 00:00:00 2001 From: Lukacs Berki Date: Fri, 26 Feb 2016 13:13:39 +0000 Subject: Pass the source path -> Artifact mapping into FdoSupport from a provider instead of special-casing it. This removes the need to deserialize artifacts in FdoSupport, which in turn removes the need to support artifact deserialization at all, which makes our lives simpler and Thoreauvian programming is good. -- MOS_MIGRATED_REVID=115660698 --- .../build/lib/actions/ArtifactFactoryTest.java | 33 ---------------------- .../build/lib/pkgcache/LoadingPhaseRunnerTest.java | 15 ---------- .../lib/skyframe/SkyframeLabelVisitorTestCase.java | 22 ++------------- 3 files changed, 2 insertions(+), 68 deletions(-) (limited to 'src/test') diff --git a/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java b/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java index 1979df4e2f..2e491b17e0 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java @@ -251,37 +251,4 @@ public class ArtifactFactoryTest { return result; } } - - @Test - public void testArtifactDeserializationWithoutReusedArtifacts() throws Exception { - PathFragment derivedPath = outRoot.getExecPath().getRelative("fruit/banana"); - artifactFactory.clear(); - artifactFactory.setDerivedArtifactRoots(ImmutableList.of(outRoot)); - MockPackageRootResolver rootResolver = new MockPackageRootResolver(); - rootResolver.setPackageRoots( - ImmutableMap.of(PackageIdentifier.createInDefaultRepo(""), clientRoot)); - Artifact artifact1 = artifactFactory.deserializeArtifact(derivedPath, rootResolver); - Artifact artifact2 = artifactFactory.deserializeArtifact(derivedPath, rootResolver); - assertEquals(artifact1, artifact2); - assertNull(artifact1.getOwner()); - assertNull(artifact2.getOwner()); - assertEquals(derivedPath, artifact1.getExecPath()); - assertEquals(derivedPath, artifact2.getExecPath()); - - // Source artifacts are always reused - PathFragment sourcePath = clientRoot.getExecPath().getRelative("fruit/mango"); - artifact1 = artifactFactory.deserializeArtifact(sourcePath, rootResolver); - artifact2 = artifactFactory.deserializeArtifact(sourcePath, rootResolver); - assertSame(artifact1, artifact2); - assertEquals(sourcePath, artifact1.getExecPath()); - } - - @Test - public void testDeserializationWithInvalidPath() throws Exception { - artifactFactory.clear(); - PathFragment randomPath = new PathFragment("maracuja/lemon/kiwi"); - Artifact artifact = artifactFactory.deserializeArtifact(randomPath, - new MockPackageRootResolver()); - assertNull(artifact); - } } diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java index 77b9ad81ec..5e9dcd41f8 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java @@ -26,7 +26,6 @@ import com.google.common.base.Predicate; import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableListMultimap; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.eventbus.EventBus; @@ -116,12 +115,6 @@ public class LoadingPhaseRunnerTest { LoadingResult loadingResult = assertNoErrors(tester.load("//base:hello")); assertThat(loadingResult.getTargets()).containsExactlyElementsIn(getTargets("//base:hello")); assertNull(loadingResult.getTestsToRun()); - // TODO(ulfjack): We don't collect package roots if we don't run the loading phase. - if (runsLoadingPhase()) { - assertEquals( - ImmutableMap.of(PackageIdentifier.createInDefaultRepo("base"), tester.getWorkspace()), - loadingResult.getPackageRoots()); - } } @Test @@ -150,7 +143,6 @@ public class LoadingPhaseRunnerTest { assertThat(loadingResult.hasLoadingError()).isFalse(); assertThat(loadingResult.getTargets()).isEmpty(); assertThat(loadingResult.getTestsToRun()).isNull(); - assertThat(loadingResult.getPackageRoots()).isEmpty(); tester.assertContainsError("Skipping '//base:missing': no such package 'base'"); tester.assertContainsWarning("Target pattern parsing failed."); } @@ -163,7 +155,6 @@ public class LoadingPhaseRunnerTest { assertThat(loadingResult.hasLoadingError()).isFalse(); assertThat(loadingResult.getTargets()).isEmpty(); assertThat(loadingResult.getTestsToRun()).isNull(); - assertThat(loadingResult.getPackageRoots()).isEmpty(); tester.assertContainsError("Skipping '//base:missing': no such target '//base:missing'"); tester.assertContainsWarning("Target pattern parsing failed."); } @@ -193,7 +184,6 @@ public class LoadingPhaseRunnerTest { assertTrue(loadingResult.hasLoadingError()); assertThat(loadingResult.getTargets()).containsExactlyElementsIn(ImmutableList.of()); assertNull(loadingResult.getTestsToRun()); - assertTrue(loadingResult.getPackageRoots().size() <= 1); tester.assertContainsError("no such package 'nonexistent': BUILD file not found"); } @@ -321,10 +311,6 @@ public class LoadingPhaseRunnerTest { LoadingResult loadingResult = assertNoErrors(tester.loadTests("//cc:tests")); assertThat(loadingResult.getTargets()).containsExactlyElementsIn(getTargets("//cc:my_test")); assertThat(loadingResult.getTestsToRun()).containsExactlyElementsIn(getTargets("//cc:my_test")); - if (runsLoadingPhase()) { - assertThat(loadingResult.getPackageRoots().entrySet()) - .contains(entryFor(PackageIdentifier.createInDefaultRepo("cc"), tester.getWorkspace())); - } assertThat(tester.getOriginalTargets()) .containsExactlyElementsIn(getTargets("//cc:tests", "//cc:my_test")); assertThat(tester.getTestSuiteTargets()) @@ -450,7 +436,6 @@ public class LoadingPhaseRunnerTest { LoadingResult secondResult = assertNoErrors(tester.load("//base:hello")); assertEquals(firstResult.getTargets(), secondResult.getTargets()); assertEquals(firstResult.getTestsToRun(), secondResult.getTestsToRun()); - assertEquals(firstResult.getPackageRoots(), secondResult.getPackageRoots()); } /** diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitorTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitorTestCase.java index 5771f522d9..13951591c1 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitorTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitorTestCase.java @@ -33,7 +33,6 @@ import com.google.devtools.build.lib.events.EventCollector; import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.packages.ConstantRuleVisibility; import com.google.devtools.build.lib.packages.NoSuchThingException; -import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.util.PackageLoadingTestCase; @@ -121,8 +120,7 @@ abstract public class SkyframeLabelVisitorTestCase extends PackageLoadingTestCas * Check that the expected targets were exactly those visited, and that the packages of these * expected targets were exactly those packages visited. */ - protected void assertExpectedTargets( - Set expectedLabels, boolean expectError, Set startingTargets) + protected void assertExpectedTargets(Set expectedLabels, Set startingTargets) throws Exception { Set