When running a absltest helper binary in absltest's own tests, also inherit the `PATH` variable.

The `PATH` variable is needed to correctly run the Python version determined by bazel, since the `py_binary` uses `#!/usr/bin/env python` to run it.

PiperOrigin-RevId: 401540949
Change-Id: Ic2aefcb5d8168117ade840c91ea1051f0a28a809

7 files changed, 65 insertions, 29 deletions

diff --git a/absl/testing/BUILD b/absl/testing/BUILD
index 6e0164a..609d626 100644
--- a/absl/testing/BUILD
+++ b/absl/testing/BUILD
@@ -98,6 +98,12 @@ py_library(
     ],
 )
 
+py_library(
+    name = "tests/absltest_env",
+    testonly = True,
+    srcs = ["tests/absltest_env.py"],
+)
+
 py_test(
     name = "tests/absltest_filtering_test",
     size = "small",
@@ -109,6 +115,7 @@ py_test(
         ":_bazelize_command",
         ":absltest",
         ":parameterized",
+        ":tests/absltest_env",
         "//absl/logging",
     ],
 )
@@ -136,6 +143,7 @@ py_test(
         ":_bazelize_command",
         ":absltest",
         ":parameterized",
+        ":tests/absltest_env",
         "//absl/logging",
     ],
 )
@@ -163,6 +171,7 @@ py_test(
         ":_bazelize_command",
         ":absltest",
         ":parameterized",
+        ":tests/absltest_env",
         "//absl/flags",
     ],
 )
@@ -188,7 +197,7 @@ py_test(
     deps = [
         ":_bazelize_command",
         ":absltest",
-        "//absl/flags",
+        ":tests/absltest_env",
     ],
 )
 
@@ -212,7 +221,7 @@ py_test(
         ":_bazelize_command",
         ":absltest",
         ":parameterized",
-        "//absl/flags",
+        ":tests/absltest_env",
         "@six_archive//:six",
     ],
 )
diff --git a/absl/testing/tests/absltest_env.py b/absl/testing/tests/absltest_env.py
new file mode 100644
index 0000000..c913785
--- /dev/null
+++ b/absl/testing/tests/absltest_env.py
@@ -0,0 +1,30 @@
+"""Helper library to get environment variables for absltest helper binaries."""
+
+import os
+
+
+_INHERITED_ENV_KEYS = frozenset({
+    # This is needed to correctly use the Python interpreter determined by
+    # bazel.
+    'PATH',
+    # This is used by the random module on Windows to locate crypto
+    # libraries.
+    'SYSTEMROOT',
+})
+
+
+def inherited_env():
+  """Returns the environment variables that should be inherited from parent.
+
+  Reason why using an explict list of environment variables instead of
+  inheriting all from parent: the absltest module itself interprets a list of
+  environment variables set by bazel, e.g. XML_OUTPUT_FILE,
+  TESTBRIDGE_TEST_ONLY. While testing absltest's own behavior, we should
+  remove them when invoking the helper subprocess. Using an explicit list is
+  safer.
+  """
+  env = {}
+  for key in _INHERITED_ENV_KEYS:
+    if key in os.environ:
+      env[key] = os.environ[key]
+  return env
diff --git a/absl/testing/tests/absltest_fail_fast_test.py b/absl/testing/tests/absltest_fail_fast_test.py
index e8ca1d6..dc967f9 100644
--- a/absl/testing/tests/absltest_fail_fast_test.py
+++ b/absl/testing/tests/absltest_fail_fast_test.py
@@ -18,13 +18,13 @@ from __future__ import absolute_import
 from __future__ import division
 from __future__ import print_function
 
-import os
 import subprocess
 
 from absl import logging
 from absl.testing import _bazelize_command
 from absl.testing import absltest
 from absl.testing import parameterized
+from absl.testing.tests import absltest_env
 
 
 @parameterized.named_parameters(
@@ -52,19 +52,13 @@ class TestFailFastTest(parameterized.TestCase):
     Returns:
       (stdout, exit_code) tuple of (string, int).
     """
-    env = {}
-    if 'SYSTEMROOT' in os.environ:
-      # This is used by the random module on Windows to locate crypto
-      # libraries.
-      env['SYSTEMROOT'] = os.environ['SYSTEMROOT']
-    additional_args = []
+    env = absltest_env.inherited_env()
     if fail_fast is not None:
       env['TESTBRIDGE_TEST_RUNNER_FAIL_FAST'] = fail_fast
     env['USE_APP_RUN'] = '1' if use_app_run else '0'
 
     proc = subprocess.Popen(
-        args=([_bazelize_command.get_executable_path(self._test_name)]
-              + additional_args),
+        args=[_bazelize_command.get_executable_path(self._test_name)],
         env=env,
         stdout=subprocess.PIPE,
         stderr=subprocess.STDOUT,
diff --git a/absl/testing/tests/absltest_filtering_test.py b/absl/testing/tests/absltest_filtering_test.py
index 964a0b4..63e3915 100644
--- a/absl/testing/tests/absltest_filtering_test.py
+++ b/absl/testing/tests/absltest_filtering_test.py
@@ -17,13 +17,13 @@ from __future__ import absolute_import
 from __future__ import division
 from __future__ import print_function
 
-import os
 import subprocess
 
 from absl import logging
 from absl.testing import _bazelize_command
 from absl.testing import absltest
 from absl.testing import parameterized
+from absl.testing.tests import absltest_env
 
 
 @parameterized.named_parameters(
@@ -56,11 +56,7 @@ class TestFilteringTest(absltest.TestCase):
     Returns:
       (stdout, exit_code) tuple of (string, int).
     """
-    env = {}
-    if 'SYSTEMROOT' in os.environ:
-      # This is used by the random module on Windows to locate crypto
-      # libraries.
-      env['SYSTEMROOT'] = os.environ['SYSTEMROOT']
+    env = absltest_env.inherited_env()
     env['USE_APP_RUN'] = '1' if use_app_run else '0'
     additional_args = []
     if test_filter is not None:
diff --git a/absl/testing/tests/absltest_randomization_test.py b/absl/testing/tests/absltest_randomization_test.py
index 16b330c..75a3868 100644
--- a/absl/testing/tests/absltest_randomization_test.py
+++ b/absl/testing/tests/absltest_randomization_test.py
@@ -18,7 +18,6 @@ from __future__ import absolute_import
 from __future__ import division
 from __future__ import print_function
 
-import os
 import random
 import subprocess
 
@@ -26,6 +25,7 @@ from absl import flags
 from absl.testing import _bazelize_command
 from absl.testing import absltest
 from absl.testing import parameterized
+from absl.testing.tests import absltest_env
 
 FLAGS = flags.FLAGS
 
@@ -50,7 +50,7 @@ class TestOrderRandomizationTest(parameterized.TestCase):
     Returns:
       (stdout, test_cases, exit_code) tuple of (str, list of strs, int).
     """
-    env = dict(os.environ)
+    env = absltest_env.inherited_env()
     # If *this* test is being run with this flag, we don't want to
     # automatically set it for all tests we run.
     env.pop('TEST_RANDOMIZE_ORDERING_SEED', '')
diff --git a/absl/testing/tests/absltest_sharding_test.py b/absl/testing/tests/absltest_sharding_test.py
index 7edc2b2..6411971 100644
--- a/absl/testing/tests/absltest_sharding_test.py
+++ b/absl/testing/tests/absltest_sharding_test.py
@@ -23,6 +23,8 @@ import subprocess
 
 from absl.testing import _bazelize_command
 from absl.testing import absltest
+from absl.testing.tests import absltest_env
+
 
 NUM_TEST_METHODS = 8  # Hard-coded, based on absltest_sharding_test_helper.py
 
@@ -34,14 +36,20 @@ class TestShardingTest(absltest.TestCase):
   """
 
   def setUp(self):
+    super().setUp()
     self._test_name = 'absl/testing/tests/absltest_sharding_test_helper'
     self._shard_file = None
 
   def tearDown(self):
+    super().tearDown()
     if self._shard_file is not None and os.path.exists(self._shard_file):
       os.unlink(self._shard_file)
 
-  def _run_sharded(self, total_shards, shard_index, shard_file=None, env=None):
+  def _run_sharded(self,
+                   total_shards,
+                   shard_index,
+                   shard_file=None,
+                   additional_env=None):
     """Runs the py_test binary in a subprocess.
 
     Args:
@@ -49,21 +57,19 @@ class TestShardingTest(absltest.TestCase):
       shard_index: int, the shard index.
       shard_file: string, if not 'None', the path to the shard file.
         This method asserts it is properly created.
-      env: Environment variables to be set for the py_test binary.
+      additional_env: Additional environment variables to be set for the py_test
+        binary.
 
     Returns:
       (stdout, exit_code) tuple of (string, int).
     """
-    if env is None:
-      env = {}
+    env = absltest_env.inherited_env()
+    if additional_env:
+      env.update(additional_env)
     env.update({
         'TEST_TOTAL_SHARDS': str(total_shards),
         'TEST_SHARD_INDEX': str(shard_index)
     })
-    if 'SYSTEMROOT' in os.environ:
-      # This is used by the random module on Windows to locate crypto
-      # libraries.
-      env['SYSTEMROOT'] = os.environ['SYSTEMROOT']
     if shard_file:
       self._shard_file = shard_file
       env['TEST_SHARD_STATUS_FILE'] = shard_file
@@ -147,7 +153,7 @@ class TestShardingTest(absltest.TestCase):
     tests_seen = []
     for seed in ('7', '17'):
       out, exit_code = self._run_sharded(
-          2, 0, env={'TEST_RANDOMIZE_ORDERING_SEED': seed})
+          2, 0, additional_env={'TEST_RANDOMIZE_ORDERING_SEED': seed})
       self.assertEqual(0, exit_code)
       tests_seen.append([x for x in out.splitlines() if x.startswith('class')])
     first_tests, second_tests = tests_seen  # pylint: disable=unbalanced-tuple-unpacking
diff --git a/absl/testing/tests/absltest_test.py b/absl/testing/tests/absltest_test.py
index a54ff55..6e8503c 100644
--- a/absl/testing/tests/absltest_test.py
+++ b/absl/testing/tests/absltest_test.py
@@ -33,6 +33,7 @@ import unittest
 from absl.testing import _bazelize_command
 from absl.testing import absltest
 from absl.testing import parameterized
+from absl.testing.tests import absltest_env
 import six
 
 try:
@@ -50,7 +51,7 @@ class HelperMixin(object):
     return _bazelize_command.get_executable_path(helper)
 
   def run_helper(self, test_id, args, env_overrides, expect_success):
-    env = os.environ.copy()
+    env = absltest_env.inherited_env()
     for key, value in six.iteritems(env_overrides):
       if value is None:
         if key in env: