Skip to content

Commit

Permalink
Dynamic config test improvements:
Browse files Browse the repository at this point in the history
- Adds a helper routine to get all configured targets with a given label
- Enhances latebound split test to check that deps actually take expected configurations

--
MOS_MIGRATED_REVID=129906477
  • Loading branch information
gregestren authored and hermione521 committed Aug 11, 2016
1 parent f3ec597 commit 247ac16
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 13 deletions.
1 change: 1 addition & 0 deletions src/test/java/com/google/devtools/build/lib/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,7 @@ java_test(
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/rules/cpp",
"//src/main/java/com/google/devtools/build/skyframe",
"//src/test/java/com/google/devtools/build/lib/skyframe:testutil",
"//third_party:auto_value",
"//third_party:guava",
"//third_party:guava-testlib",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import com.google.devtools.build.lib.pkgcache.LoadingFailedException;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternKey;
import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils;
import com.google.devtools.build.lib.testutil.Suite;
import com.google.devtools.build.lib.testutil.TestSpec;
import com.google.devtools.build.lib.testutil.TestUtils;
Expand Down Expand Up @@ -1265,12 +1266,18 @@ public void lateBoundSplitAttributeConfigs() throws Exception {
scratch.file("foo/BUILD",
"rule_with_latebound_split(",
" name = 'foo')",
"sh_binary(",
" name = 'latebound_dep',",
" srcs = ['latebound_dep.sh'])");
"rule_with_test_fragment(",
" name = 'latebound_dep')");
update("//foo:foo");
assertNotNull(getConfiguredTarget("//foo:foo"));
// TODO(bazel-team): also check that the dep is created in each expected configuration.
Iterable<ConfiguredTarget> deps = SkyframeExecutorTestUtils.getExistingConfiguredTargets(
skyframeExecutor, Label.parseAbsolute("//foo:latebound_dep"));
assertThat(deps).hasSize(2);
assertThat(
ImmutableList.of(
LateBoundSplitUtil.getOptions(Iterables.get(deps, 0).getConfiguration()).fooFlag,
LateBoundSplitUtil.getOptions(Iterables.get(deps, 1).getConfiguration()).fooFlag))
.containsExactly("one", "two");
}

/** Runs the same test with the reduced loading phase. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public class LateBoundSplitUtil {
/**
* A custom {@link FragmentOptions} with the option to be split.
*/
public static class Options extends FragmentOptions { // public for options loader
public static class TestOptions extends FragmentOptions { // public for options loader
@Option(
name = "foo",
defaultValue = "",
Expand All @@ -66,9 +66,9 @@ public List<Attribute.SplitTransition<BuildOptions>> getPotentialSplitTransition
@Override
public List<BuildOptions> split(BuildOptions buildOptions) {
BuildOptions split1 = buildOptions.clone();
split1.get(Options.class).fooFlag = "one";
split1.get(TestOptions.class).fooFlag = "one";
BuildOptions split2 = buildOptions.clone();
split2.get(Options.class).fooFlag = "two";
split2.get(TestOptions.class).fooFlag = "two";
return ImmutableList.<BuildOptions>of(split1, split2);
}

Expand All @@ -81,7 +81,7 @@ public boolean defaultsToSelf() {
/**
* The {@link BuildConfiguration.Fragment} that contains the options.
*/
private static class Fragment extends BuildConfiguration.Fragment {
private static class TestFragment extends BuildConfiguration.Fragment {
}

/**
Expand All @@ -92,17 +92,17 @@ static class FragmentLoader implements ConfigurationFragmentFactory {
public BuildConfiguration.Fragment create(ConfigurationEnvironment env,
BuildOptions buildOptions)
throws InvalidConfigurationException {
return new Fragment();
return new TestFragment();
}

@Override
public Class<? extends BuildConfiguration.Fragment> creates() {
return Fragment.class;
return TestFragment.class;
}

@Override
public ImmutableSet<Class<? extends FragmentOptions>> requiredOptions() {
return ImmutableSet.<Class<? extends FragmentOptions>>of(Options.class);
return ImmutableSet.<Class<? extends FragmentOptions>>of(TestOptions.class);
}
}

Expand All @@ -129,7 +129,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment envi
.allowedRuleClasses(Attribute.ANY_RULE)
.cfg(SIMPLE_SPLIT)
.value(SIMPLE_LATEBOUND_RESOLVER))
.requiresConfigurationFragments(Fragment.class)
.requiresConfigurationFragments(TestFragment.class)
.build();
}

Expand All @@ -143,15 +143,44 @@ public Metadata getMetadata() {
}
}

/**
* A custom rule that requires {@link TestFragment}.
*/
static class RuleWithTestFragment implements RuleDefinition {
@Override
public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment environment) {
return builder
.requiresConfigurationFragments(TestFragment.class)
.build();
}

@Override
public Metadata getMetadata() {
return RuleDefinition.Metadata.builder()
.name("rule_with_test_fragment")
.ancestors(BaseRuleClasses.RuleBase.class)
.factoryClass(UnknownRuleConfiguredTarget.class)
.build();
}
}

/**
* Returns a rule class provider with standard test setup plus the above rules/configs.
*/
static ConfiguredRuleClassProvider getRuleClassProvider() {
ConfiguredRuleClassProvider.Builder builder = new ConfiguredRuleClassProvider.Builder();
TestRuleClassProvider.addStandardRules(builder);
builder.addRuleDefinition(new RuleWithLateBoundSplitAttribute());
builder.addRuleDefinition(new RuleWithTestFragment());
builder.addConfigurationFragment(new FragmentLoader());
builder.addConfigurationOptions(Options.class);
builder.addConfigurationOptions(TestOptions.class);
return builder.build();
}

/**
* Returns the {@link TestOptions} from the given configuration.
*/
static TestOptions getOptions(BuildConfiguration config) {
return config.getOptions().get(TestOptions.class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe.util;

import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
Expand Down Expand Up @@ -104,6 +106,23 @@ public static ConfiguredTarget getExistingConfiguredTarget(
return value.getConfiguredTarget();
}

/**
* Returns all configured targets currently in the graph with the given label.
*
* <p>Unlike {@link #getExistingConfiguredTarget(SkyframeExecutor, Label, BuildConfiguration)},
* this doesn't make the caller request a specific configuration.
*/
public static Iterable<ConfiguredTarget> getExistingConfiguredTargets(
SkyframeExecutor skyframeExecutor, final Label label) {
return Iterables.filter(getAllExistingConfiguredTargets(skyframeExecutor),
new Predicate<ConfiguredTarget>() {
@Override
public boolean apply(ConfiguredTarget input) {
return input.getTarget().getLabel().equals(label);
}
});
}

/**
* Returns all configured targets currently in the graph.
*/
Expand Down

0 comments on commit 247ac16

Please sign in to comment.