Pass in the test environment using BuildConfiguration.Options.testEnvironment instead of special-casing it in a large number of classes.

The variables in the client environment are read in BlazeRuntime#beforeCommand() now.

Note that this entails a slight loss of caching: before, "--test_env=a=A,b=B" and "--test_env=b=B,a=A" were equivalent, now they are not, since instead of comparing Map<String, String>, List<Map.Entry<String,String>> instances are compared.

--
MOS_MIGRATED_REVID=91570828

3 files changed, 3 insertions, 16 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
index 3ce610b223..bec1477c48 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
@@ -259,7 +259,7 @@ public abstract class AnalysisTestCase extends FoundationTestCase {
     BuildRequestOptions requestOptions = optionsParser.getOptions(BuildRequestOptions.class);
     ImmutableSortedSet<String> multiCpu = ImmutableSortedSet.copyOf(requestOptions.multiCpus);
     BuildConfigurationKey configurationKey = new BuildConfigurationKey(
-        buildOptions, directories, ImmutableMap.<String, String>of(), multiCpu);
+        buildOptions, directories, multiCpu);
     masterConfig = skyframeExecutor.createConfigurations(configurationFactory, configurationKey);
     analysisResult = buildView.update(loadingResult, masterConfig, viewOptions,
         AnalysisTestUtil.TOP_LEVEL_ARTIFACT_CONTEXT, reporter, eventBus);
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index 63199a9064..5f22d938c6 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -133,10 +133,8 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
 
@@ -251,19 +249,10 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
       optionsParser.parse(configurationArgs);
       optionsParser.parse(args);
 
-      // --test_env is a special snowflake: even though it is in BuildConfiguration.Options, it is
-      // only used in BlazeRuntime to compute the actual test environment, which is passed into
-      // BuildConfiguration using its own very special data path. Thus, we need to special-case it
-      // here until it is handled in a more consistent way.
-      Map<String, String> testEnv = new LinkedHashMap<>();
-      for (Map.Entry<String, String> entry :
-          optionsParser.getOptions(BuildConfiguration.Options.class).testEnvironment) {
-        testEnv.put(entry.getKey(), entry.getValue());
-      }
       configurationFactory.forbidSanityCheck();
       BuildOptions buildOptions = ruleClassProvider.createBuildOptions(optionsParser);
       ensureTargetsVisited(buildOptions.getAllLabels().values());
-      BuildConfigurationKey key = new BuildConfigurationKey(buildOptions, directories, testEnv);
+      BuildConfigurationKey key = new BuildConfigurationKey(buildOptions, directories);
       skyframeExecutor.invalidateConfigurationCollection();
       return skyframeExecutor.createConfigurations(configurationFactory, key);
     } catch (InvalidConfigurationException | OptionsParsingException e) {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java
index b65837adac..f64ee44f9b 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java
@@ -150,9 +150,7 @@ public abstract class ConfigurationTestCase extends FoundationTestCase {
     BuildConfigurationCollection collection = skyframeExecutor.createConfigurations(
         configurationFactory,
         new BuildConfigurationKey(buildOptions,
-        new BlazeDirectories(outputBase, outputBase, workspace),
-        ImmutableMap.<String, String>of(),
-        multiCpu));
+        new BlazeDirectories(outputBase, outputBase, workspace), multiCpu));
     return collection;
   }