Remove the full client environment from BuildConfiguration in favor of computing the test environment as early as possible, and passing that along.

--
MOS_MIGRATED_REVID=91388451

11 files changed, 79 insertions, 92 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationCollectionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationCollectionFactory.java
index 8ffac43d7e..e63a631739 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationCollectionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationCollectionFactory.java
@@ -36,7 +36,7 @@ public interface ConfigurationCollectionFactory {
    * via configuration transitions.
    * @param loadedPackageProvider the package provider
    * @param buildOptions top-level build options representing the command-line
-   * @param clientEnv the system environment
+   * @param testEnv the test environment
    * @param errorEventListener the event listener for errors
    * @param performSanityCheck flag to signal about performing sanity check. Can be false only for
    * tests in skyframe. Legacy mode uses loadedPackageProvider == null condition for this.
@@ -48,7 +48,7 @@ public interface ConfigurationCollectionFactory {
       ConfigurationFactory configurationFactory,
       PackageProviderForConfigurations loadedPackageProvider,
       BuildOptions buildOptions,
-      Map<String, String> clientEnv,
+      Map<String, String> testEnv,
       EventHandler errorEventListener,
       boolean performSanityCheck) throws InvalidConfigurationException;
 }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index 4bafd3f2f9..1b002a5420 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -554,6 +554,10 @@ public final class BuildConfiguration implements Serializable {
         help = "Specifies a suffix to be added to the configuration directory.")
     public String platformSuffix;
 
+    // TODO(bazel-team): The test environment is actually computed in BlazeRuntime and this option
+    // is not read anywhere else. Thus, it should be in a different options class, preferably one
+    // specific to the "test" command or maybe in its own configuration fragment.
+    // BlazeRuntime, though.
     @Option(name = "test_env",
         converter = Converters.OptionalAssignmentConverter.class,
         allowMultiple = true,
@@ -902,11 +906,7 @@ public final class BuildConfiguration implements Serializable {
   private final String mnemonic;
   private final String platformName;
 
-  /**
-   * It is not fingerprinted because it should only be used to access
-   * variables that do not break the hermetism of build rules.
-   */
-  private final ImmutableMap<String, String> clientEnvironment;
+  private final ImmutableMap<String, String> testEnvironment;
 
   /**
    * Helper container for {@link #transitiveOptionsMap} below.
@@ -995,17 +995,16 @@ public final class BuildConfiguration implements Serializable {
   BuildConfiguration(BlazeDirectories directories,
                      Map<Class<? extends Fragment>, Fragment> fragmentsMap,
                      BuildOptions buildOptions,
-                     Map<String, String> clientEnv,
+                     Map<String, String> testEnv,
                      boolean actionsDisabled) {
     this.actionsEnabled = !actionsDisabled;
-    fragments = ImmutableMap.copyOf(fragmentsMap);
-
-    // This is a view that will be updated upon each client command.
-    this.clientEnvironment = ImmutableMap.copyOf(clientEnv);
+    this.fragments = ImmutableMap.copyOf(fragmentsMap);
 
     this.buildOptions = buildOptions;
     this.options = buildOptions.get(Options.class);
 
+    this.testEnvironment = ImmutableMap.copyOf(testEnv);
+
     this.mnemonic = buildMnemonic();
     String outputDirName = (options.shortName != null) ? options.shortName : mnemonic;
     this.shortName = buildShortName(outputDirName);
@@ -1055,7 +1054,7 @@ public final class BuildConfiguration implements Serializable {
     globalMakeEnv = globalMakeEnvBuilder.build();
 
     cacheKey = computeCacheKey(
-        directories, fragmentsMap, this.buildOptions, this.clientEnvironment);
+        directories, fragmentsMap, this.buildOptions, this.testEnvironment);
     shortCacheKey = shortName + "-" + Fingerprint.md5Digest(cacheKey);
   }
 
@@ -1313,18 +1312,6 @@ public final class BuildConfiguration implements Serializable {
   }
 
   /**
-   * Avoid this method. The client environment is not part of the configuration's signature, so
-   * calls to this method introduce a non-hermetic access to data that is not visible to Skyframe.
-   *
-   * @return an unmodifiable view of the bazel client's environment
-   *         upon its most recent request.
-   */
-  // TODO(bazel-team): Remove this.
-  public Map<String, String> getClientEnv() {
-    return clientEnvironment;
-  }
-
-  /**
    * Returns the {@link Option} class the defines the given option, null if the
    * option isn't recognized.
    *
@@ -1705,30 +1692,7 @@ public final class BuildConfiguration implements Serializable {
    * set by the --test_env options.
    */
   public Map<String, String> getTestEnv() {
-    return getTestEnv(options.testEnvironment, clientEnvironment);
-  }
-
-  /**
-   * Returns user-specified test environment variables and their values, as
-   * set by the --test_env options.
-   *
-   * @param envOverrides The --test_env flag values.
-   * @param clientEnvironment The full client environment.
-   */
-  public static Map<String, String> getTestEnv(List<Map.Entry<String, String>> envOverrides,
-                                        Map<String, String> clientEnvironment) {
-    Map<String, String> testEnv = new HashMap<>();
-    for (Map.Entry<String, String> var : envOverrides) {
-      if (var.getValue() != null) {
-        testEnv.put(var.getKey(), var.getValue());
-      } else {
-        String value = clientEnvironment.get(var.getKey());
-        if (value != null) {
-          testEnv.put(var.getKey(), value);
-        }
-      }
-    }
-    return testEnv;
+    return testEnvironment;
   }
 
   public TriState cacheTestResults() {
@@ -1810,7 +1774,7 @@ public final class BuildConfiguration implements Serializable {
    */
   static String computeCacheKey(BlazeDirectories directories,
       Map<Class<? extends Fragment>, Fragment> fragments,
-      BuildOptions buildOptions, Map<String, String> clientEnv) {
+      BuildOptions buildOptions, Map<String, String> testEnvironment) {
 
     // Creates a full fingerprint of all constructor parameters, used for
     // canonicalization.
@@ -1830,8 +1794,7 @@ public final class BuildConfiguration implements Serializable {
     keys.add(buildOptions.computeCacheKey());
     // This is needed so that if we have --test_env=VAR, the configuration key is updated if the
     // environment variable VAR is updated.
-    keys.add(BuildConfiguration.getTestEnv(
-        buildOptions.get(Options.class).testEnvironment, clientEnv).toString());
+    keys.add(testEnvironment.toString());
     keys.add(directories.getWorkspace().toString());
 
     for (Fragment fragment : fragments.values()) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationKey.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationKey.java
index e8fcf344c7..53fba105b3 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationKey.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationKey.java
@@ -32,7 +32,7 @@ public final class BuildConfigurationKey {
 
   private final BuildOptions buildOptions;
   private final BlazeDirectories directories;
-  private final Map<String, String> clientEnv;
+  private final Map<String, String> testEnv;
   private final ImmutableSortedSet<String> multiCpu;
 
   /**
@@ -41,16 +41,16 @@ public final class BuildConfigurationKey {
    * Note that the BuildConfiguration.Options instance must not contain unresolved relative paths.
    */
   public BuildConfigurationKey(BuildOptions buildOptions, BlazeDirectories directories,
-      Map<String, String> clientEnv, Set<String> multiCpu) {
+      Map<String, String> testEnv, Set<String> multiCpu) {
     this.buildOptions = Preconditions.checkNotNull(buildOptions);
     this.directories = Preconditions.checkNotNull(directories);
-    this.clientEnv = ImmutableMap.copyOf(clientEnv);
+    this.testEnv = ImmutableMap.copyOf(testEnv);
     this.multiCpu = ImmutableSortedSet.copyOf(multiCpu);
   }
 
   public BuildConfigurationKey(BuildOptions buildOptions, BlazeDirectories directories,
-      Map<String, String> clientEnv) {
-    this(buildOptions, directories, clientEnv, ImmutableSet.<String>of());
+      Map<String, String> testEnv) {
+    this(buildOptions, directories, testEnv, ImmutableSet.<String>of());
   }
 
   public BuildOptions getBuildOptions() {
@@ -61,8 +61,8 @@ public final class BuildConfigurationKey {
     return directories;
   }
 
-  public Map<String, String> getClientEnv() {
-    return clientEnv;
+  public Map<String, String> getTestEnv() {
+    return testEnv;
   }
 
   public ImmutableSortedSet<String> getMultiCpu() {
@@ -81,12 +81,12 @@ public final class BuildConfigurationKey {
     BuildConfigurationKey k = (BuildConfigurationKey) o;
     return buildOptions.equals(k.buildOptions)
         && directories.equals(k.directories)
-        && clientEnv.equals(k.clientEnv)
+        && testEnv.equals(k.testEnv)
         && multiCpu.equals(k.multiCpu);
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(buildOptions, directories, clientEnv, multiCpu);
+    return Objects.hash(buildOptions, directories, testEnv, multiCpu);
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationFactory.java
index 3f8a6b5baa..2db6c3122d 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationFactory.java
@@ -84,7 +84,7 @@ public final class ConfigurationFactory {
       BuildConfigurationKey key, EventHandler errorEventListener)
           throws InvalidConfigurationException {
     return configurationCollectionFactory.createConfigurations(this,
-        loadedPackageProvider, buildOptions, key.getClientEnv(),
+        loadedPackageProvider, buildOptions, key.getTestEnv(),
         errorEventListener, performSanityCheck);
   }
 
@@ -94,10 +94,10 @@ public final class ConfigurationFactory {
    */
   @Nullable
   public BuildConfiguration getHostConfiguration(
-      PackageProviderForConfigurations loadedPackageProvider, Map<String, String> clientEnv,
+      PackageProviderForConfigurations loadedPackageProvider, Map<String, String> testEnv,
       BuildOptions buildOptions, boolean fallback) throws InvalidConfigurationException {
     return getConfiguration(loadedPackageProvider, buildOptions.createHostOptions(fallback),
-        clientEnv, false, hostConfigCache);
+        testEnv, false, hostConfigCache);
   }
 
   /**
@@ -106,7 +106,7 @@ public final class ConfigurationFactory {
    */
   @Nullable
   public BuildConfiguration getConfiguration(PackageProviderForConfigurations loadedPackageProvider,
-      BuildOptions buildOptions, Map<String, String> clientEnv,
+      BuildOptions buildOptions, Map<String, String> testEnv,
       boolean actionsDisabled, Cache<String, BuildConfiguration> cache)
           throws InvalidConfigurationException {
 
@@ -136,11 +136,11 @@ public final class ConfigurationFactory {
     fragments = ImmutableMap.copyOf(fragments);
 
     String key = BuildConfiguration.computeCacheKey(
-        directories, fragments, buildOptions, clientEnv);
+        directories, fragments, buildOptions, testEnv);
     BuildConfiguration configuration = cache.getIfPresent(key);
     if (configuration == null) {
       configuration = new BuildConfiguration(directories, fragments, buildOptions,
-          clientEnv, actionsDisabled);
+          testEnv, actionsDisabled);
       cache.put(key, configuration);
     }
     return configuration;
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java
index 1472b43043..eb3d1889f9 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java
@@ -58,7 +58,7 @@ public class BazelConfigurationCollection implements ConfigurationCollectionFact
       ConfigurationFactory configurationFactory,
       PackageProviderForConfigurations loadedPackageProvider,
       BuildOptions buildOptions,
-      Map<String, String> clientEnv,
+      Map<String, String> testEnv,
       EventHandler errorEventListener,
       boolean performSanityCheck) throws InvalidConfigurationException {
 
@@ -71,7 +71,7 @@ public class BazelConfigurationCollection implements ConfigurationCollectionFact
 
     // Target configuration
     BuildConfiguration targetConfiguration = configurationFactory.getConfiguration(
-        loadedPackageProvider, buildOptions, clientEnv, false, cache);
+        loadedPackageProvider, buildOptions, testEnv, false, cache);
     if (targetConfiguration == null) {
       return null;
     }
@@ -82,7 +82,7 @@ public class BazelConfigurationCollection implements ConfigurationCollectionFact
     // Note that this passes in the dataConfiguration, not the target
     // configuration. This is intentional.
     BuildConfiguration hostConfiguration = getHostConfigurationFromRequest(configurationFactory,
-        loadedPackageProvider, clientEnv, dataConfiguration, buildOptions);
+        loadedPackageProvider, testEnv, dataConfiguration, buildOptions);
     if (hostConfiguration == null) {
       return null;
     }
@@ -131,7 +131,7 @@ public class BazelConfigurationCollection implements ConfigurationCollectionFact
   @Nullable
   private BuildConfiguration getHostConfigurationFromRequest(
       ConfigurationFactory configurationFactory,
-      PackageProviderForConfigurations loadedPackageProvider, Map<String, String> clientEnv,
+      PackageProviderForConfigurations loadedPackageProvider, Map<String, String> testEnv,
       BuildConfiguration requestConfig, BuildOptions buildOptions)
       throws InvalidConfigurationException {
     BuildConfiguration.Options commonOptions = buildOptions.get(BuildConfiguration.Options.class);
@@ -139,7 +139,7 @@ public class BazelConfigurationCollection implements ConfigurationCollectionFact
       return requestConfig;
     } else {
       BuildConfiguration hostConfig = configurationFactory.getHostConfiguration(
-          loadedPackageProvider, clientEnv, buildOptions, /*fallback=*/false);
+          loadedPackageProvider, testEnv, buildOptions, /*fallback=*/false);
       if (hostConfig == null) {
         return null;
       }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java
index 8a6adc2e07..d7621d905d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java
@@ -59,8 +59,8 @@ public class StandaloneTestStrategy extends TestStrategy {
     */
 
   public StandaloneTestStrategy(OptionsClassProvider requestOptions,
-      OptionsClassProvider startupOptions, BinTools binTools) {
-    super(requestOptions, startupOptions, binTools);
+      OptionsClassProvider startupOptions, BinTools binTools, Map<String, String> clientEnv) {
+    super(requestOptions, startupOptions, binTools, clientEnv);
   }
 
   private static final String TEST_SETUP = "tools/test/test-setup.sh";
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/TestStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/test/TestStrategy.java
index 103ba62624..6442943ddd 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/TestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/TestStrategy.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.rules.test;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.io.ByteStreams;
 import com.google.common.io.Closeables;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
@@ -114,13 +115,16 @@ public abstract class TestStrategy implements TestActionContext {
   private static final String TEST_BRIDGE_TEST_FILTER_ENV = "TESTBRIDGE_TEST_ONLY";
 
   private final boolean statusServerRunning;
+  protected final ImmutableMap<String, String> clientEnv;
   protected final ExecutionOptions executionOptions;
   protected final BinTools binTools;
 
   public TestStrategy(OptionsClassProvider requestOptionsProvider,
-      OptionsClassProvider startupOptionsProvider, BinTools binTools) {
+      OptionsClassProvider startupOptionsProvider, BinTools binTools,
+      Map<String, String> clientEnv) {
     this.executionOptions = requestOptionsProvider.getOptions(ExecutionOptions.class);
     this.binTools = binTools;
+    this.clientEnv = ImmutableMap.copyOf(clientEnv);
     BlazeServerStartupOptions startupOptions =
         startupOptionsProvider.getOptions(BlazeServerStartupOptions.class);
     statusServerRunning = startupOptions != null && startupOptions.useWebStatusServer > 0;
@@ -209,7 +213,7 @@ public abstract class TestStrategy implements TestActionContext {
    */
   public Map<String, String> getAdmissibleShellEnvironment(BuildConfiguration config,
       Iterable<String> variables) {
-    return getMapping(variables, config.getClientEnv());
+    return getMapping(variables, clientEnv);
   }
 
   /*
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index 868f454579..2a0e87df96 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -225,6 +225,29 @@ public final class BlazeRuntime {
 
   private final ProjectFile.Provider projectFileProvider;
 
+  /**
+   * Returns user-specified test environment variables and their values, as
+   * set by the --test_env options.
+   *
+   * @param envOverrides The --test_env flag values.
+   * @param clientEnvironment The full client environment.
+   */
+  private static Map<String, String> computeTestEnv(List<Map.Entry<String, String>> envOverrides,
+      Map<String, String> clientEnvironment) {
+    Map<String, String> testEnv = new HashMap<>();
+    for (Map.Entry<String, String> var : envOverrides) {
+      if (var.getValue() != null) {
+        testEnv.put(var.getKey(), var.getValue());
+      } else {
+        String value = clientEnvironment.get(var.getKey());
+        if (value != null) {
+          testEnv.put(var.getKey(), value);
+        }
+      }
+    }
+    return testEnv;
+  }
+
   private class BlazeModuleEnvironment implements BlazeModule.ModuleEnvironment {
     @Override
     public Path getFileFromDepot(Label label)
@@ -961,7 +984,10 @@ public final class BlazeRuntime {
    */
   public BuildConfigurationKey getBuildConfigurationKey(BuildOptions buildOptions,
       ImmutableSortedSet<String> multiCpu) {
-    return new BuildConfigurationKey(buildOptions, directories, clientEnv, multiCpu);
+    Map<String, String> testEnv = computeTestEnv(
+        buildOptions.get(BuildConfiguration.Options.class).testEnvironment,
+        clientEnv);
+    return new BuildConfigurationKey(buildOptions, directories, testEnv, multiCpu);
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java
index 89828c3cda..085238d1cf 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java
@@ -44,15 +44,15 @@ import javax.annotation.Nullable;
 public class ConfigurationCollectionFunction implements SkyFunction {
 
   private final Supplier<ConfigurationFactory> configurationFactory;
-  private final Supplier<Map<String, String>> clientEnv;
+  private final Supplier<Map<String, String>> testEnv;
   private final Supplier<Set<Package>> configurationPackages;
 
   public ConfigurationCollectionFunction(
       Supplier<ConfigurationFactory> configurationFactory,
-      Supplier<Map<String, String>> clientEnv,
+      Supplier<Map<String, String>> testEnv,
       Supplier<Set<Package>> configurationPackages) {
     this.configurationFactory = configurationFactory;
-    this.clientEnv = clientEnv;
+    this.testEnv = testEnv;
     this.configurationPackages = configurationPackages;
   }
 
@@ -61,8 +61,6 @@ public class ConfigurationCollectionFunction implements SkyFunction {
       ConfigurationCollectionFunctionException {
     ConfigurationCollectionKey collectionKey = (ConfigurationCollectionKey) skyKey.argument();
     try {
-      // We are not using this value, because test_environment can be created from clientEnv. But
-      // we want ConfigurationCollection to be recomputed each time when test_environment changes.
       PrecomputedValue.TEST_ENVIRONMENT_VARIABLES.get(env);
       BlazeDirectories directories = PrecomputedValue.BLAZE_DIRECTORIES.get(env);
       if (env.valuesMissing()) {
@@ -72,7 +70,7 @@ public class ConfigurationCollectionFunction implements SkyFunction {
       BuildConfigurationCollection result =
           getConfigurations(env.getListener(),
           new SkyframePackageLoaderWithValueEnvironment(env, configurationPackages.get()),
-          new BuildConfigurationKey(collectionKey.getBuildOptions(), directories, clientEnv.get(),
+          new BuildConfigurationKey(collectionKey.getBuildOptions(), directories, testEnv.get(),
               collectionKey.getMultiCpu()));
 
       // BuildConfigurationCollection can be created, but dependencies to some files might be
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index e3e954f434..fd170a1f95 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -230,7 +230,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
       new SkyframeIncrementalBuildMonitor();
 
   private MutableSupplier<ConfigurationFactory> configurationFactory = new MutableSupplier<>();
-  private MutableSupplier<Map<String, String>> clientEnv = new MutableSupplier<>();
+  private MutableSupplier<Map<String, String>> testEnv = new MutableSupplier<>();
   private MutableSupplier<ImmutableList<ConfigurationFragmentFactory>> configurationFragments =
       new MutableSupplier<>();
   private MutableSupplier<Set<Package>> configurationPackages = new MutableSupplier<>();
@@ -316,7 +316,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     map.put(SkyFunctions.POST_CONFIGURED_TARGET,
         new PostConfiguredTargetFunction(new BuildViewProvider()));
     map.put(SkyFunctions.CONFIGURATION_COLLECTION, new ConfigurationCollectionFunction(
-        configurationFactory, clientEnv, configurationPackages));
+        configurationFactory, testEnv, configurationPackages));
     map.put(SkyFunctions.CONFIGURATION_FRAGMENT, new ConfigurationFragmentFunction(
         configurationFragments, configurationPackages));
     map.put(SkyFunctions.WORKSPACE_FILE, new WorkspaceFileFunction(pkgFactory, directories));
@@ -864,18 +864,14 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
       throws InvalidConfigurationException, InterruptedException {
 
     this.configurationPackages.set(Sets.<Package>newConcurrentHashSet());
-    this.clientEnv.set(configurationKey.getClientEnv());
+    this.testEnv.set(configurationKey.getTestEnv());
     this.configurationFactory.set(configurationFactory);
     this.configurationFragments.set(ImmutableList.copyOf(configurationFactory.getFactories()));
-    BuildOptions buildOptions = configurationKey.getBuildOptions();
-    Map<String, String> testEnv = BuildConfiguration.getTestEnv(
-        buildOptions.get(BuildConfiguration.Options.class).testEnvironment,
-        configurationKey.getClientEnv());
     // TODO(bazel-team): find a way to use only BuildConfigurationKey instead of
     // TestEnvironmentVariables and BlazeDirectories. There is a problem only with
     // TestEnvironmentVariables because BuildConfigurationKey stores client environment variables
     // and we don't want to rebuild everything when any variable changes.
-    PrecomputedValue.TEST_ENVIRONMENT_VARIABLES.set(injectable(), testEnv);
+    PrecomputedValue.TEST_ENVIRONMENT_VARIABLES.set(injectable(), configurationKey.getTestEnv());
     PrecomputedValue.BLAZE_DIRECTORIES.set(injectable(), configurationKey.getDirectories());
 
     SkyKey skyKey = ConfigurationCollectionValue.key(configurationKey.getBuildOptions(),
diff --git a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneContextProvider.java b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneContextProvider.java
index a70d1e109e..7a8789fb4c 100644
--- a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneContextProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneContextProvider.java
@@ -77,7 +77,7 @@ public class StandaloneContextProvider implements ActionContextProvider {
     this.runtime = runtime;
 
     TestActionContext testStrategy = new StandaloneTestStrategy(buildRequest,
-        runtime.getStartupOptionsProvider(), runtime.getBinTools());
+        runtime.getStartupOptionsProvider(), runtime.getBinTools(), runtime.getClientEnv());
     Builder<ActionContext> strategiesBuilder = ImmutableList.builder();
     // order of strategies passed to builder is significant - when there are many strategies that
     // could potentially be used and a spawnActionContext doesn't specify which one it wants, the