bazel skylark: remove cruft from tests
This change removes all the test "helpers" from the util/ subpackage
that had a RuleContext parameter. Now, the tests each explicitly
call setRuleContext, which is simpler and makes clear that this
is (and always was) a side effect.
Also:
- remove many other test helpers
- remove unsound casts
- move a test of nested set to a more appropriate place
- fold SkylarkFileHelperTest (two tests) into SRIFT.java
If you think this is tedious to review, I can assure you it was more tedious to write.
PiperOrigin-RevId: 274021984
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
index f490bf6..43bdbc7 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
@@ -57,7 +57,6 @@
import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
import com.google.devtools.build.lib.syntax.StarlarkFile;
-import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.syntax.StarlarkThread;
import com.google.devtools.build.lib.syntax.SyntaxError;
import com.google.devtools.build.lib.testutil.MoreAsserts;
@@ -150,7 +149,7 @@
String[] strings = lines.clone();
strings[strings.length - 1] = String.format("%s = %s", name, strings[strings.length - 1]);
evalAndExport(strings);
- Descriptor lookup = (Descriptor) ev.lookup(name);
+ Descriptor lookup = (Descriptor) lookup(name);
return lookup != null ? lookup.build(name) : null;
}
@@ -192,9 +191,8 @@
@Test
public void testAttrAllowedFileTypesWrongType() throws Exception {
- checkErrorContains(
- "allow_files should be a boolean or a string list",
- "attr.label_list(allow_files = 18)");
+ checkEvalErrorContains(
+ "allow_files should be a boolean or a string list", "attr.label_list(allow_files = 18)");
}
@Test
@@ -213,13 +211,7 @@
@Test
public void testDisableDeprecatedParams() throws Exception {
- ev =
- createEvaluationTestCase(
- StarlarkSemantics.DEFAULT_SEMANTICS
- .toBuilder()
- .incompatibleDisableDeprecatedAttrParams(true)
- .build());
- ev.initialize();
+ setSkylarkSemanticsOptions("--incompatible_disable_deprecated_attr_params=true");
// Verify 'single_file' deprecation.
EvalException expected =
@@ -240,7 +232,7 @@
@Test
public void testAttrAllowedSingleFileTypesWrongType() throws Exception {
- checkErrorContains(
+ checkEvalErrorContains(
"allow_single_file should be a boolean or a string list",
"attr.label(allow_single_file = 18)");
}
@@ -345,8 +337,8 @@
" pass",
"my_aspect = aspect(implementation = _impl)",
"a = attr.label_list(aspects = [my_aspect])");
- SkylarkAttr.Descriptor attr = (SkylarkAttr.Descriptor) ev.lookup("a");
- SkylarkDefinedAspect aspect = (SkylarkDefinedAspect) ev.lookup("my_aspect");
+ SkylarkAttr.Descriptor attr = (SkylarkAttr.Descriptor) lookup("a");
+ SkylarkDefinedAspect aspect = (SkylarkDefinedAspect) lookup("my_aspect");
assertThat(aspect).isNotNull();
assertThat(attr.build("xxx").getAspectClasses()).containsExactly(aspect.getAspectClass());
}
@@ -358,15 +350,15 @@
" pass",
"my_aspect = aspect(implementation = _impl)",
"a = attr.label(aspects = [my_aspect])");
- SkylarkAttr.Descriptor attr = (SkylarkAttr.Descriptor) ev.lookup("a");
- SkylarkDefinedAspect aspect = (SkylarkDefinedAspect) ev.lookup("my_aspect");
+ SkylarkAttr.Descriptor attr = (SkylarkAttr.Descriptor) lookup("a");
+ SkylarkDefinedAspect aspect = (SkylarkDefinedAspect) lookup("my_aspect");
assertThat(aspect).isNotNull();
assertThat(attr.build("xxx").getAspectClasses()).containsExactly(aspect.getAspectClass());
}
@Test
public void testLabelListWithAspectsError() throws Exception {
- checkErrorContains(
+ checkEvalErrorContains(
"expected type 'Aspect' for 'aspects' element but got type 'int' instead",
"def _impl(target, ctx):",
" pass",
@@ -382,7 +374,7 @@
"my_aspect = aspect(_impl,",
" attrs = { '_extra_deps' : attr.label(default = Label('//foo/bar:baz')) }",
")");
- SkylarkDefinedAspect aspect = (SkylarkDefinedAspect) ev.lookup("my_aspect");
+ SkylarkDefinedAspect aspect = (SkylarkDefinedAspect) lookup("my_aspect");
Attribute attribute = Iterables.getOnlyElement(aspect.getAttributes());
assertThat(attribute.getName()).isEqualTo("$extra_deps");
assertThat(attribute.getDefaultValue(null))
@@ -401,16 +393,16 @@
"my_aspect = aspect(_impl,",
" attrs = { 'param' : attr.string(values=['a', 'b']) }",
")");
- SkylarkDefinedAspect aspect = (SkylarkDefinedAspect) ev.lookup("my_aspect");
+ SkylarkDefinedAspect aspect = (SkylarkDefinedAspect) lookup("my_aspect");
Attribute attribute = Iterables.getOnlyElement(aspect.getAttributes());
assertThat(attribute.getName()).isEqualTo("param");
}
@Test
public void testAspectParameterRequiresValues() throws Exception {
- checkErrorContains(
+ checkEvalErrorContains(
"Aspect parameter attribute 'param' must have type 'string' and use the 'values' "
- + "restriction.",
+ + "restriction.",
"def _impl(target, ctx):",
" pass",
"my_aspect = aspect(_impl,",
@@ -420,9 +412,9 @@
@Test
public void testAspectParameterBadType() throws Exception {
- checkErrorContains(
+ checkEvalErrorContains(
"Aspect parameter attribute 'param' must have type 'string' and use the 'values' "
- + "restriction.",
+ + "restriction.",
"def _impl(target, ctx):",
" pass",
"my_aspect = aspect(_impl,",
@@ -439,14 +431,14 @@
" attrs = { 'param' : attr.string(values=['a', 'b']),",
" '_extra' : attr.label(default = Label('//foo/bar:baz')) }",
")");
- SkylarkDefinedAspect aspect = (SkylarkDefinedAspect) ev.lookup("my_aspect");
+ SkylarkDefinedAspect aspect = (SkylarkDefinedAspect) lookup("my_aspect");
assertThat(aspect.getAttributes()).hasSize(2);
assertThat(aspect.getParamAttributes()).containsExactly("param");
}
@Test
public void testAspectNoDefaultValueAttribute() throws Exception {
- checkErrorContains(
+ checkEvalErrorContains(
"Aspect attribute '_extra_deps' has no default value",
"def _impl(target, ctx):",
" pass",
@@ -466,7 +458,7 @@
@Test
public void testNonLabelAttrWithProviders() throws Exception {
- checkErrorContains(
+ checkEvalErrorContains(
"unexpected keyword 'providers', for call to method "
+ "string(default = '', doc = '', mandatory = False, values = []) "
+ "of 'attr (a language module)'",
@@ -538,24 +530,24 @@
@Test
public void testLabelAttrDefaultValueAsStringBadValue() throws Exception {
- checkErrorContains(
+ checkEvalErrorContains(
"invalid label '/foo:bar' in parameter 'default' of attribute 'label': "
+ "invalid target name '/foo:bar'",
"attr.label(default = '/foo:bar')");
- checkErrorContains(
+ checkEvalErrorContains(
"invalid label '/bar:foo' in element 1 of parameter 'default' of attribute "
+ "'label_list': invalid target name '/bar:foo'",
"attr.label_list(default = ['//foo:bar', '/bar:foo'])");
- checkErrorContains(
+ checkEvalErrorContains(
"invalid label '/bar:foo' in dict key element: invalid target name '/bar:foo'",
"attr.label_keyed_string_dict(default = {'//foo:bar': 'a', '/bar:foo': 'b'})");
}
@Test
public void testAttrDefaultValueBadType() throws Exception {
- checkErrorContains(
+ checkEvalErrorContains(
"expected value of type 'string' for parameter 'default', for call to method "
+ "string(default = '', doc = '', mandatory = False, values = []) "
+ "of 'attr (a language module)'",
@@ -571,12 +563,8 @@
@Test
public void testAttrNonEmpty() throws Exception {
- ev =
- createEvaluationTestCase(
- StarlarkSemantics.DEFAULT_SEMANTICS.toBuilder()
- .incompatibleDisableDeprecatedAttrParams(false)
- .build());
- ev.initialize();
+ setSkylarkSemanticsOptions("--incompatible_disable_deprecated_attr_params=false");
+ reset();
Attribute attr = buildAttribute("a1", "attr.string_list(non_empty=True)");
assertThat(attr.isNonEmpty()).isTrue();
@@ -592,7 +580,7 @@
@Test
public void testAttrBadKeywordArguments() throws Exception {
- checkErrorContains(
+ checkEvalErrorContains(
"unexpected keyword 'bad_keyword', for call to method "
+ "string(default = '', doc = '', mandatory = False, values = []) of "
+ "'attr (a language module)'",
@@ -663,7 +651,7 @@
@Test
public void testAttrDocValueBadType() throws Exception {
- checkErrorContains(
+ checkEvalErrorContains(
"expected value of type 'string' for parameter 'doc', for call to method "
+ "string(default = '', doc = '', mandatory = False, values = []) "
+ "of 'attr (a language module)'",
@@ -684,7 +672,7 @@
@Test
public void testLateBoundAttrWorksWithOnlyLabel() throws Exception {
- checkEvalError(
+ checkEvalErrorContains(
"expected value of type 'string' for parameter 'default', for call to method "
+ "string(default = '', doc = '', mandatory = False, values = []) "
+ "of 'attr (a language module)'",
@@ -780,14 +768,14 @@
@Test
public void testRuleUnknownKeyword() throws Exception {
registerDummyStarlarkFunction();
- checkErrorContains(
+ checkEvalErrorContains(
"unexpected keyword 'bad_keyword', for call to function rule(",
"rule(impl, bad_keyword = 'some text')");
}
@Test
public void testRuleImplementationMissing() throws Exception {
- checkErrorContains(
+ checkEvalErrorContains(
"parameter 'implementation' has no default value, for call to function rule(",
"rule(attrs = {})");
}
@@ -795,7 +783,7 @@
@Test
public void testRuleBadTypeForAdd() throws Exception {
registerDummyStarlarkFunction();
- checkErrorContains(
+ checkEvalErrorContains(
"expected value of type 'dict or NoneType' for parameter 'attrs', "
+ "for call to function rule(",
"rule(impl, attrs = 'some text')");
@@ -804,7 +792,7 @@
@Test
public void testRuleBadTypeInAdd() throws Exception {
registerDummyStarlarkFunction();
- checkErrorContains(
+ checkEvalErrorContains(
"expected <String, Descriptor> type for 'attrs' but got <string, string> instead",
"rule(impl, attrs = {'a1': 'some text'})");
}
@@ -812,32 +800,32 @@
@Test
public void testRuleBadTypeForDoc() throws Exception {
registerDummyStarlarkFunction();
- checkErrorContains(
+ checkEvalErrorContains(
"expected value of type 'string' for parameter 'doc', for call to function rule(",
"rule(impl, doc = 1)");
}
@Test
public void testLabel() throws Exception {
- Object result = evalRuleClassCode("Label('//foo/foo:foo')");
+ Object result = eval("Label('//foo/foo:foo')");
assertThat(result).isInstanceOf(Label.class);
assertThat(result.toString()).isEqualTo("//foo/foo:foo");
}
@Test
public void testLabelSameInstance() throws Exception {
- Object l1 = evalRuleClassCode("Label('//foo/foo:foo')");
+ Object l1 = eval("Label('//foo/foo:foo')");
// Implicitly creates a new pkgContext and environment, yet labels should be the same.
- Object l2 = evalRuleClassCode("Label('//foo/foo:foo')");
+ Object l2 = eval("Label('//foo/foo:foo')");
assertThat(l1).isSameInstanceAs(l2);
}
@Test
public void testLabelNameAndPackage() throws Exception {
- Object result = evalRuleClassCode("Label('//foo/bar:baz').name");
+ Object result = eval("Label('//foo/bar:baz').name");
assertThat(result).isEqualTo("baz");
// NB: implicitly creates a new pkgContext and environments, yet labels should be the same.
- result = evalRuleClassCode("Label('//foo/bar:baz').package");
+ result = eval("Label('//foo/bar:baz').package");
assertThat(result).isEqualTo("foo/bar");
}
@@ -876,7 +864,7 @@
private void checkTextMessage(String from, String... lines) throws Exception {
String[] strings = lines.clone();
- Object result = evalRuleClassCode(from);
+ Object result = eval(from);
String expect = "";
if (strings.length > 0) {
expect = Joiner.on("\n").join(lines) + "\n";
@@ -892,13 +880,11 @@
@Test
public void testStructRestrictedOverrides() throws Exception {
- checkErrorContains(
- "cannot override built-in struct function 'to_json'",
- "struct(to_json='foo')");
+ checkEvalErrorContains(
+ "cannot override built-in struct function 'to_json'", "struct(to_json='foo')");
- checkErrorContains(
- "cannot override built-in struct function 'to_proto'",
- "struct(to_proto='foo')");
+ checkEvalErrorContains(
+ "cannot override built-in struct function 'to_proto'", "struct(to_proto='foo')");
}
@Test
@@ -985,7 +971,7 @@
@Test
public void testTextMessageInvalidElementInListStructure() throws Exception {
- checkErrorContains(
+ checkEvalErrorContains(
"Invalid text format, expected a struct, a dict, a string, a bool, or "
+ "an int but got a list for list element in struct field 'a'",
"struct(a=[['b']]).to_proto()");
@@ -993,14 +979,14 @@
@Test
public void testTextMessageInvalidStructure() throws Exception {
- checkErrorContains(
+ checkEvalErrorContains(
"Invalid text format, expected a struct, a dict, a string, a bool, or an int "
+ "but got a function for struct field 'a'",
"struct(a=rule).to_proto()");
}
private void checkJson(String from, String expected) throws Exception {
- Object result = evalRuleClassCode(from);
+ Object result = eval(from);
assertThat(result).isEqualTo(expected);
}
@@ -1014,13 +1000,13 @@
public void testJsonDictFields() throws Exception {
checkJson("struct(config={}).to_json()", "{\"config\":{}}");
checkJson("struct(config={'key': 'value'}).to_json()", "{\"config\":{\"key\":\"value\"}}");
- checkErrorContains(
+ checkEvalErrorContains(
"Keys must be a string but got a int for struct field 'config'",
"struct(config={1:2}).to_json()");
- checkErrorContains(
+ checkEvalErrorContains(
"Keys must be a string but got a int for dict value 'foo'",
"struct(config={'foo':{1:2}}).to_json()");
- checkErrorContains(
+ checkEvalErrorContains(
"Keys must be a string but got a bool for struct field 'config'",
"struct(config={True: False}).to_json()");
}
@@ -1055,7 +1041,7 @@
@Test
public void testJsonInvalidStructure() throws Exception {
- checkErrorContains(
+ checkEvalErrorContains(
"Invalid text format, expected a struct, a string, a bool, or an int but got a "
+ "function for struct field 'a'",
"struct(a=rule).to_json()");
@@ -1063,7 +1049,7 @@
@Test
public void testLabelAttrWrongDefault() throws Exception {
- checkErrorContains(
+ checkEvalErrorContains(
"expected value of type 'Label or string or LateBoundDefault or function or NoneType' "
+ "for parameter 'default', for call to method label(",
"attr.label(default = 123)");
@@ -1077,7 +1063,7 @@
@Test
public void testLabelGetRelativeSyntaxError() throws Exception {
- checkErrorContains(
+ checkEvalErrorContains(
"invalid target name 'bad//syntax': target names may not contain '//' path separators",
"Label('//foo:bar').relative('bad//syntax')");
}
@@ -1120,10 +1106,10 @@
@Test
public void testStructIncomparability() throws Exception {
- checkErrorContains("Cannot compare structs", "struct(a = 1) < struct(a = 2)");
- checkErrorContains("Cannot compare structs", "struct(a = 1) > struct(a = 2)");
- checkErrorContains("Cannot compare structs", "struct(a = 1) <= struct(a = 2)");
- checkErrorContains("Cannot compare structs", "struct(a = 1) >= struct(a = 2)");
+ checkEvalErrorContains("Cannot compare structs", "struct(a = 1) < struct(a = 2)");
+ checkEvalErrorContains("Cannot compare structs", "struct(a = 1) > struct(a = 2)");
+ checkEvalErrorContains("Cannot compare structs", "struct(a = 1) <= struct(a = 2)");
+ checkEvalErrorContains("Cannot compare structs", "struct(a = 1) >= struct(a = 2)");
}
@Test
@@ -1135,21 +1121,22 @@
@Test
public void testStructAccessingUnknownField() throws Exception {
- checkErrorContains(
- "'struct' object has no attribute 'c'\n" + "Available attributes: a, b",
- "x = struct(a = 1, b = 2)",
- "y = x.c");
+ checkEvalErrorContains(
+ "'struct' object has no attribute 'c'\n" + "Available attributes: a, b",
+ "x = struct(a = 1, b = 2)",
+ "y = x.c");
}
@Test
public void testStructAccessingUnknownFieldWithArgs() throws Exception {
- checkErrorContains(
+ checkEvalErrorContains(
"type 'struct' has no method c()", "x = struct(a = 1, b = 2)", "y = x.c()");
}
@Test
public void testStructAccessingNonFunctionFieldWithArgs() throws Exception {
- checkErrorContains("'int' object is not callable", "x = struct(a = 1, b = 2)", "x1 = x.a(1)");
+ checkEvalErrorContains(
+ "'int' object is not callable", "x = struct(a = 1, b = 2)", "x1 = x.a(1)");
}
@Test
@@ -1160,7 +1147,7 @@
@Test
public void testStructPosArgs() throws Exception {
- checkErrorContains(
+ checkEvalErrorContains(
"expected no more than 0 positional arguments, but got 1", "x = struct(1, b = 2)");
}
@@ -1189,8 +1176,11 @@
@Test
public void testStructConcatenationCommonFields() throws Exception {
- checkErrorContains("Cannot use '+' operator on provider instances with overlapping field(s): a",
- "x = struct(a = 1, b = 2)", "y = struct(c = 1, a = 2)", "z = x + y\n");
+ checkEvalErrorContains(
+ "Cannot use '+' operator on provider instances with overlapping field(s): a",
+ "x = struct(a = 1, b = 2)",
+ "y = struct(c = 1, a = 2)",
+ "z = x + y\n");
}
@Test
@@ -1211,7 +1201,7 @@
@Test
public void testGetattrNoAttr() throws Exception {
- checkErrorContains(
+ checkEvalErrorContains(
"'struct' object has no attribute 'b'\nAvailable attributes: a",
"s = struct(a='val')",
"getattr(s, 'b')");
@@ -1251,9 +1241,7 @@
assertThat(eval("d[struct(b = 2)]")).isEqualTo("bb");
assertThat(eval("str([d[k] for k in d])")).isEqualTo("[\"aa\", \"bb\"]");
- checkErrorContains(
- "unhashable type: 'struct'",
- "{struct(a = []): 'foo'}");
+ checkEvalErrorContains("unhashable type: 'struct'", "{struct(a = []): 'foo'}");
}
@Test
@@ -1274,8 +1262,8 @@
@Test
public void testNsetBadMutableItem() throws Exception {
- checkEvalError("depsets cannot contain mutable items", "depset([([],)])");
- checkEvalError("depsets cannot contain mutable items", "depset([struct(a=[])])");
+ checkEvalErrorContains("depsets cannot contain mutable items", "depset([([],)])");
+ checkEvalErrorContains("depsets cannot contain mutable items", "depset([struct(a=[])])");
}
private static StructImpl makeStruct(String field, Object value) {
@@ -1364,11 +1352,11 @@
"data2 = provider()"
);
- checkEvalError("Cannot use '+' operator on instances of different providers (data1 and data2)",
+ checkEvalErrorContains(
+ "Cannot use '+' operator on instances of different providers (data1 and data2)",
"d1 = data1(x = 1)",
"d2 = data2(y = 2)",
- "d = d1 + d2"
- );
+ "d = d1 + d2");
}
@Test
@@ -1387,7 +1375,7 @@
@Test
public void declaredProvidersWithFieldsConcatError() throws Exception {
evalAndExport("data1 = provider(fields=['f1', 'f2'])", "data2 = provider(fields=['f3'])");
- checkEvalError(
+ checkEvalErrorContains(
"Cannot use '+' operator on instances of different providers (data1 and data2)",
"d1 = data1(f1=1, f2=2)",
"d2 = data2(f3=3)",
@@ -1397,7 +1385,7 @@
@Test
public void declaredProvidersWithOverlappingFieldsConcatError() throws Exception {
evalAndExport("data = provider(fields=['f1', 'f2'])");
- checkEvalError(
+ checkEvalErrorContains(
"Cannot use '+' operator on provider instances with overlapping field(s): f1",
"d1 = data(f1 = 4)",
"d2 = data(f1 = 5)",
@@ -1422,7 +1410,7 @@
@Test
public void declaredProvidersBadTypeForDoc() throws Exception {
- checkErrorContains(
+ checkEvalErrorContains(
"expected value of type 'string' for parameter 'doc', for call to function "
+ "provider(doc = '', fields = None)",
"provider(doc = 1)");
@@ -1564,7 +1552,7 @@
@Test
public void aspectBadTypeForDoc() throws Exception {
registerDummyStarlarkFunction();
- checkErrorContains(
+ checkEvalErrorContains(
"expected value of type 'string' for parameter 'doc', for call to function aspect(",
"aspect(impl, doc = 1)");
}
@@ -1686,7 +1674,8 @@
@Test
public void starTheOnlyAspectArg() throws Exception {
- checkEvalError("'*' must be the only string in 'attr_aspects' list",
+ checkEvalErrorContains(
+ "'*' must be the only string in 'attr_aspects' list",
"def _impl(target, ctx):",
" pass",
"aspect(_impl, attr_aspects=['*', 'foo'])");
@@ -1740,12 +1729,9 @@
@Test
public void testTargetsCanAddExecutionPlatformConstraints_enabled() throws Exception {
- StarlarkSemantics semantics =
- StarlarkSemantics.DEFAULT_SEMANTICS.toBuilder()
- .incompatibleDisallowRuleExecutionPlatformConstraintsAllowed(false)
- .build();
- ev = createEvaluationTestCase(semantics);
- ev.initialize();
+ setSkylarkSemanticsOptions(
+ "--incompatible_disallow_rule_execution_platform_constraints_allowed=false");
+ reset();
registerDummyStarlarkFunction();
scratch.file("test/BUILD", "toolchain_type(name = 'my_toolchain_type')");
@@ -1761,12 +1747,9 @@
@Test
public void testTargetsCanAddExecutionPlatformConstraints_notEnabled() throws Exception {
- StarlarkSemantics semantics =
- StarlarkSemantics.DEFAULT_SEMANTICS.toBuilder()
- .incompatibleDisallowRuleExecutionPlatformConstraintsAllowed(false)
- .build();
- ev = createEvaluationTestCase(semantics);
- ev.initialize();
+ setSkylarkSemanticsOptions(
+ "--incompatible_disallow_rule_execution_platform_constraints_allowed=false");
+ reset();
registerDummyStarlarkFunction();
scratch.file("test/BUILD", "toolchain_type(name = 'my_toolchain_type')");
@@ -1782,14 +1765,11 @@
@Test
public void testTargetsCanAddExecutionPlatformConstraints_disallowed() throws Exception {
- StarlarkSemantics semantics =
- StarlarkSemantics.DEFAULT_SEMANTICS.toBuilder()
- .incompatibleDisallowRuleExecutionPlatformConstraintsAllowed(true)
- .build();
- ev = createEvaluationTestCase(semantics);
- ev.setFailFast(false);
- ev.initialize();
+ setSkylarkSemanticsOptions(
+ "--incompatible_disallow_rule_execution_platform_constraints_allowed=true");
+ reset();
+ ev.setFailFast(false);
registerDummyStarlarkFunction();
scratch.file("test/BUILD", "toolchain_type(name = 'my_toolchain_type')");
evalAndExport(
