From d8d35939f7b02793255520c215adf4069806cd3b Mon Sep 17 00:00:00 2001 From: Googler Date: Fri, 15 Sep 2023 13:57:04 -0700 Subject: [PATCH] Re-arrange PlatformInfo and clean up some code. Work towards platform-based flags: #19409. PiperOrigin-RevId: 565770775 Change-Id: Ibb75e66f6a73aad565ade28e5cda99ebaa280121 --- .../lib/analysis/platform/PlatformInfo.java | 73 +++--- .../analysis/platform/PlatformInfoTest.java | 246 +++++++++--------- .../rules/platform/PlatformInfoApiTest.java | 114 ++++---- 3 files changed, 216 insertions(+), 217 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformInfo.java b/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformInfo.java index a813de56a542b2..a607b2fa5836a8 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformInfo.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformInfo.java @@ -53,10 +53,22 @@ public class PlatformInfo extends NativeInfo /** Name used in Starlark for accessing this provider. */ public static final String STARLARK_NAME = "PlatformInfo"; + /** Empty {@link PlatformInfo} instance representing an invalid or empty platform. */ + public static final PlatformInfo EMPTY_PLATFORM_INFO; + + static { + try { + EMPTY_PLATFORM_INFO = PlatformInfo.builder().build(); + } catch (DuplicateConstraintException | ExecPropertiesException e) { + // This can never happen since we're not passing any values to the builder. + throw new VerifyException(e); + } + } + /** Provider singleton constant. */ public static final BuiltinProvider PROVIDER = new Provider(); - /** Provider for {@link ToolchainInfo} objects. */ + /** Provider for {@link PlatformInfo} objects. */ private static class Provider extends BuiltinProvider implements PlatformInfoApi.Provider< ConstraintSettingInfo, ConstraintValueInfo, PlatformInfo> { @@ -101,7 +113,10 @@ public PlatformInfo platformInfo( private final Label label; private final ConstraintCollection constraints; private final String remoteExecutionProperties; + /** execProperties will deprecate and replace remoteExecutionProperties */ + // TODO(blaze-configurability): If we want to remove remoteExecutionProperties, we need to fix + // PlatformUtils.getPlatformProto to use the dict-typed execProperties and do a migration. private final ImmutableMap execProperties; private PlatformInfo( @@ -117,18 +132,6 @@ private PlatformInfo( this.execProperties = execProperties; } - /** Empty {@link PlatformInfo} instance representing an invalid or empty platform. */ - public static final PlatformInfo EMPTY_PLATFORM_INFO; - - static { - try { - EMPTY_PLATFORM_INFO = PlatformInfo.builder().build(); - } catch (DuplicateConstraintException | ExecPropertiesException e) { - // This can never happen since we're not passing any values to the builder. - throw new VerifyException(e); - } - } - @Override public BuiltinProvider getProvider() { return PROVIDER; @@ -159,11 +162,6 @@ public void repr(Printer printer) { printer.append(String.format("PlatformInfo(%s, constraints=%s)", label, constraints)); } - /** Returns a new {@link Builder} for creating a fresh {@link PlatformInfo} instance. */ - public static Builder builder() { - return new Builder(); - } - /** Add this platform to the given fingerprint. */ public void addTo(Fingerprint fp) { fp.addString(label.toString()); @@ -172,6 +170,28 @@ public void addTo(Fingerprint fp) { constraints.addToFingerprint(fp); } + @Override + public boolean equals(Object o) { + if (!(o instanceof PlatformInfo)) { + return false; + } + PlatformInfo that = (PlatformInfo) o; + return Objects.equals(label, that.label) + && Objects.equals(constraints, that.constraints) + && Objects.equals(remoteExecutionProperties, that.remoteExecutionProperties) + && Objects.equals(execProperties, that.execProperties); + } + + @Override + public int hashCode() { + return Objects.hash(label, constraints, remoteExecutionProperties, execProperties); + } + + /** Returns a new {@link Builder} for creating a fresh {@link PlatformInfo} instance. */ + public static Builder builder() { + return new Builder(); + } + /** Builder class to facilitate creating valid {@link PlatformInfo} instances. */ public static class Builder { @@ -380,23 +400,6 @@ private static ImmutableMap mergeExecProperties( } } - @Override - public boolean equals(Object o) { - if (!(o instanceof PlatformInfo)) { - return false; - } - PlatformInfo that = (PlatformInfo) o; - return Objects.equals(label, that.label) - && Objects.equals(constraints, that.constraints) - && Objects.equals(remoteExecutionProperties, that.remoteExecutionProperties) - && Objects.equals(execProperties, that.execProperties); - } - - @Override - public int hashCode() { - return Objects.hash(label, constraints, remoteExecutionProperties); - } - /** Exception that indicates something is wrong in exec_properties configuration. */ public static class ExecPropertiesException extends Exception { ExecPropertiesException(String message) { diff --git a/src/test/java/com/google/devtools/build/lib/analysis/platform/PlatformInfoTest.java b/src/test/java/com/google/devtools/build/lib/analysis/platform/PlatformInfoTest.java index 29d3d40634fe3f..cc6bd0f0c4a9b3 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/platform/PlatformInfoTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/platform/PlatformInfoTest.java @@ -31,7 +31,7 @@ public class PlatformInfoTest extends BuildViewTestCase { @Test - public void platformInfo() throws Exception { + public void builder() throws Exception { ConstraintSettingInfo setting1 = ConstraintSettingInfo.create(Label.parseCanonicalUnchecked("//constraint:s1")); ConstraintSettingInfo setting2 = @@ -54,17 +54,7 @@ public void platformInfo() throws Exception { } @Test - public void platformInfo_remoteExecutionProperties() throws Exception { - PlatformInfo.Builder builder = PlatformInfo.builder(); - builder.setRemoteExecutionProperties("properties"); - PlatformInfo platformInfo = builder.build(); - - assertThat(platformInfo).isNotNull(); - assertThat(platformInfo.remoteExecutionProperties()).isEqualTo("properties"); - } - - @Test - public void platformInfo_parentPlatform_noOverlaps() throws Exception { + public void constraints_parentPlatform_noOverlaps() throws Exception { ConstraintSettingInfo setting1 = ConstraintSettingInfo.create(Label.parseCanonicalUnchecked("//constraint:s1")); ConstraintSettingInfo setting2 = @@ -100,7 +90,7 @@ public void platformInfo_parentPlatform_noOverlaps() throws Exception { } @Test - public void platformInfo_parentPlatform_overlaps() throws Exception { + public void constraints_parentPlatform_overlaps() throws Exception { ConstraintSettingInfo setting1 = ConstraintSettingInfo.create(Label.parseCanonicalUnchecked("//constraint:s1")); ConstraintSettingInfo setting2 = @@ -135,7 +125,56 @@ public void platformInfo_parentPlatform_overlaps() throws Exception { } @Test - public void platformInfo_parentPlatform_keepRemoteExecutionProperties() throws Exception { + public void constraints_overlappingError() throws Exception { + ConstraintSettingInfo setting1 = + ConstraintSettingInfo.create(Label.parseCanonicalUnchecked("//constraint:basic")); + ConstraintSettingInfo setting2 = + ConstraintSettingInfo.create(Label.parseCanonicalUnchecked("//constraint:complex")); + ConstraintSettingInfo setting3 = + ConstraintSettingInfo.create(Label.parseCanonicalUnchecked("//constraint:single")); + + PlatformInfo.Builder builder = PlatformInfo.builder(); + + builder.addConstraint( + ConstraintValueInfo.create(setting1, Label.parseCanonicalUnchecked("//constraint:value1"))); + builder.addConstraint( + ConstraintValueInfo.create(setting1, Label.parseCanonicalUnchecked("//constraint:value2"))); + + builder.addConstraint( + ConstraintValueInfo.create(setting2, Label.parseCanonicalUnchecked("//constraint:value3"))); + builder.addConstraint( + ConstraintValueInfo.create(setting2, Label.parseCanonicalUnchecked("//constraint:value4"))); + builder.addConstraint( + ConstraintValueInfo.create(setting2, Label.parseCanonicalUnchecked("//constraint:value5"))); + + builder.addConstraint( + ConstraintValueInfo.create(setting3, Label.parseCanonicalUnchecked("//constraint:value6"))); + + ConstraintCollection.DuplicateConstraintException exception = + assertThrows( + ConstraintCollection.DuplicateConstraintException.class, () -> builder.build()); + assertThat(exception) + .hasMessageThat() + .contains( + "Duplicate constraint values detected: " + + "constraint_setting //constraint:basic has " + + "[//constraint:value1, //constraint:value2], " + + "constraint_setting //constraint:complex has " + + "[//constraint:value3, //constraint:value4, //constraint:value5]"); + } + + @Test + public void remoteExecutionProperties() throws Exception { + PlatformInfo.Builder builder = PlatformInfo.builder(); + builder.setRemoteExecutionProperties("properties"); + PlatformInfo platformInfo = builder.build(); + + assertThat(platformInfo).isNotNull(); + assertThat(platformInfo.remoteExecutionProperties()).isEqualTo("properties"); + } + + @Test + public void remoteExecutionProperties_parentPlatform_keep() throws Exception { PlatformInfo parent = PlatformInfo.builder().setRemoteExecutionProperties("parent properties").build(); @@ -148,7 +187,7 @@ public void platformInfo_parentPlatform_keepRemoteExecutionProperties() throws E } @Test - public void platformInfo_parentPlatform_overrideRemoteExecutionProperties() throws Exception { + public void remoteExecutionProperties_parentPlatform_override() throws Exception { PlatformInfo parent = PlatformInfo.builder().setRemoteExecutionProperties("parent properties").build(); @@ -163,7 +202,7 @@ public void platformInfo_parentPlatform_overrideRemoteExecutionProperties() thro } @Test - public void platformInfo_parentPlatform_mergeRemoteExecutionProperties() throws Exception { + public void remoteExecutionProperties_parentPlatform_merge() throws Exception { PlatformInfo parent = PlatformInfo.builder().setRemoteExecutionProperties("parent properties").build(); @@ -178,8 +217,7 @@ public void platformInfo_parentPlatform_mergeRemoteExecutionProperties() throws } @Test - public void platformInfo_parentPlatform_mergeRemoteExecutionProperties_noParent() - throws Exception { + public void remoteExecutionProperties_parentPlatform_merge_noParent() throws Exception { PlatformInfo.Builder builder = PlatformInfo.builder(); builder.setRemoteExecutionProperties("child {PARENT_REMOTE_EXECUTION_PROPERTIES} properties"); PlatformInfo platformInfo = builder.build(); @@ -189,8 +227,7 @@ public void platformInfo_parentPlatform_mergeRemoteExecutionProperties_noParent( } @Test - public void platformInfo_parentPlatform_mergeRemoteExecutionProperties_parentNotSet() - throws Exception { + public void remoteExecutionProperties_parentPlatform_merge_parentNotSet() throws Exception { PlatformInfo parent = PlatformInfo.builder().build(); PlatformInfo.Builder builder = PlatformInfo.builder(); @@ -203,104 +240,7 @@ public void platformInfo_parentPlatform_mergeRemoteExecutionProperties_parentNot } @Test - public void platformInfo_overlappingConstraintsError() throws Exception { - ConstraintSettingInfo setting1 = - ConstraintSettingInfo.create(Label.parseCanonicalUnchecked("//constraint:basic")); - ConstraintSettingInfo setting2 = - ConstraintSettingInfo.create(Label.parseCanonicalUnchecked("//constraint:complex")); - ConstraintSettingInfo setting3 = - ConstraintSettingInfo.create(Label.parseCanonicalUnchecked("//constraint:single")); - - PlatformInfo.Builder builder = PlatformInfo.builder(); - - builder.addConstraint( - ConstraintValueInfo.create(setting1, Label.parseCanonicalUnchecked("//constraint:value1"))); - builder.addConstraint( - ConstraintValueInfo.create(setting1, Label.parseCanonicalUnchecked("//constraint:value2"))); - - builder.addConstraint( - ConstraintValueInfo.create(setting2, Label.parseCanonicalUnchecked("//constraint:value3"))); - builder.addConstraint( - ConstraintValueInfo.create(setting2, Label.parseCanonicalUnchecked("//constraint:value4"))); - builder.addConstraint( - ConstraintValueInfo.create(setting2, Label.parseCanonicalUnchecked("//constraint:value5"))); - - builder.addConstraint( - ConstraintValueInfo.create(setting3, Label.parseCanonicalUnchecked("//constraint:value6"))); - - ConstraintCollection.DuplicateConstraintException exception = - assertThrows( - ConstraintCollection.DuplicateConstraintException.class, () -> builder.build()); - assertThat(exception) - .hasMessageThat() - .contains( - "Duplicate constraint values detected: " - + "constraint_setting //constraint:basic has " - + "[//constraint:value1, //constraint:value2], " - + "constraint_setting //constraint:complex has " - + "[//constraint:value3, //constraint:value4, //constraint:value5]"); - } - - @Test - public void platformInfo_equalsTester() throws Exception { - ConstraintSettingInfo setting1 = - ConstraintSettingInfo.create(Label.parseCanonicalUnchecked("//constraint:basic")); - ConstraintSettingInfo setting2 = - ConstraintSettingInfo.create(Label.parseCanonicalUnchecked("//constraint:other")); - - ConstraintValueInfo value1 = - ConstraintValueInfo.create(setting1, Label.parseCanonicalUnchecked("//constraint:value1")); - ConstraintValueInfo value2 = - ConstraintValueInfo.create(setting2, Label.parseCanonicalUnchecked("//constraint:value2")); - ConstraintValueInfo value3 = - ConstraintValueInfo.create(setting2, Label.parseCanonicalUnchecked("//constraint:value3")); - - new EqualsTester() - .addEqualityGroup( - // Base case. - PlatformInfo.builder() - .setLabel(Label.parseCanonicalUnchecked("//platform/plat1")) - .addConstraint(value1) - .addConstraint(value2) - .build(), - PlatformInfo.builder() - .setLabel(Label.parseCanonicalUnchecked("//platform/plat1")) - .addConstraint(value1) - .addConstraint(value2) - .build()) - .addEqualityGroup( - // Different label. - PlatformInfo.builder() - .setLabel(Label.parseCanonicalUnchecked("//platform/plat2")) - .addConstraint(value1) - .addConstraint(value2) - .build()) - .addEqualityGroup( - // Extra constraint. - PlatformInfo.builder() - .setLabel(Label.parseCanonicalUnchecked("//platform/plat1")) - .addConstraint(value1) - .addConstraint(value3) - .build()) - .addEqualityGroup( - // Missing constraint. - PlatformInfo.builder() - .setLabel(Label.parseCanonicalUnchecked("//platform/plat1")) - .addConstraint(value1) - .build()) - .addEqualityGroup( - // Different remote exec properties. - PlatformInfo.builder() - .setLabel(Label.parseCanonicalUnchecked("//platform/plat1")) - .addConstraint(value1) - .addConstraint(value2) - .setRemoteExecutionProperties("foo") - .build()) - .testEquals(); - } - - @Test - public void platformInfo_execProperties_empty() throws Exception { + public void execProperties_empty() throws Exception { PlatformInfo.Builder builder = PlatformInfo.builder(); builder.setExecProperties(ImmutableMap.of()); PlatformInfo platformInfo = builder.build(); @@ -311,7 +251,7 @@ public void platformInfo_execProperties_empty() throws Exception { } @Test - public void platformInfo_execProperties_one() throws Exception { + public void execProperties_one() throws Exception { PlatformInfo.Builder builder = PlatformInfo.builder(); builder.setExecProperties(ImmutableMap.of("elem1", "value1")); PlatformInfo platformInfo = builder.build(); @@ -322,7 +262,7 @@ public void platformInfo_execProperties_one() throws Exception { } @Test - public void platformInfo_parentPlatform_keepExecProperties() throws Exception { + public void execProperties_parentPlatform_keep() throws Exception { PlatformInfo parent = PlatformInfo.builder().setExecProperties(ImmutableMap.of("parent", "properties")).build(); @@ -335,7 +275,7 @@ public void platformInfo_parentPlatform_keepExecProperties() throws Exception { } @Test - public void platformInfo_parentPlatform_inheritanceRules() throws Exception { + public void execProperties_parentPlatform_inheritance() throws Exception { PlatformInfo parent = PlatformInfo.builder() .setExecProperties( @@ -352,7 +292,7 @@ public void platformInfo_parentPlatform_inheritanceRules() throws Exception { } @Test - public void platformInfo_constructor() throws Exception { + public void starlark_constructor() throws Exception { scratch.file( "test/platform/my_platform.bzl", "def _impl(ctx):", @@ -398,7 +338,7 @@ public void platformInfo_constructor() throws Exception { } @Test - public void platformInfo_constructor_parent() throws Exception { + public void starlark_constructor_parent() throws Exception { scratch.file( "test/platform/my_platform.bzl", "def _impl(ctx):", @@ -468,7 +408,7 @@ public void platformInfo_constructor_parent() throws Exception { } @Test - public void platformInfo_constructor_error_duplicateConstraints() throws Exception { + public void starlark_constructor_error_duplicateConstraints() throws Exception { scratch.file( "test/platform/my_platform.bzl", "def _impl(ctx):", @@ -500,4 +440,62 @@ public void platformInfo_constructor_error_duplicateConstraints() throws Excepti " ],", ")"); } + + @Test + public void equalsTester() throws Exception { + ConstraintSettingInfo setting1 = + ConstraintSettingInfo.create(Label.parseCanonicalUnchecked("//constraint:basic")); + ConstraintSettingInfo setting2 = + ConstraintSettingInfo.create(Label.parseCanonicalUnchecked("//constraint:other")); + + ConstraintValueInfo value1 = + ConstraintValueInfo.create(setting1, Label.parseCanonicalUnchecked("//constraint:value1")); + ConstraintValueInfo value2 = + ConstraintValueInfo.create(setting2, Label.parseCanonicalUnchecked("//constraint:value2")); + ConstraintValueInfo value3 = + ConstraintValueInfo.create(setting2, Label.parseCanonicalUnchecked("//constraint:value3")); + + new EqualsTester() + .addEqualityGroup( + // Base case. + PlatformInfo.builder() + .setLabel(Label.parseCanonicalUnchecked("//platform/plat1")) + .addConstraint(value1) + .addConstraint(value2) + .build(), + PlatformInfo.builder() + .setLabel(Label.parseCanonicalUnchecked("//platform/plat1")) + .addConstraint(value1) + .addConstraint(value2) + .build()) + .addEqualityGroup( + // Different label. + PlatformInfo.builder() + .setLabel(Label.parseCanonicalUnchecked("//platform/plat2")) + .addConstraint(value1) + .addConstraint(value2) + .build()) + .addEqualityGroup( + // Extra constraint. + PlatformInfo.builder() + .setLabel(Label.parseCanonicalUnchecked("//platform/plat1")) + .addConstraint(value1) + .addConstraint(value3) + .build()) + .addEqualityGroup( + // Missing constraint. + PlatformInfo.builder() + .setLabel(Label.parseCanonicalUnchecked("//platform/plat1")) + .addConstraint(value1) + .build()) + .addEqualityGroup( + // Different remote exec properties. + PlatformInfo.builder() + .setLabel(Label.parseCanonicalUnchecked("//platform/plat1")) + .addConstraint(value1) + .addConstraint(value2) + .setRemoteExecutionProperties("foo") + .build()) + .testEquals(); + } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/platform/PlatformInfoApiTest.java b/src/test/java/com/google/devtools/build/lib/rules/platform/PlatformInfoApiTest.java index 60a5fa07365d71..f560546bfbc60a 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/platform/PlatformInfoApiTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/platform/PlatformInfoApiTest.java @@ -32,7 +32,7 @@ public class PlatformInfoApiTest extends PlatformTestCase { @Test - public void testPlatform() throws Exception { + public void constructor() throws Exception { constraintBuilder("//foo:basic").addConstraintValue("value1").write(); platformBuilder("//foo:my_platform").addConstraint("value1").write(); assertNoEvents(); @@ -49,8 +49,31 @@ public void testPlatform() throws Exception { } @Test - public void testPlatform_overlappingConstraintValueError() throws Exception { + public void tooManyParentsError() throws Exception { List lines = + new ImmutableList.Builder() + .addAll(platformBuilder("//foo:parent_platform1").lines()) + .addAll(platformBuilder("//foo:parent_platform2").lines()) + .addAll( + ImmutableList.of( + "platform(name = 'my_platform',\n", + " parents = [\n", + " ':parent_platform1',\n", + " ':parent_platform2',\n", + " ])")) + .build(); + + checkError( + "foo", + "my_platform", + "in parents attribute of platform rule //foo:my_platform: " + + "parents attribute must have a single value", + lines.toArray(new String[] {})); + } + + @Test + public void constraints_overlappingError() throws Exception { + ImmutableList lines = new ImmutableList.Builder() .addAll( constraintBuilder("//foo:basic") @@ -73,17 +96,7 @@ public void testPlatform_overlappingConstraintValueError() throws Exception { } @Test - public void testPlatform_remoteExecution() throws Exception { - platformBuilder("//foo:my_platform").setRemoteExecutionProperties("foo: val1").write(); - assertNoEvents(); - - PlatformInfo platformInfo = fetchPlatformInfo("//foo:my_platform"); - assertThat(platformInfo).isNotNull(); - assertThat(platformInfo.remoteExecutionProperties()).isEqualTo("foo: val1"); - } - - @Test - public void testPlatform_parent() throws Exception { + public void constraints_parent() throws Exception { constraintBuilder("//foo:setting1").addConstraintValue("value1").write(); constraintBuilder("//foo:setting2").addConstraintValue("value2").write(); platformBuilder("//foo:parent_platform").addConstraint("value1").write(); @@ -110,7 +123,7 @@ public void testPlatform_parent() throws Exception { } @Test - public void testPlatform_parent_override() throws Exception { + public void constraints_parent_override() throws Exception { constraintBuilder("//foo:setting1") .addConstraintValue("value1a") .addConstraintValue("value1b") @@ -130,7 +143,17 @@ public void testPlatform_parent_override() throws Exception { } @Test - public void testPlatform_parent_remoteExecProperties_entireOverride() throws Exception { + public void remoteExecutionProperties() throws Exception { + platformBuilder("//foo:my_platform").setRemoteExecutionProperties("foo: val1").write(); + assertNoEvents(); + + PlatformInfo platformInfo = fetchPlatformInfo("//foo:my_platform"); + assertThat(platformInfo).isNotNull(); + assertThat(platformInfo.remoteExecutionProperties()).isEqualTo("foo: val1"); + } + + @Test + public void remoteExecutionProperties_parent_entireOverride() throws Exception { platformBuilder("//foo:parent_platform").setRemoteExecutionProperties("parent props").write(); platformBuilder("//foo:my_platform") .setParent("//foo:parent_platform") @@ -144,7 +167,7 @@ public void testPlatform_parent_remoteExecProperties_entireOverride() throws Exc } @Test - public void testPlatform_parent_remoteExecProperties_includeParent() throws Exception { + public void remoteExecutionProperties_parent_merge() throws Exception { platformBuilder("//foo:parent_platform").setRemoteExecutionProperties("parent props").write(); platformBuilder("//foo:my_platform") .setParent("//foo:parent_platform") @@ -158,30 +181,25 @@ public void testPlatform_parent_remoteExecProperties_includeParent() throws Exce } @Test - public void testPlatform_parent_tooManyParentsError() throws Exception { - List lines = - new ImmutableList.Builder() - .addAll(platformBuilder("//foo:parent_platform1").lines()) - .addAll(platformBuilder("//foo:parent_platform2").lines()) - .addAll( - ImmutableList.of( - "platform(name = 'my_platform',\n", - " parents = [\n", - " ':parent_platform1',\n", - " ':parent_platform2',\n", - " ])")) - .build(); + public void remoteExecutionProperties_parentSpecifiesExecProperties_error() throws Exception { + ImmutableMap propsChild = ImmutableMap.of("k2", "child_v2", "k3", "child_v3"); + platformBuilder("//foo:parent_platform").setRemoteExecutionProperties("properties").write(); + PlatformBuilder builder = + platformBuilder("//bar:my_platform") + .setParent("//foo:parent_platform") + .setExecProperties(propsChild); checkError( - "foo", + "bar", "my_platform", - "in parents attribute of platform rule //foo:my_platform: " - + "parents attribute must have a single value", - lines.toArray(new String[] {})); + "Platform specifies exec_properties but its parent //foo:parent_platform specifies" + + " remote_execution_properties. Prefer exec_properties over the deprecated" + + " remote_execution_properties.", + builder.lines().toArray(new String[] {})); } @Test - public void testPlatform_execProperties() throws Exception { + public void execProperties() throws Exception { ImmutableMap props = ImmutableMap.of("k1", "v1", "k2", "v2"); platformBuilder("//foo:my_platform").setExecProperties(props).write(); assertNoEvents(); @@ -192,7 +210,7 @@ public void testPlatform_execProperties() throws Exception { } @Test - public void testPlatform_conflictingProperties_errorsOut() throws Exception { + public void execProperties_conflictingProperties_error() throws Exception { ImmutableMap props = ImmutableMap.of("k1", "v1", "k2", "v2"); PlatformBuilder builder = platformBuilder("//foo:my_platform") @@ -208,7 +226,7 @@ public void testPlatform_conflictingProperties_errorsOut() throws Exception { } @Test - public void testPlatform_execProperties_parent() throws Exception { + public void execProperties_parent() throws Exception { ImmutableMap props = ImmutableMap.of("k1", "v1", "k2", "v2"); platformBuilder("//foo:parent_platform").setExecProperties(props).write(); platformBuilder("//foo:my_platform").setParent("//foo:parent_platform").write(); @@ -220,7 +238,7 @@ public void testPlatform_execProperties_parent() throws Exception { } @Test - public void testPlatform_execProperties_parent_mixed() throws Exception { + public void execProperties_parent_merged() throws Exception { ImmutableMap propsParent = ImmutableMap.of("k1", "v1", "k2", "v2"); ImmutableMap propsChild = ImmutableMap.of("k2", "child_v2", "k3", "child_v3"); platformBuilder("//foo:parent_platform").setExecProperties(propsParent).write(); @@ -238,8 +256,7 @@ public void testPlatform_execProperties_parent_mixed() throws Exception { } @Test - public void testPlatform_execProperties_parentSpecifiesRemoteExecutionProperties_errorsOut() - throws Exception { + public void execProperties_parentSpecifiesRemoteExecutionProperties_error() throws Exception { ImmutableMap propsParent = ImmutableMap.of("k1", "v1", "k2", "v2"); platformBuilder("//foo:parent_platform").setExecProperties(propsParent).write(); PlatformBuilder builder = @@ -254,23 +271,4 @@ public void testPlatform_execProperties_parentSpecifiesRemoteExecutionProperties + " Prefer exec_properties over the deprecated remote_execution_properties.", builder.lines().toArray(new String[] {})); } - - @Test - public void testPlatform_remoteExecutionProperties_parentSpecifiesExecProperties_errorsOut() - throws Exception { - ImmutableMap propsChild = ImmutableMap.of("k2", "child_v2", "k3", "child_v3"); - platformBuilder("//foo:parent_platform").setRemoteExecutionProperties("properties").write(); - PlatformBuilder builder = - platformBuilder("//bar:my_platform") - .setParent("//foo:parent_platform") - .setExecProperties(propsChild); - - checkError( - "bar", - "my_platform", - "Platform specifies exec_properties but its parent //foo:parent_platform specifies" - + " remote_execution_properties. Prefer exec_properties over the deprecated" - + " remote_execution_properties.", - builder.lines().toArray(new String[] {})); - } }