Skip to content

Commit

Permalink
bazel tests: delete TestMode and other test junk
Browse files Browse the repository at this point in the history
The only differences in the test modes, BUILD and SKYLARK, were:
- the name of the Mutability, which is insignificant.
- whether "BUILD dialect checks", such as no def, no f(**kwargs),
  are applied. These are moved from syntax.EvaluationTest to packages.PackageFactoryTest.
- whether validation errors were reported as events or a thrown exception.
  Now it's always an exception.
- whether the validator gets the 'isBuildFile' flag.
  This affects a small (and diminishing) set of things,
  which should be checked directly.

The ModalTestCase hierarchy (base, BUILD only, Skylark only, both modes)
has been flattened into a single class, Scenario.

EvaluationTestCase:
- newStarlarkThreadWithSkylarkOptions is now setSemantics (stateful)
- inline the code formerly in TestMode and simplify newStarlarkThread et al.
- make bad dependencies on build-base explicit. (They are revealed, but not new.)
- hide fields
- simplify exec().

SkylarkEvaluationTest:
- break "extends EvaluationTest" edge:
  There's no need to duplicate the whole suite for both modes;
  nearly every test was independent of mode (and one suspects most
  were added with no knowledge of the intended design).
- move tests of FlagGuardedValue into StarlarkFlagGuardingTest.
  (They were the only tests to use the 'builtins' parameter, now removed.)
PiperOrigin-RevId: 294451364
  • Loading branch information
adonovan authored and copybara-github committed Feb 11, 2020
1 parent 59d7864 commit 469d855
Show file tree
Hide file tree
Showing 13 changed files with 841 additions and 1,041 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.RootedPath;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -1209,4 +1210,56 @@ public void testGlobPatternExtractor() {
assertThat(globs).containsExactly("ab", "a", "**/*");
assertThat(globsWithDirs).containsExactly("c");
}

// Tests of BUILD file dialect checks:

@Test
public void testDefInBuild() throws Exception {
checkBuildDialectError(
"def func(): pass", //
"function definitions are not allowed in BUILD files");
}

@Test
public void testForStatementForbiddenInBuild() throws Exception {
checkBuildDialectError(
"for _ in []: pass", //
"for loops are not allowed");
}

@Test
public void testIfStatementForbiddenInBuild() throws Exception {
checkBuildDialectError(
"if False: pass", //
"if statements are not allowed");
}

@Test
public void testKwargsForbiddenInBuild() throws Exception {
checkBuildDialectError(
"print(**dict)", //
"**kwargs arguments are not allowed in BUILD files");
checkBuildDialectError(
"len(dict(**{'a': 1}))", //
"**kwargs arguments are not allowed in BUILD files");
}

@Test
public void testArgsForbiddenInBuild() throws Exception {
checkBuildDialectError(
"print(*['a'])", //
"*args arguments are not allowed in BUILD files");
}

// Asserts that evaluation of the specified BUILD file produces the expected error.
// Modifies: scratch, events, packages; be careful when calling more than once per @Test!
private void checkBuildDialectError(String content, String expectedError)
throws IOException, InterruptedException, NoSuchPackageException {
events.clear();
events.setFailFast(false);
Path file = scratch.overwriteFile("/p/BUILD", content);
Package pkg = packages.eval("p", RootedPath.toRootedPath(root, file));
assertThat(pkg.containsErrors()).isTrue();
events.assertContainsError(expectedError);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ public void testSplitEscaped() throws Exception {
.testExpression("split_escaped('a%:', ':')", StarlarkList.of(mu, "a:"));
}

private ModalTestCase newTest(String... skylarkOptions) throws IOException {
return new SkylarkTest(skylarkOptions)
private Scenario newTest(String... skylarkOptions) throws IOException {
return new Scenario(skylarkOptions)
// A mock implementation of Label to be able to parse lib_cc_configure under default
// Skylark environment (lib_cc_configure is meant to be used from the repository
// environment).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.devtools.build.lib.packages.SymbolGenerator;
import com.google.devtools.build.lib.rules.platform.PlatformCommon;
import com.google.devtools.build.lib.syntax.Module;
import com.google.devtools.build.lib.syntax.Mutability;
import com.google.devtools.build.lib.syntax.Starlark;
import com.google.devtools.build.lib.syntax.StarlarkThread;
import com.google.devtools.build.lib.syntax.util.EvaluationTestCase;
Expand Down Expand Up @@ -55,9 +56,10 @@ private EvaluationTestCase createEvaluationTestCase() throws Exception {
EvaluationTestCase ev =
new EvaluationTestCase() {
@Override
public StarlarkThread newStarlarkThread() throws Exception {
public StarlarkThread newStarlarkThread() {
Mutability mu = Mutability.create("test");
StarlarkThread thread =
StarlarkThread.builder(mutability)
StarlarkThread.builder(mu)
.setSemantics(getSkylarkSemantics())
.setGlobals(
globals.withLabel(
Expand Down
93 changes: 47 additions & 46 deletions src/test/java/com/google/devtools/build/lib/syntax/DepsetTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -136,23 +136,22 @@ public void testOrderItems() throws Exception {

@Test
public void testBadOrder() throws Exception {
new BothModesTest().testIfExactError(
"Invalid order: non_existing",
"depset(['a'], order='non_existing')");
new Scenario()
.testIfExactError("Invalid order: non_existing", "depset(['a'], order='non_existing')");
}

@Test
public void testBadOrderDirect() throws Exception {
new BothModesTest().testIfExactError(
"Invalid order: non_existing",
"depset(direct = ['a'], order='non_existing')");
new Scenario()
.testIfExactError(
"Invalid order: non_existing", "depset(direct = ['a'], order='non_existing')");
}

@Test
public void testBadOrderItems() throws Exception {
new BothModesTest().testIfExactError(
"Invalid order: non_existing",
"depset(items = ['a'], order='non_existing')");
new Scenario()
.testIfExactError(
"Invalid order: non_existing", "depset(items = ['a'], order='non_existing')");
}

@Test
Expand Down Expand Up @@ -195,49 +194,53 @@ public void testTransitiveIncompatibleOrder() throws Exception {

@Test
public void testBadGenericType() throws Exception {
new BothModesTest().testIfExactError(
"cannot add an item of type 'int' to a depset of 'string'",
"depset(['a', 5])");
new Scenario()
.testIfExactError(
"cannot add an item of type 'int' to a depset of 'string'", "depset(['a', 5])");
}

@Test
public void testBadGenericTypeDirect() throws Exception {
new BothModesTest().testIfExactError(
"cannot add an item of type 'int' to a depset of 'string'",
"depset(direct = ['a', 5])");
new Scenario()
.testIfExactError(
"cannot add an item of type 'int' to a depset of 'string'",
"depset(direct = ['a', 5])");
}

@Test
public void testBadGenericTypeItems() throws Exception {
new BothModesTest().testIfExactError(
"cannot add an item of type 'int' to a depset of 'string'",
"depset(items = ['a', 5])");
new Scenario()
.testIfExactError(
"cannot add an item of type 'int' to a depset of 'string'", "depset(items = ['a', 5])");
}

@Test
public void testBadGenericTypeTransitive() throws Exception {
new BothModesTest().testIfExactError(
"cannot add an item of type 'int' to a depset of 'string'",
"depset(['a', 'b'], transitive=[depset([1])])");
new Scenario()
.testIfExactError(
"cannot add an item of type 'int' to a depset of 'string'",
"depset(['a', 'b'], transitive=[depset([1])])");
}

@Test
public void testLegacyAndNewApi() throws Exception {
new BothModesTest().testIfExactError(
"Do not pass both 'direct' and 'items' argument to depset constructor.",
"depset(['a', 'b'], direct = ['c', 'd'])");
new Scenario()
.testIfExactError(
"Do not pass both 'direct' and 'items' argument to depset constructor.",
"depset(['a', 'b'], direct = ['c', 'd'])");
}

@Test
public void testItemsAndTransitive() throws Exception {
new BothModesTest().testIfExactError(
"expected type 'sequence' for items but got type 'depset' instead",
"depset(items = depset(), transitive = [depset()])");
new Scenario()
.testIfExactError(
"expected type 'sequence' for items but got type 'depset' instead",
"depset(items = depset(), transitive = [depset()])");
}

@Test
public void testTooManyPositionals() throws Exception {
new BothModesTest()
new Scenario()
.testIfErrorContains(
"depset() accepts no more than 2 positional arguments but got 3",
"depset([], 'default', [])");
Expand Down Expand Up @@ -272,10 +275,10 @@ public void testTransitiveOrderDirect() throws Exception {

@Test
public void testIncompatibleUnion() throws Exception {
new BothModesTest("--incompatible_depset_union=true")
new Scenario("--incompatible_depset_union=true")
.testIfErrorContains("`+` operator on a depset is forbidden", "depset([]) + ['a']");

new BothModesTest("--incompatible_depset_union=true")
new Scenario("--incompatible_depset_union=true")
.testIfErrorContains("`|` operator on a depset is forbidden", "depset([]) | ['a']");
}

Expand All @@ -288,7 +291,7 @@ private void assertContainsInOrder(String statement, Object... expectedElements)

@Test
public void testUnionOrder() throws Exception {
thread = newStarlarkThreadWithSkylarkOptions("--incompatible_depset_union=false");
setSemantics("--incompatible_depset_union=false");
exec(
"def func():",
" s1 = depset()",
Expand All @@ -303,15 +306,15 @@ public void testUnionOrder() throws Exception {

@Test
public void testUnionIncompatibleOrder() throws Exception {
thread = newStarlarkThreadWithSkylarkOptions("--incompatible_depset_union=false");
setSemantics("--incompatible_depset_union=false");
checkEvalError(
"Order mismatch: topological != postorder",
"depset(['a', 'b'], order='postorder') + depset(['c', 'd'], order='topological')");
}

@Test
public void testFunctionReturnsDepset() throws Exception {
thread = newStarlarkThreadWithSkylarkOptions("--incompatible_depset_union=false");
setSemantics("--incompatible_depset_union=false");
exec(
"def func():", //
" t = depset()",
Expand All @@ -324,7 +327,7 @@ public void testFunctionReturnsDepset() throws Exception {

@Test
public void testPlusEqualsWithList() throws Exception {
thread = newStarlarkThreadWithSkylarkOptions("--incompatible_depset_union=false");
setSemantics("--incompatible_depset_union=false");
exec(
"def func():", //
" t = depset()",
Expand All @@ -336,7 +339,7 @@ public void testPlusEqualsWithList() throws Exception {

@Test
public void testPlusEqualsNoSideEffects() throws Exception {
thread = newStarlarkThreadWithSkylarkOptions("--incompatible_depset_union=false");
setSemantics("--incompatible_depset_union=false");
exec(
"def func():",
" s1 = depset()",
Expand All @@ -350,7 +353,7 @@ public void testPlusEqualsNoSideEffects() throws Exception {

@Test
public void testFuncParamNoSideEffects() throws Exception {
thread = newStarlarkThreadWithSkylarkOptions("--incompatible_depset_union=false");
setSemantics("--incompatible_depset_union=false");
exec(
"def func1(t):",
" t += ['b']",
Expand All @@ -365,7 +368,7 @@ public void testFuncParamNoSideEffects() throws Exception {

@Test
public void testTransitiveOrdering() throws Exception {
thread = newStarlarkThreadWithSkylarkOptions("--incompatible_depset_union=false");
setSemantics("--incompatible_depset_union=false");
exec(
"def func():",
" sa = depset(['a'], order='postorder')",
Expand All @@ -379,7 +382,7 @@ public void testTransitiveOrdering() throws Exception {

@Test
public void testLeftRightDirectOrdering() throws Exception {
thread = newStarlarkThreadWithSkylarkOptions("--incompatible_depset_union=false");
setSemantics("--incompatible_depset_union=false");
exec(
"def func():",
" t = depset()",
Expand All @@ -394,7 +397,7 @@ public void testLeftRightDirectOrdering() throws Exception {

@Test
public void testToString() throws Exception {
thread = newStarlarkThreadWithSkylarkOptions("--incompatible_depset_union=false");
setSemantics("--incompatible_depset_union=false");
exec(
"s = depset() + [2, 4, 6] + [3, 4, 5]", //
"x = str(s)");
Expand All @@ -403,7 +406,7 @@ public void testToString() throws Exception {

@Test
public void testToStringWithOrder() throws Exception {
thread = newStarlarkThreadWithSkylarkOptions("--incompatible_depset_union=false");
setSemantics("--incompatible_depset_union=false");
exec(
"s = depset(order = 'topological') + [2, 4, 6] + [3, 4, 5]", //
"x = str(s)");
Expand All @@ -416,7 +419,7 @@ private Depset get(String varname) throws Exception {

@Test
public void testToList() throws Exception {
thread = newStarlarkThreadWithSkylarkOptions("--incompatible_depset_union=false");
setSemantics("--incompatible_depset_union=false");
exec(
"s = depset() + [2, 4, 6] + [3, 4, 5]", //
"x = s.to_list()");
Expand Down Expand Up @@ -520,8 +523,7 @@ public Depset merge(Depset[] sets) throws Exception {
@Test
public void testMutableDepsetElementsLegacyBehavior() throws Exception {
// See b/144992997 and github.com/bazelbuild/bazel/issues/10313.
thread =
newStarlarkThreadWithSkylarkOptions("--incompatible_always_check_depset_elements=false");
setSemantics("--incompatible_always_check_depset_elements=false");

// Test legacy depset(...) and new depset(direct=...) constructors.

Expand Down Expand Up @@ -554,8 +556,7 @@ public void testMutableDepsetElementsLegacyBehavior() throws Exception {
@Test
public void testMutableDepsetElementsDesiredBehavior() throws Exception {
// See b/144992997 and github.com/bazelbuild/bazel/issues/10313.
thread =
newStarlarkThreadWithSkylarkOptions("--incompatible_always_check_depset_elements=true");
setSemantics("--incompatible_always_check_depset_elements=true");

// Test legacy depset(...) and new depset(direct=...) constructors.

Expand Down Expand Up @@ -588,7 +589,7 @@ public void testMutableDepsetElementsDesiredBehavior() throws Exception {
@Test
public void testDepthExceedsLimitDuringIteration() throws Exception {
NestedSet.setApplicationDepthLimit(2000);
new SkylarkTest()
new Scenario()
.setUp(
"def create_depset(depth):",
" x = depset([0])",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,30 +74,33 @@ public void testDatatypeMutabilityShallow() throws Exception {

assertThat(EvalUtils.isImmutable(makeList(null))).isTrue();
assertThat(EvalUtils.isImmutable(makeDict(null))).isTrue();
assertThat(EvalUtils.isImmutable(makeList(thread.mutability()))).isFalse();
assertThat(EvalUtils.isImmutable(makeDict(thread.mutability()))).isFalse();

Mutability mu = Mutability.create("test");
assertThat(EvalUtils.isImmutable(makeList(mu))).isFalse();
assertThat(EvalUtils.isImmutable(makeDict(mu))).isFalse();
}

@Test
public void testDatatypeMutabilityDeep() throws Exception {
Mutability mu = Mutability.create("test");
assertThat(EvalUtils.isImmutable(Tuple.of(makeList(null)))).isTrue();

assertThat(EvalUtils.isImmutable(Tuple.of(makeList(thread.mutability())))).isFalse();
assertThat(EvalUtils.isImmutable(Tuple.of(makeList(mu)))).isFalse();
}

@Test
public void testComparatorWithDifferentTypes() throws Exception {
Mutability mu = Mutability.create("test");
Object[] objects = {
"1",
2,
true,
Starlark.NONE,
Tuple.of(1, 2, 3),
Tuple.of("1", "2", "3"),
StarlarkList.of(thread.mutability(), 1, 2, 3),
StarlarkList.of(thread.mutability(), "1", "2", "3"),
Dict.of(thread.mutability(), "key", 123),
Dict.of(thread.mutability(), 123, "value"),
StarlarkList.of(mu, 1, 2, 3),
StarlarkList.of(mu, "1", "2", "3"),
Dict.of(mu, "key", 123),
Dict.of(mu, 123, "value"),
StructProvider.STRUCT.create(ImmutableMap.of("key", (Object) "value"), "no field %s"),
};

Expand Down
Loading

0 comments on commit 469d855

Please sign in to comment.