From b6e33bca1db50b3c6e8019351bc61e0e576cc912 Mon Sep 17 00:00:00 2001
From: Janak Ramakrishnan <janakr@google.com>
Date: Sun, 6 Sep 2015 21:05:23 +0000
Subject: Rollback of commit 5a94e59f02833f9142bad9203acd72626b089535.

*** Reason for rollback ***

Breaks serialization of SkyValues.

--
MOS_MIGRATED_REVID=102457225
---
 .../lib/events/util/EventCollectionApparatus.java  |   6 +-
 .../lib/packages/util/PackageFactoryApparatus.java |   4 +-
 .../build/lib/syntax/BuildFileASTTest.java         | 118 ++++++++++++---------
 .../devtools/build/lib/syntax/EnvironmentTest.java |  98 +++--------------
 .../devtools/build/lib/syntax/EvaluationTest.java  |   4 +-
 .../build/lib/syntax/EvaluationTestCase.java       |  90 ++++++----------
 .../devtools/build/lib/syntax/ParserTest.java      |  32 +++---
 .../build/lib/syntax/SkylarkEvaluationTest.java    |   2 +-
 .../devtools/build/lib/syntax/SkylarkListTest.java |   4 +-
 .../devtools/build/lib/syntax/SkylarkShell.java    |   8 +-
 .../devtools/build/lib/syntax/ValidationTests.java |   6 +-
 .../devtools/build/lib/testutil/TestMode.java      |  45 ++++----
 12 files changed, 167 insertions(+), 250 deletions(-)

(limited to 'src/test')

diff --git a/src/test/java/com/google/devtools/build/lib/events/util/EventCollectionApparatus.java b/src/test/java/com/google/devtools/build/lib/events/util/EventCollectionApparatus.java
index c7f85d6fcb..aa8d8f83a6 100644
--- a/src/test/java/com/google/devtools/build/lib/events/util/EventCollectionApparatus.java
+++ b/src/test/java/com/google/devtools/build/lib/events/util/EventCollectionApparatus.java
@@ -18,7 +18,7 @@ import com.google.devtools.build.lib.events.EventCollector;
 import com.google.devtools.build.lib.events.EventKind;
 import com.google.devtools.build.lib.events.PrintingEventHandler;
 import com.google.devtools.build.lib.events.Reporter;
-import com.google.devtools.build.lib.syntax.Environment;
+import com.google.devtools.build.lib.syntax.EvaluationContext;
 import com.google.devtools.build.lib.testutil.MoreAsserts;
 import com.google.devtools.build.lib.util.io.OutErr;
 
@@ -67,9 +67,9 @@ public class EventCollectionApparatus {
    */
   public void setFailFast(boolean failFast) {
     if (failFast) {
-      reporter.addHandler(Environment.FAIL_FAST_HANDLER);
+      reporter.addHandler(EvaluationContext.FAIL_FAST_HANDLER);
     } else {
-      reporter.removeHandler(Environment.FAIL_FAST_HANDLER);
+      reporter.removeHandler(EvaluationContext.FAIL_FAST_HANDLER);
     }
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
index 2f298518e3..24ac455762 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
@@ -30,9 +30,9 @@ import com.google.devtools.build.lib.packages.PackageFactory;
 import com.google.devtools.build.lib.packages.PackageFactory.LegacyGlobber;
 import com.google.devtools.build.lib.packages.RuleClassProvider;
 import com.google.devtools.build.lib.syntax.BuildFileAST;
-import com.google.devtools.build.lib.syntax.Environment;
 import com.google.devtools.build.lib.syntax.Label;
 import com.google.devtools.build.lib.syntax.ParserInputSource;
+import com.google.devtools.build.lib.syntax.SkylarkEnvironment;
 import com.google.devtools.build.lib.testutil.Scratch;
 import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
 import com.google.devtools.build.lib.testutil.TestUtils;
@@ -134,7 +134,7 @@ public class PackageFactoryApparatus {
     LegacyBuilder resultBuilder = factory.evaluateBuildFile(
         externalPkg, packageId, buildFileAST, buildFile,
         globber, ImmutableList.<Event>of(), ConstantRuleVisibility.PUBLIC, false, false,
-        new MakeEnvironment.Builder(), ImmutableMap.<PathFragment, Environment>of(),
+        new MakeEnvironment.Builder(), ImmutableMap.<PathFragment, SkylarkEnvironment>of(),
         ImmutableList.<Label>of());
     Package result = resultBuilder.build();
     Event.replayEventsOn(events.reporter(), result.getEvents());
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/BuildFileASTTest.java b/src/test/java/com/google/devtools/build/lib/syntax/BuildFileASTTest.java
index 00e9062d63..eff5badfb6 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/BuildFileASTTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/BuildFileASTTest.java
@@ -22,7 +22,11 @@ import static org.junit.Assert.assertTrue;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventCollector;
+import com.google.devtools.build.lib.events.EventKind;
+import com.google.devtools.build.lib.events.Reporter;
+import com.google.devtools.build.lib.events.util.EventCollectionApparatus;
 import com.google.devtools.build.lib.packages.CachingPackageLocator;
+import com.google.devtools.build.lib.testutil.MoreAsserts;
 import com.google.devtools.build.lib.testutil.Scratch;
 import com.google.devtools.build.lib.vfs.Path;
 
@@ -33,14 +37,13 @@ import org.junit.runners.JUnit4;
 import java.io.IOException;
 import java.util.Arrays;
 
-/**
- * Unit tests for BuildFileAST.
- */
 @RunWith(JUnit4.class)
-public class BuildFileASTTest extends EvaluationTestCase {
+public class BuildFileASTTest {
 
   private Scratch scratch = new Scratch();
 
+  private EventCollectionApparatus events = new EventCollectionApparatus(EventKind.ALL_EVENTS);
+
   private class ScratchPathPackageLocator implements CachingPackageLocator {
     @Override
     public Path getBuildFileForPackage(PackageIdentifier packageName) {
@@ -50,18 +53,13 @@ public class BuildFileASTTest extends EvaluationTestCase {
 
   private CachingPackageLocator locator = new ScratchPathPackageLocator();
 
-  @Override
-  public Environment newEnvironment() throws Exception {
-    return newBuildEnvironment();
-  }
-
   /**
    * Parses the contents of the specified string (using DUMMY_PATH as the fake
    * filename) and returns the AST. Resets the error handler beforehand.
    */
   private BuildFileAST parseBuildFile(String... lines) throws IOException {
     Path file = scratch.file("/a/build/file/BUILD", lines);
-    return BuildFileAST.parseBuildFile(file, getEventHandler(), locator, false);
+    return BuildFileAST.parseBuildFile(file, events.reporter(), locator, false);
   }
 
   @Test
@@ -71,9 +69,11 @@ public class BuildFileASTTest extends EvaluationTestCase {
         "",
         "x = [1,2,'foo',4] + [1,2, \"%s%d\" % ('foo', 1)]");
 
-    BuildFileAST buildfile = BuildFileAST.parseBuildFile(buildFile, getEventHandler(), null, false);
+    Environment env = new Environment();
+    Reporter reporter = new Reporter();
+    BuildFileAST buildfile = BuildFileAST.parseBuildFile(buildFile, reporter, null, false);
 
-    assertTrue(buildfile.exec(env, getEventHandler()));
+    assertTrue(buildfile.exec(env, reporter));
 
     // Test final environment is correctly modified:
     //
@@ -91,11 +91,15 @@ public class BuildFileASTTest extends EvaluationTestCase {
         "",
         "z = x + y");
 
-    setFailFast(false);
-    BuildFileAST buildfile = BuildFileAST.parseBuildFile(buildFile, getEventHandler(), null, false);
+    Environment env = new Environment();
+    Reporter reporter = new Reporter();
+    EventCollector collector = new EventCollector(EventKind.ALL_EVENTS);
+    reporter.addHandler(collector);
+    BuildFileAST buildfile = BuildFileAST.parseBuildFile(buildFile, reporter, null, false);
 
-    assertFalse(buildfile.exec(env, getEventHandler()));
-    Event e = assertContainsEvent("unsupported operand type(s) for +: 'int' and 'List'");
+    assertFalse(buildfile.exec(env, reporter));
+    Event e = MoreAsserts.assertContainsEvent(collector,
+        "unsupported operand type(s) for +: 'int' and 'List'");
     assertEquals(4, e.getLocation().getStartLineAndColumn().getLine());
   }
 
@@ -110,12 +114,13 @@ public class BuildFileASTTest extends EvaluationTestCase {
 
   @Test
   public void testFailsIfNewlinesAreMissing() throws Exception {
-    setFailFast(false);
+    events.setFailFast(false);
 
     BuildFileAST buildFileAST =
       parseBuildFile("foo() bar() something = baz() bar()");
 
-    Event event = assertContainsEvent("syntax error at \'bar\': expected newline");
+    Event event = events.collector().iterator().next();
+    assertEquals("syntax error at \'bar\': expected newline", event.getMessage());
     assertEquals("/a/build/file/BUILD",
                  event.getLocation().getPath().toString());
     assertEquals(1, event.getLocation().getStartLineAndColumn().getLine());
@@ -124,10 +129,11 @@ public class BuildFileASTTest extends EvaluationTestCase {
 
   @Test
   public void testImplicitStringConcatenationFails() throws Exception {
-    setFailFast(false);
+    events.setFailFast(false);
     BuildFileAST buildFileAST = parseBuildFile("a = 'foo' 'bar'");
-    Event event = assertContainsEvent(
-        "Implicit string concatenation is forbidden, use the + operator");
+    Event event = events.collector().iterator().next();
+    assertEquals("Implicit string concatenation is forbidden, use the + operator",
+        event.getMessage());
     assertEquals("/a/build/file/BUILD",
                  event.getLocation().getPath().toString());
     assertEquals(1, event.getLocation().getStartLineAndColumn().getLine());
@@ -137,10 +143,11 @@ public class BuildFileASTTest extends EvaluationTestCase {
 
   @Test
   public void testImplicitStringConcatenationAcrossLinesIsIllegal() throws Exception {
-    setFailFast(false);
+    events.setFailFast(false);
     BuildFileAST buildFileAST = parseBuildFile("a = 'foo'\n  'bar'");
 
-    Event event = assertContainsEvent("indentation error");
+    Event event = events.collector().iterator().next();
+    assertEquals("indentation error", event.getMessage());
     assertEquals("/a/build/file/BUILD",
                  event.getLocation().getPath().toString());
     assertEquals(2, event.getLocation().getStartLineAndColumn().getLine());
@@ -165,7 +172,7 @@ public class BuildFileASTTest extends EvaluationTestCase {
 
   @Test
   public void testWithSyntaxErrorsDoesNotPrintDollarError() throws Exception {
-    setFailFast(false);
+    events.setFailFast(false);
     BuildFileAST buildFile = parseBuildFile(
         "abi = cxx_abi + '-glibc-' + glibc_version + '-' + generic_cpu + '-' + sysname",
         "libs = [abi + opt_level + '/lib/libcc.a']",
@@ -175,11 +182,14 @@ public class BuildFileASTTest extends EvaluationTestCase {
         "           srcs = libs,",
         "           includes = [ abi + opt_level + '/include' ])");
     assertTrue(buildFile.containsErrors());
-    assertContainsEvent("syntax error at '+': expected expression");
-    assertFalse(buildFile.exec(env, getEventHandler()));
-    assertNull(findEvent(getEventCollector(), "$error$"));
+    Event event = events.collector().iterator().next();
+    assertEquals("syntax error at '+': expected expression", event.getMessage());
+    Environment env = new Environment();
+    assertFalse(buildFile.exec(env, events.reporter()));
+    assertNull(findEvent(events.collector(), "$error$"));
     // This message should not be printed anymore.
-    assertNull(findEvent(getEventCollector(), "contains syntax error(s)"));
+    Event event2 = findEvent(events.collector(), "contains syntax error(s)");
+    assertNull(event2);
   }
 
   @Test
@@ -192,7 +202,7 @@ public class BuildFileASTTest extends EvaluationTestCase {
         + "include(\"//foo/bar:BUILD\")\n"
         + "b = 4\n");
 
-    BuildFileAST buildFileAST = BuildFileAST.parseBuildFile(buildFile, getEventHandler(),
+    BuildFileAST buildFileAST = BuildFileAST.parseBuildFile(buildFile, events.reporter(),
                                                             locator, false);
 
     assertFalse(buildFileAST.containsErrors());
@@ -207,14 +217,15 @@ public class BuildFileASTTest extends EvaluationTestCase {
         "include(\"//foo/bar:defs\")\n"
         + "b = a + 1\n");
 
-    BuildFileAST buildFileAST = BuildFileAST.parseBuildFile(buildFile, getEventHandler(),
+    BuildFileAST buildFileAST = BuildFileAST.parseBuildFile(buildFile, events.reporter(),
                                                             locator, false);
 
     assertFalse(buildFileAST.containsErrors());
     assertThat(buildFileAST.getStatements()).hasSize(3);
 
-    setFailFast(false);
-    assertFalse(buildFileAST.exec(env, getEventHandler()));
+    Environment env = new Environment();
+    Reporter reporter = new Reporter();
+    assertFalse(buildFileAST.exec(env, reporter));
     assertEquals(2, env.lookup("b"));
   }
 
@@ -236,53 +247,63 @@ public class BuildFileASTTest extends EvaluationTestCase {
     assertFalse(buildFileAST.containsErrors());
     assertThat(buildFileAST.getStatements()).hasSize(8);
 
-    setFailFast(false);
-    assertFalse(buildFileAST.exec(env, getEventHandler()));
+    Environment env = new Environment();
+    Reporter reporter = new Reporter();
+    assertFalse(buildFileAST.exec(env, reporter));
     assertEquals(5, env.lookup("b"));
     assertEquals(7, env.lookup("c"));
   }
 
   @Test
   public void testFailInclude() throws Exception {
-    setFailFast(false);
+    events.setFailFast(false);
     BuildFileAST buildFileAST = parseBuildFile("include(\"//nonexistent\")");
     assertThat(buildFileAST.getStatements()).hasSize(1);
-    assertContainsEvent("Include of '//nonexistent' failed");
+    events.assertContainsEvent("Include of '//nonexistent' failed");
+  }
+
+
+  private class EmptyPackageLocator implements CachingPackageLocator {
+    @Override
+    public Path getBuildFileForPackage(PackageIdentifier packageName) {
+      return null;
+    }
   }
 
+  private CachingPackageLocator emptyLocator = new EmptyPackageLocator();
 
   @Test
   public void testFailInclude2() throws Exception {
-    setFailFast(false);
+    events.setFailFast(false);
     Path buildFile = scratch.file("/foo/bar/BUILD",
         "include(\"//nonexistent:foo\")\n");
-    BuildFileAST buildFileAST = BuildFileAST.parseBuildFile(
-        buildFile, getEventHandler(), Environment.EMPTY_PACKAGE_LOCATOR, false);
+    BuildFileAST buildFileAST = BuildFileAST.parseBuildFile(buildFile, events.reporter(),
+                                                            emptyLocator, false);
     assertThat(buildFileAST.getStatements()).hasSize(1);
-    assertContainsEvent("Package 'nonexistent' not found");
+    events.assertContainsEvent("Package 'nonexistent' not found");
   }
 
   @Test
   public void testInvalidInclude() throws Exception {
-    setFailFast(false);
+    events.setFailFast(false);
     BuildFileAST buildFileAST = parseBuildFile("include(2)");
     assertThat(buildFileAST.getStatements()).isEmpty();
-    assertContainsEvent("syntax error at '2'");
+    events.assertContainsEvent("syntax error at '2'");
   }
 
   @Test
   public void testRecursiveInclude() throws Exception {
-    setFailFast(false);
+    events.setFailFast(false);
     Path buildFile = scratch.file("/foo/bar/BUILD",
         "include(\"//foo/bar:BUILD\")\n");
 
-    BuildFileAST.parseBuildFile(buildFile, getEventHandler(), locator, false);
-    assertContainsEvent("Recursive inclusion");
+    BuildFileAST.parseBuildFile(buildFile, events.reporter(), locator, false);
+    events.assertContainsEvent("Recursive inclusion");
   }
 
   @Test
   public void testParseErrorInclude() throws Exception {
-    setFailFast(false);
+    events.setFailFast(false);
 
     scratch.file("/foo/bar/file",
         "a = 2 + % 3\n"); // parse error
@@ -290,13 +311,14 @@ public class BuildFileASTTest extends EvaluationTestCase {
     parseBuildFile("include(\"//foo/bar:file\")");
 
     // Check the location is properly reported
-    Event event = assertContainsEvent("syntax error at '%': expected expression");
+    Event event = events.collector().iterator().next();
     assertEquals("/foo/bar/file:1:9", event.getLocation().print());
+    assertEquals("syntax error at '%': expected expression", event.getMessage());
   }
 
   @Test
   public void testNonExistentIncludeReported() throws Exception {
-    setFailFast(false);
+    events.setFailFast(false);
     BuildFileAST buildFileAST = parseBuildFile("include('//foo:bar')");
     assertThat(buildFileAST.getStatements()).hasSize(1);
   }
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java
index 02021245c6..a324b13ff5 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java
@@ -31,8 +31,8 @@ import org.junit.runners.JUnit4;
 public class EnvironmentTest extends EvaluationTestCase {
 
   @Override
-  public Environment newEnvironment() {
-    return newBuildEnvironment();
+  public EvaluationContext newEvaluationContext() {
+    return EvaluationContext.newBuildContext(getEventHandler());
   }
 
   // Test the API directly
@@ -104,41 +104,25 @@ public class EnvironmentTest extends EvaluationTestCase {
 
   @Test
   public void testGetVariableNames() throws Exception {
-    Environment outerEnv;
-    Environment innerEnv;
-    try (Mutability mut = Mutability.create("outer")) {
-      outerEnv = Environment.builder(mut)
-          .setGlobals(Environment.BUILD).build()
-          .update("foo", "bar")
-          .update("wiz", 3);
-    }
-    try (Mutability mut = Mutability.create("inner")) {
-      innerEnv = Environment.builder(mut)
-          .setGlobals(outerEnv.getGlobals()).build()
-          .update("foo", "bat")
-          .update("quux", 42);
-    }
+    update("foo", "bar");
+    update("wiz", 3);
 
-    assertEquals(Sets.newHashSet("foo", "wiz",
-            "False", "None", "True",
-            "-", "bool", "dict", "enumerate", "int", "len", "list",
-            "range", "repr", "select", "sorted", "str", "zip"),
-        outerEnv.getVariableNames());
-    assertEquals(Sets.newHashSet("foo", "wiz", "quux",
-            "False", "None", "True",
-            "-", "bool", "dict", "enumerate", "int", "len", "list",
-            "range", "repr", "select", "sorted", "str", "zip"),
-        innerEnv.getVariableNames());
+    Environment nestedEnv = new Environment(getEnvironment());
+    nestedEnv.update("foo", "bat");
+    nestedEnv.update("quux", 42);
+
+    assertEquals(Sets.newHashSet("True", "False", "None", "foo", "wiz"),
+        getEnvironment().getVariableNames());
+    assertEquals(Sets.newHashSet("True", "False", "None", "foo", "wiz", "quux"),
+        nestedEnv.getVariableNames());
   }
 
   @Test
   public void testToString() throws Exception {
     update("subject", new StringLiteral("Hello, 'world'.", '\''));
     update("from", new StringLiteral("Java", '"'));
-    assertThat(getEnvironment().toString())
-        .startsWith("Environment(lexicalFrame=null, "
-            + "globalFrame=Frame[test]{\"from\": \"Java\", \"subject\": 'Hello, \\'world\\'.'}=>"
-            + "(BUILD){");
+    assertEquals("Environment{False -> false, None -> None, True -> true, from -> \"Java\", "
+        + "subject -> 'Hello, \\'world\\'.', }", getEnvironment().toString());
   }
 
   @Test
@@ -150,58 +134,4 @@ public class EnvironmentTest extends EvaluationTestCase {
       assertThat(e).hasMessage("update(value == null)");
     }
   }
-
-  @Test
-  public void testFrozen() throws Exception {
-    Environment env;
-    try (Mutability mutability = Mutability.create("testFrozen")) {
-      env = Environment.builder(mutability)
-          .setGlobals(Environment.BUILD).setEventHandler(Environment.FAIL_FAST_HANDLER).build();
-      env.update("x", 1);
-      assertEquals(env.lookup("x"), 1);
-      env.update("y", 2);
-      assertEquals(env.lookup("y"), 2);
-      assertEquals(env.lookup("x"), 1);
-      env.update("x", 3);
-      assertEquals(env.lookup("x"), 3);
-    }
-    try {
-      // This update to an existing variable should fail because the environment was frozen.
-      env.update("x", 4);
-      throw new Exception("failed to fail"); // not an AssertionError like fail()
-    } catch (AssertionError e) {
-      assertThat(e).hasMessage("Can't update x to 4 in frozen environment");
-    }
-    try {
-      // This update to a new variable should also fail because the environment was frozen.
-      env.update("newvar", 5);
-      throw new Exception("failed to fail"); // not an AssertionError like fail()
-    } catch (AssertionError e) {
-      assertThat(e).hasMessage("Can't update newvar to 5 in frozen environment");
-    }
-  }
-
-  @Test
-  public void testReadOnly() throws Exception {
-    Environment env = newSkylarkEnvironment()
-        .setup("special_var", 42)
-        .update("global_var", 666);
-
-    // We don't even get a runtime exception trying to modify these,
-    // because we get compile-time exceptions even before we reach runtime!
-    try {
-      env.eval("special_var = 41");
-      throw new AssertionError("failed to fail");
-    } catch (IllegalArgumentException e) {
-      assertThat(e).hasMessage("ERROR 1:1: Variable special_var is read only");
-    }
-
-    try {
-      env.eval("def foo(x): x += global_var; global_var = 36; return x", "foo(1)");
-      throw new AssertionError("failed to fail");
-    } catch (EvalExceptionWithStackTrace e) {
-      assertThat(e.getMessage()).contains("Variable 'global_var' is referenced before assignment. "
-          + "The variable is defined in the global scope.");
-    }
-  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
index 83a55be2d8..cf23858207 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
@@ -482,9 +482,9 @@ public class EvaluationTest extends EvaluationTestCase {
   @Test
   public void testDictComprehensions_ToString() throws Exception {
     assertEquals("{x: x for x in [1, 2]}",
-        parseExpression("{x : x for x in [1, 2]}").toString());
+        evaluationContext.parseExpression("{x : x for x in [1, 2]}").toString());
     assertEquals("{x + 'a': x for x in [1, 2]}",
-        parseExpression("{x + 'a' : x for x in [1, 2]}").toString());
+        evaluationContext.parseExpression("{x + 'a' : x for x in [1, 2]}").toString());
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTestCase.java b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTestCase.java
index e26234af0c..b2d661b7d4 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTestCase.java
@@ -35,76 +35,48 @@ import java.util.List;
  * Base class for test cases that use parsing and evaluation services.
  */
 public class EvaluationTestCase {
-
+  
   private EventCollectionApparatus eventCollectionApparatus;
   private PackageFactory factory;
   private TestMode testMode = TestMode.SKYLARK;
-  protected Environment env;
-  protected Mutability mutability = Mutability.create("test");
+  
+  protected EvaluationContext evaluationContext;
 
   public EvaluationTestCase()   {
     createNewInfrastructure();
   }
-
+  
   @Before
   public void setUp() throws Exception {
     createNewInfrastructure();
-    env = newEnvironment();
-  }
-
-  /**
-   * Creates a standard Environment for tests in the BUILD language.
-   * No PythonPreprocessing, mostly empty mutable Environment.
-   */
-  public Environment newBuildEnvironment() {
-    return Environment.builder(mutability)
-        .setGlobals(Environment.BUILD)
-        .setEventHandler(getEventHandler())
-        .setLoadingPhase()
-        .build();
+    evaluationContext = newEvaluationContext();
   }
 
-  /**
-   * Creates an Environment for Skylark with a mostly empty initial environment.
-   * For internal initialization or tests.
-   */
-  public Environment newSkylarkEnvironment() {
-    return Environment.builder(mutability)
-        .setSkylark()
-        .setGlobals(Environment.SKYLARK)
-        .setEventHandler(getEventHandler())
-        .build();
-  }
-
-  /**
-   * Creates a new Environment suitable for the test case. Subclasses may override it
-   * to fit their purpose and e.g. call newBuildEnvironment or newSkylarkEnvironment;
-   * or they may play with the testMode to run tests in either or both kinds of Environment.
-   * Note that all Environment-s may share the same Mutability, so don't close it.
-   * @return a fresh Environment.
-   */
-  public Environment newEnvironment() throws Exception {
+  public EvaluationContext newEvaluationContext() throws Exception {
     if (testMode == null) {
       throw new IllegalArgumentException(
           "TestMode is null. Please set a Testmode via setMode() or set the "
-          + "Environment manually by overriding newEnvironment()");
+          + "evaluatenContext manually by overriding newEvaluationContext()");
     }
-    return testMode.createEnvironment(getEventHandler(), null);
+
+    return testMode.createContext(getEventHandler(), factory.getEnvironment());
   }
 
   protected void createNewInfrastructure()  {
     eventCollectionApparatus = new EventCollectionApparatus(EventKind.ALL_EVENTS);
     factory = new PackageFactory(TestRuleClassProvider.getRuleClassProvider());
   }
-
+  
   /**
-   * Sets the specified {@code TestMode} and tries to create the appropriate {@code Environment}
+   * Sets the specified {@code TestMode} and tries to create the appropriate {@code
+   * EvaluationContext}
+   * 
    * @param testMode
    * @throws Exception
    */
   protected void setMode(TestMode testMode) throws Exception {
     this.testMode = testMode;
-    env = newEnvironment();
+    evaluationContext = newEvaluationContext();
   }
 
   protected void enableSkylarkMode() throws Exception {
@@ -124,33 +96,32 @@ public class EvaluationTestCase {
   }
 
   public Environment getEnvironment() {
-    return env;
+    return evaluationContext.getEnvironment();
   }
-
+  
   public boolean isSkylark() {
-    return env.isSkylark();
+    return evaluationContext.isSkylark();
   }
 
   protected List<Statement> parseFile(String... input) {
-    return env.parseFile(input);
+    return evaluationContext.parseFile(input);
   }
 
-  /** Parses an Expression from string without a supporting file */
   Expression parseExpression(String... input) {
-    return Parser.parseExpression(env.createLexer(input), getEventHandler());
+    return evaluationContext.parseExpression(input);
   }
 
   public EvaluationTestCase update(String varname, Object value) throws Exception {
-    env.update(varname, value);
+    evaluationContext.update(varname, value);
     return this;
   }
 
   public Object lookup(String varname) throws Exception {
-    return env.lookup(varname);
+    return evaluationContext.lookup(varname);
   }
 
   public Object eval(String... input) throws Exception {
-    return env.eval(input);
+    return evaluationContext.eval(input);
   }
 
   public void checkEvalError(String msg, String... input) throws Exception {
@@ -218,14 +189,15 @@ public class EvaluationTestCase {
 
   /**
    * Base class for test cases that run in specific modes (e.g. Build and/or Skylark)
-   */
+   *
+   */  
   protected abstract class ModalTestCase {
     private final SetupActions setup;
-
-    protected ModalTestCase() {
+    
+    protected ModalTestCase()   {
       setup = new SetupActions();
     }
-
+    
     /**
      * Allows the execution of several statements before each following test
      * @param statements The statement(s) to be executed
@@ -246,7 +218,7 @@ public class EvaluationTestCase {
       setup.registerUpdate(name, value);
       return this;
     }
-
+    
     /**
      * Evaluates two parameters and compares their results.
      * @param statement The statement to be evaluated
@@ -283,7 +255,7 @@ public class EvaluationTestCase {
       runTest(collectionTestable(statement, false, items));
       return this;
     }
-
+    
     /**
      * Evaluates the given statement and compares its result to the collection of expected objects
      * while considering their order
@@ -437,7 +409,7 @@ public class EvaluationTestCase {
   }
 
   /**
-   * A simple decorator that allows the execution of setup actions before running
+   * A simple decorator that allows the execution of setup actions before running 
    * a {@code Testable}
    */
   class TestableDecorator implements Testable {
@@ -508,7 +480,7 @@ public class EvaluationTestCase {
       }
     }
   }
-
+    
   /**
    * A class that executes each separate test in both modes (Build and Skylark)
    */
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
index 5137b45c65..e0c074a126 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
@@ -32,45 +32,43 @@ import org.junit.runners.JUnit4;
 import java.util.LinkedList;
 import java.util.List;
 
+
 /**
  *  Tests of parser behaviour.
  */
 @RunWith(JUnit4.class)
 public class ParserTest extends EvaluationTestCase {
 
-  Environment buildEnvironment;
+  EvaluationContext buildContext;
+  EvaluationContext buildContextWithPython;
 
   @Before
   @Override
   public void setUp() throws Exception {
     super.setUp();
-    buildEnvironment = newBuildEnvironment();
+    buildContext = EvaluationContext.newBuildContext(getEventHandler());
+    buildContextWithPython = EvaluationContext.newBuildContext(
+        getEventHandler(), new Environment(), /*parsePython*/true);
   }
 
   private Parser.ParseResult parseFileWithComments(String... input) {
-    return buildEnvironment.parseFileWithComments(input);
+    return buildContext.parseFileWithComments(input);
   }
-
-  /** Parses build code (not Skylark) */
   @Override
   protected List<Statement> parseFile(String... input) {
-    return buildEnvironment.parseFile(input);
+    return buildContext.parseFile(input);
   }
-
-  /** Parses a build code (not Skylark) with PythonProcessing enabled */
   private List<Statement> parseFileWithPython(String... input) {
-    return Parser.parseFile(
-        buildEnvironment.createLexer(input),
-        getEventHandler(),
-        Environment.EMPTY_PACKAGE_LOCATOR,
-        /*parsePython=*/true).statements;
+    return buildContextWithPython.parseFile(input);
   }
-
-  /** Parses Skylark code */
   private List<Statement> parseFileForSkylark(String... input) {
-    return env.parseFile(input);
+    return evaluationContext.parseFile(input);
+  }
+  private Statement parseStatement(String... input) {
+    return buildContext.parseStatement(input);
   }
 
+
   private static String getText(String text, ASTNode node) {
     return text.substring(node.getLocation().getStartOffset(),
                           node.getLocation().getEndOffset());
@@ -709,7 +707,7 @@ public class ParserTest extends EvaluationTestCase {
   @Test
   public void testParserContainsErrors() throws Exception {
     setFailFast(false);
-    parseFile("+");
+    parseStatement("+");
     assertContainsEvent("syntax error at '+'");
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
index a473f9525b..16bcb8010f 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -526,7 +526,7 @@ public class SkylarkEvaluationTest extends EvaluationTest {
     new SkylarkTest()
         .update("mock", new Mock())
         .testIfExactError("Keyword arguments are not allowed when calling a java method"
-            + "\nwhile calling method 'string' for type Mock",
+            + "\nwhile calling method 'string' on object of type Mock",
             "mock.string(key=True)");
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java
index 7558b6c4f3..86ad1fb557 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java
@@ -45,8 +45,8 @@ public class SkylarkListTest extends EvaluationTestCase {
       SkylarkList.lazyList(new CustomIterable(), Integer.class);
 
   @Override
-  public Environment newEnvironment() throws Exception {
-    return newSkylarkEnvironment().update("lazy", list);
+  public EvaluationContext newEvaluationContext() throws Exception {
+    return super.newEvaluationContext().update("lazy", list);
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkShell.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkShell.java
index b12f51b1d0..c4bb7d84a9 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkShell.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkShell.java
@@ -15,6 +15,7 @@ package com.google.devtools.build.lib.syntax;
 
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.rules.SkylarkModules;
 
 import java.io.BufferedReader;
 import java.io.IOException;
@@ -41,9 +42,8 @@ class SkylarkShell {
 
   private final BufferedReader reader = new BufferedReader(
       new InputStreamReader(System.in, Charset.defaultCharset()));
-  private final Mutability mutability = Mutability.create("shell");
-  private final Environment env = Environment.builder(mutability)
-      .setSkylark().setGlobals(Environment.SKYLARK).setEventHandler(PRINT_HANDLER).build();
+  private final EvaluationContext ev =
+      SkylarkModules.newEvaluationContext(PRINT_HANDLER);
 
   public String read() {
     StringBuilder input = new StringBuilder();
@@ -70,7 +70,7 @@ class SkylarkShell {
     String input;
     while ((input = read()) != null) {
       try {
-        Object result = env.eval(input);
+        Object result = ev.eval(input);
         if (result != null) {
           System.out.println(Printer.repr(result));
         }
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTests.java b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTests.java
index 3ebdf8dde4..57b341f3b9 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTests.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTests.java
@@ -95,7 +95,7 @@ public class ValidationTests extends EvaluationTestCase {
 
   @Test
   public void testBuiltinSymbolsAreReadOnly() throws Exception {
-    checkError("Variable repr is read only", "repr = 1");
+    checkError("Variable rule is read only", "rule = 1");
   }
 
   @Test
@@ -300,8 +300,8 @@ public class ValidationTests extends EvaluationTestCase {
   @Test
   public void testFunctionReturnsFunction() {
     parse(
-        "def rule(*, implementation): return None",
-        "def impl(ctx): return None",
+        "def impl(ctx):",
+        "  return None",
         "",
         "skylark_rule = rule(implementation = impl)",
         "",
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/TestMode.java b/src/test/java/com/google/devtools/build/lib/testutil/TestMode.java
index ea887f9d68..5ec7ac27b2 100644
--- a/src/test/java/com/google/devtools/build/lib/testutil/TestMode.java
+++ b/src/test/java/com/google/devtools/build/lib/testutil/TestMode.java
@@ -14,36 +14,31 @@
 package com.google.devtools.build.lib.testutil;
 
 import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.rules.SkylarkModules;
 import com.google.devtools.build.lib.syntax.Environment;
-import com.google.devtools.build.lib.syntax.Mutability;
+import com.google.devtools.build.lib.syntax.EvaluationContext;
 
 /**
  * Describes a particular testing mode by determining how the
- * appropriate {@code Environment} has to be created
+ * appropriate {@code EvaluationContext} has to be created
  */
 public abstract class TestMode {
-  public static final TestMode BUILD =
-      new TestMode() {
-        @Override
-        public Environment createEnvironment(EventHandler eventHandler, Environment environment) {
-          return Environment.builder(Mutability.create("build test"))
-              .setGlobals(environment == null ? Environment.BUILD : environment.getGlobals())
-              .setEventHandler(eventHandler)
-              .build();
-        }
-      };
+  public static final TestMode BUILD = new TestMode() {
+    @Override
+    public EvaluationContext createContext(EventHandler eventHandler, Environment environment)
+        throws Exception {
+      return EvaluationContext.newBuildContext(eventHandler, environment);
+    }
+  };
 
-  public static final TestMode SKYLARK =
-      new TestMode() {
-        @Override
-        public Environment createEnvironment(EventHandler eventHandler, Environment environment) {
-          return Environment.builder(Mutability.create("skylark test"))
-              .setSkylark()
-              .setGlobals(environment == null ? Environment.SKYLARK : environment.getGlobals())
-              .setEventHandler(eventHandler)
-              .build();
-        }
-      };
+  public static final TestMode SKYLARK = new TestMode() {
+    @Override
+    public EvaluationContext createContext(EventHandler eventHandler, Environment environment)
+        throws Exception {
+      return SkylarkModules.newEvaluationContext(eventHandler);
+    }
+  };
 
-  public abstract Environment createEnvironment(EventHandler eventHandler, Environment environment);
-}
+  public abstract EvaluationContext createContext(
+      EventHandler eventHandler, Environment environment) throws Exception;
+}
\ No newline at end of file
-- 
cgit v1.2.3