Skip to content

Commit

Permalink
Ease the configuration-related restrictions of several more tests.
Browse files Browse the repository at this point in the history
Use update() and other methods which properly respect the trimming
transition over those which don't. Avoid using the target configuration
if the configuration actually used by a target is available.

RELNOTES: None.
PiperOrigin-RevId: 196588405
  • Loading branch information
mstaib authored and Copybara-Service committed May 14, 2018
1 parent a6ee1ee commit eb5a80c
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1182,7 +1182,8 @@ public ConfiguredTarget getConfiguredTargetForTesting(
@VisibleForTesting
public ConfiguredTargetAndData getConfiguredTargetAndDataForTesting(
ExtendedEventHandler eventHandler, Label label, BuildConfiguration config) {
return skyframeExecutor.getConfiguredTargetAndDataForTesting(eventHandler, label, config);
return skyframeExecutor.getConfiguredTargetAndDataForTesting(
eventHandler, label, config, getTopLevelTransitionForTarget(label, config, eventHandler));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1812,8 +1812,9 @@ public ConfiguredTarget getConfiguredTargetForTesting(
return configuredTargetAndData == null ? null : configuredTargetAndData.getConfiguredTarget();
}

@VisibleForTesting
@Nullable
private ConfiguredTargetAndData getConfiguredTargetAndDataForTesting(
public ConfiguredTargetAndData getConfiguredTargetAndDataForTesting(
ExtendedEventHandler eventHandler,
Label label,
BuildConfiguration configuration,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,11 +302,14 @@ public void testActionConflictMarksTargetInvalid() throws Exception {
"cc_library(name='x', srcs=['foo.cc'])",
"cc_binary(name='_objs/x/conflict/foo.o', srcs=['bar.cc'])");
reporter.removeHandler(failFastHandler); // expect errors
update(defaultFlags().with(Flag.KEEP_GOING),
"//conflict:x", "//conflict:_objs/x/conflict/foo.pic.o");
ConfiguredTarget a = getConfiguredTarget("//conflict:x");
ConfiguredTarget b = getConfiguredTarget("//conflict:_objs/x/conflict/foo.pic.o");
assertThat(hasTopLevelAnalysisError(a) ^ hasTopLevelAnalysisError(b)).isTrue();
int successfulAnalyses =
update(
defaultFlags().with(Flag.KEEP_GOING),
"//conflict:x",
"//conflict:_objs/x/conflict/foo.pic.o")
.getTargetsToBuild()
.size();
assertThat(successfulAnalyses).isEqualTo(1);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
import com.google.devtools.build.lib.testutil.Suite;
import com.google.devtools.build.lib.testutil.TestConstants;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
import com.google.devtools.build.lib.testutil.TestSpec;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.Path;
Expand Down Expand Up @@ -387,12 +388,20 @@ public Label apply(ConfiguredTarget target) {

@Test
public void testGetDirectPrerequisiteDependencies() throws Exception {
// Override the trimming transition to not distort the results.
ConfiguredRuleClassProvider.Builder builder =
new ConfiguredRuleClassProvider.Builder();
TestRuleClassProvider.addStandardRules(builder);
builder.overrideTrimmingTransitionFactoryForTesting((rule) -> NoTransition.INSTANCE);
useRuleClassProvider(builder.build());

update();

scratch.file(
"package/BUILD",
"filegroup(name='top', srcs=[':inner', 'file'])",
"sh_binary(name='inner', srcs=['script.sh'])");
update("//package:top");
ConfiguredTarget top = getConfiguredTarget("//package:top", getTargetConfiguration());
ConfiguredTarget top = Iterables.getOnlyElement(update("//package:top").getTargetsToBuild());
Iterable<Dependency> targets = getView().getDirectPrerequisiteDependenciesForTesting(
reporter, top, getBuildConfigurationCollection(), /*toolchainContext=*/ null).values();

Expand Down Expand Up @@ -617,9 +626,8 @@ public void testNewActionsAreDifferentAndDontConflict() throws Exception {
"genrule(name='a', ",
" cmd='',",
" outs=['a.out'])");
update("//pkg:a.out");
OutputFileConfiguredTarget outputCT = (OutputFileConfiguredTarget)
getConfiguredTarget("//pkg:a.out");
Iterables.getOnlyElement(update("//pkg:a.out").getTargetsToBuild());
Artifact outputArtifact = outputCT.getArtifact();
Action action = getGeneratingAction(outputArtifact);
assertThat(action).isNotNull();
Expand Down Expand Up @@ -656,8 +664,7 @@ public void testMultiBuildInvalidationRevalidation() throws Exception {
" srcs = glob(['A*.java']))",
"java_test(name = 'B',",
" srcs = ['B.java'])");
update("//java/a:A");
ConfiguredTarget ct = getConfiguredTarget("//java/a:A");
ConfiguredTarget ct = Iterables.getOnlyElement(update("//java/a:A").getTargetsToBuild());
scratch.deleteFile("java/a/C.java");
update("//java/a:B");
update("//java/a:A");
Expand Down Expand Up @@ -772,10 +779,10 @@ public void testActionsNotRegisteredInLegacyWhenError() throws Exception {
// Then update the BUILD file and re-analyze.
scratch.file("actions_not_registered/BUILD",
"cc_binary(name = 'foo', srcs = ['foo.cc'])");
update("//actions_not_registered:foo");
Artifact fooOut = Iterables.getOnlyElement(
getConfiguredTarget("//actions_not_registered:foo")
.getProvider(FileProvider.class).getFilesToBuild());
ConfiguredTarget foo =
Iterables.getOnlyElement(update("//actions_not_registered:foo").getTargetsToBuild());
Artifact fooOut =
Iterables.getOnlyElement(foo.getProvider(FileProvider.class).getFilesToBuild());
assertThat(getActionGraph().getGeneratingAction(fooOut)).isNotNull();
clearAnalysisResult();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@ public void lateBoundSplitAttributeInTargetConfiguration() throws Exception {
" name = 'foo')",
"rule_with_test_fragment(",
" name = 'latebound_dep')");
update("//foo:foo");
assertThat(getConfiguredTarget("//foo:foo")).isNotNull();
// if the target fails to analyze, this iterable will be empty
assertThat(update("//foo:foo").getTargetsToBuild()).isNotEmpty();
Iterable<ConfiguredTarget> deps = SkyframeExecutorTestUtils.getExistingConfiguredTargets(
skyframeExecutor, Label.parseAbsolute("//foo:latebound_dep"));
assertThat(deps).hasSize(2);
Expand Down

0 comments on commit eb5a80c

Please sign in to comment.