Skip to content

Commit

Permalink
absltest: do not fail tests on Python 3.12+ when no tests ran AND:
Browse files Browse the repository at this point in the history
- Either test filtering is used
- Sharding is used and the shard index > 0, i.e. only the first shard will fail when no tests ran on Python 3.12+.

Context: Python 3.12 unittest will now fail when no tests ran after the change from python/cpython#102051. Since `absltest` is built on top of `unittest`, it will follow this behavior change in Python 3.12.

However, when test filtering is used in `absltest`, often used via `bazel test --test_filter=<my_filter>`, the current user expectation is the `bazel test` command should NOT fail is at least one test ran. Since the test runner here has no visibility of the overall `bazel` invocation, we'll make the test not fail when test filtering is used via bazel's environment variable. This is the existing behavior before Python 3.12.

Also test absl-py on Python 3.12.

PiperOrigin-RevId: 565190992
  • Loading branch information
yilei authored and copybara-github committed Sep 14, 2023
1 parent f9281cb commit 6c95d4d
Show file tree
Hide file tree
Showing 7 changed files with 176 additions and 37 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
strategy:
fail-fast: false
matrix:
python-version: ["3.7", "3.8", "3.9", "3.10", "3.11"]
python-version: ["3.7", "3.8", "3.9", "3.10", "3.11", "3.12"]
os: [ubuntu-latest, macOS-latest, windows-latest]

steps:
Expand All @@ -30,6 +30,7 @@ jobs:
id: setup_python
with:
python-version: ${{ matrix.python-version }}
allow-prereleases: true

- name: Install virtualenv
run: |
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com).

* `absl-py` no longer supports Python 3.6. It has reached end-of-life for more
than a year now.
* Support Python 3.12.
* (logging) `logging.exception` can now take `exc_info` as argument, with
default value `True`. Prior to this change setting `exc_info` would raise
`KeyError`, this change fixes this behaviour.
Expand Down
12 changes: 11 additions & 1 deletion absl/testing/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,10 @@ py_test(
name = "tests/absltest_sharding_test",
size = "small",
srcs = ["tests/absltest_sharding_test.py"],
data = [":tests/absltest_sharding_test_helper"],
data = [
":tests/absltest_sharding_test_helper",
":tests/absltest_sharding_test_helper_no_tests",
],
python_version = "PY3",
srcs_version = "PY3",
deps = [
Expand All @@ -182,6 +185,13 @@ py_binary(
deps = [":absltest"],
)

py_binary(
name = "tests/absltest_sharding_test_helper_no_tests",
testonly = 1,
srcs = ["tests/absltest_sharding_test_helper_no_tests.py"],
deps = [":absltest"],
)

py_test(
name = "tests/absltest_test",
size = "small",
Expand Down
104 changes: 82 additions & 22 deletions absl/testing/absltest.py
Original file line number Diff line number Diff line change
Expand Up @@ -2317,8 +2317,7 @@ def get_default_xml_output_filename():
os.path.splitext(os.path.basename(sys.argv[0]))[0] + '.xml')


def _setup_filtering(argv):
# type: (MutableSequence[Text]) -> None
def _setup_filtering(argv: MutableSequence[str]) -> bool:
"""Implements the bazel test filtering protocol.
The following environment variable is used in this method:
Expand All @@ -2333,16 +2332,20 @@ def _setup_filtering(argv):
Args:
argv: the argv to mutate in-place.
Returns:
Whether test filtering is requested.
"""
test_filter = os.environ.get('TESTBRIDGE_TEST_ONLY')
if argv is None or not test_filter:
return
return False

filters = shlex.split(test_filter)
if sys.version_info[:2] >= (3, 7):
filters = ['-k=' + test_filter for test_filter in filters]

argv[1:1] = filters
return True


def _setup_test_runner_fail_fast(argv):
Expand All @@ -2369,8 +2372,9 @@ def _setup_test_runner_fail_fast(argv):
argv[1:1] = ['--failfast']


def _setup_sharding(custom_loader=None):
# type: (Optional[unittest.TestLoader]) -> unittest.TestLoader
def _setup_sharding(
custom_loader: Optional[unittest.TestLoader] = None,
) -> Tuple[unittest.TestLoader, Optional[int]]:
"""Implements the bazel sharding protocol.
The following environment variables are used in this method:
Expand All @@ -2389,8 +2393,10 @@ def _setup_sharding(custom_loader=None):
custom_loader: A TestLoader to be made sharded.
Returns:
The test loader for shard-filtering or the standard test loader, depending
on the sharding environment variables.
A tuple of ``(test_loader, shard_index)``. ``test_loader`` is for
shard-filtering or the standard test loader depending on the sharding
environment variables. ``shard_index`` is the shard index, or ``None`` when
sharding is not used.
"""

# It may be useful to write the shard file even if the other sharding
Expand All @@ -2408,7 +2414,7 @@ def _setup_sharding(custom_loader=None):
base_loader = custom_loader or TestLoader()
if 'TEST_TOTAL_SHARDS' not in os.environ:
# Not using sharding, use the expected test loader.
return base_loader
return base_loader, None

total_shards = int(os.environ['TEST_TOTAL_SHARDS'])
shard_index = int(os.environ['TEST_SHARD_INDEX'])
Expand Down Expand Up @@ -2437,25 +2443,70 @@ def getShardedTestCaseNames(testCaseClass):
return [x for x in ordered_names if x in filtered_names]

base_loader.getTestCaseNames = getShardedTestCaseNames
return base_loader
return base_loader, shard_index


def _run_and_get_tests_result(
argv: MutableSequence[str],
args: Sequence[Any],
kwargs: MutableMapping[str, Any],
xml_test_runner_class: Type[unittest.TextTestRunner],
) -> Tuple[unittest.TestResult, bool]:
"""Same as run_tests, but it doesn't exit.
Args:
argv: sys.argv with the command-line flags removed from the front, i.e. the
argv with which :func:`app.run()<absl.app.run>` has called
``__main__.main``. It is passed to
``unittest.TestProgram.__init__(argv=)``, which does its own flag parsing.
It is ignored if kwargs contains an argv entry.
args: Positional arguments passed through to
``unittest.TestProgram.__init__``.
kwargs: Keyword arguments passed through to
``unittest.TestProgram.__init__``.
xml_test_runner_class: The type of the test runner class.
# pylint: disable=line-too-long
def _run_and_get_tests_result(argv, args, kwargs, xml_test_runner_class):
# type: (MutableSequence[Text], Sequence[Any], MutableMapping[Text, Any], Type) -> unittest.TestResult
# pylint: enable=line-too-long
"""Same as run_tests, except it returns the result instead of exiting."""
Returns:
A tuple of ``(test_result, fail_when_no_tests_ran)``.
``fail_when_no_tests_ran`` indicates whether the test should fail when
no tests ran.
"""

# The entry from kwargs overrides argv.
argv = kwargs.pop('argv', argv)

if sys.version_info[:2] >= (3, 12):
# Python 3.12 unittest changed the behavior from PASS to FAIL in
# https://github.com/python/cpython/pull/102051. absltest follows this.
fail_when_no_tests_ran = True
else:
# Historically, absltest and unittest before Python 3.12 passes if no tests
# ran.
fail_when_no_tests_ran = False

# Set up test filtering if requested in environment.
_setup_filtering(argv)
if _setup_filtering(argv):
# When test filtering is requested, ideally we also want to fail when no
# tests ran. However, the test filters are usually done when running bazel.
# When you run multiple targets, e.g. `bazel test //my_dir/...
# --test_filter=MyTest`, you don't necessarily want individual tests to fail
# because no tests match in that particular target.
# Due to this use case, we don't fail when test filtering is requested via
# the environment variable from bazel.
fail_when_no_tests_ran = False

# Set up --failfast as requested in environment
_setup_test_runner_fail_fast(argv)

# Shard the (default or custom) loader if sharding is turned on.
kwargs['testLoader'] = _setup_sharding(kwargs.get('testLoader', None))
kwargs['testLoader'], shard_index = _setup_sharding(
kwargs.get('testLoader', None)
)
if shard_index is not None and shard_index > 0:
# When sharding is requested, all the shards except the first one shall not
# fail when no tests ran. This happens when the shard count is greater than
# the test case count.
fail_when_no_tests_ran = False

# XML file name is based upon (sorted by priority):
# --xml_output_file flag, XML_OUTPUT_FILE variable,
Expand Down Expand Up @@ -2533,9 +2584,13 @@ def _run_and_get_tests_result(argv, args, kwargs, xml_test_runner_class):
# on argv, which is sys.argv without the command-line flags.
kwargs['argv'] = argv

# Request unittest.TestProgram to not exit. The exit will be handled by
# `absltest.run_tests`.
kwargs['exit'] = False

try:
test_program = unittest.TestProgram(*args, **kwargs)
return test_program.result
return test_program.result, fail_when_no_tests_ran
finally:
if xml_buffer:
try:
Expand All @@ -2545,9 +2600,11 @@ def _run_and_get_tests_result(argv, args, kwargs, xml_test_runner_class):
xml_buffer.close()


def run_tests(argv, args, kwargs): # pylint: disable=line-too-long
# type: (MutableSequence[Text], Sequence[Any], MutableMapping[Text, Any]) -> None
# pylint: enable=line-too-long
def run_tests(
argv: MutableSequence[Text],
args: Sequence[Any],
kwargs: MutableMapping[Text, Any],
) -> None:
"""Executes a set of Python unit tests.
Most users should call absltest.main() instead of run_tests.
Expand All @@ -2568,8 +2625,11 @@ def run_tests(argv, args, kwargs): # pylint: disable=line-too-long
kwargs: Keyword arguments passed through to
``unittest.TestProgram.__init__``.
"""
result = _run_and_get_tests_result(
argv, args, kwargs, xml_reporter.TextAndXMLTestRunner)
result, fail_when_no_tests_ran = _run_and_get_tests_result(
argv, args, kwargs, xml_reporter.TextAndXMLTestRunner
)
if fail_when_no_tests_ran and result.testsRun == 0:
sys.exit(5)
sys.exit(not result.wasSuccessful())


Expand Down
7 changes: 6 additions & 1 deletion absl/testing/tests/absltest_filtering_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,12 @@ def test_not_found_filters_py36(self, use_env_variable, use_app_run):
def test_not_found_filters_py37(self, use_env_variable, use_app_run):
out, exit_code = self._run_filtered('NotExistedClass.not_existed_method',
use_env_variable, use_app_run)
self.assertEqual(0, exit_code)
if not use_env_variable and sys.version_info[:2] >= (3, 12):
# When test filter is requested with the unittest `-k` flag, absltest
# respect unittest to fail when no tests run on Python 3.12+.
self.assertEqual(5, exit_code)
else:
self.assertEqual(0, exit_code)
self.assertIn('Ran 0 tests', out)

@absltest.skipIf(
Expand Down
61 changes: 49 additions & 12 deletions absl/testing/tests/absltest_sharding_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,45 +16,50 @@

import os
import subprocess
import sys

from absl.testing import _bazelize_command
from absl.testing import absltest
from absl.testing import parameterized
from absl.testing.tests import absltest_env


NUM_TEST_METHODS = 8 # Hard-coded, based on absltest_sharding_test_helper.py


class TestShardingTest(absltest.TestCase):
class TestShardingTest(parameterized.TestCase):
"""Integration tests: Runs a test binary with sharding.
This is done by setting the sharding environment variables.
"""

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,
additional_env=None):
def _run_sharded(
self,
total_shards,
shard_index,
shard_file=None,
additional_env=None,
helper_name='absltest_sharding_test_helper',
):
"""Runs the py_test binary in a subprocess.
Args:
total_shards: int, the total number of shards.
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.
shard_file: string, if not 'None', the path to the shard file. This method
asserts it is properly created.
additional_env: Additional environment variables to be set for the py_test
binary.
helper_name: The name of the helper binary.
Returns:
(stdout, exit_code) tuple of (string, int).
Expand All @@ -72,12 +77,14 @@ def _run_sharded(self,
if os.path.exists(shard_file):
os.unlink(shard_file)

helper = 'absl/testing/tests/' + helper_name
proc = subprocess.Popen(
args=[_bazelize_command.get_executable_path(self._test_name)],
args=[_bazelize_command.get_executable_path(helper)],
env=env,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
universal_newlines=True)
universal_newlines=True,
)
stdout = proc.communicate()[0]

if shard_file:
Expand Down Expand Up @@ -140,7 +147,11 @@ def test_with_one_shard(self):
self._assert_sharding_correctness(1)

def test_with_ten_shards(self):
self._assert_sharding_correctness(10)
shards = 10
# This test relies on the shard count to be greater than the number of
# tests.
self.assertGreater(NUM_TEST_METHODS, shards)
self._assert_sharding_correctness(shards)

def test_sharding_with_randomization(self):
# If we're both sharding *and* randomizing, we need to confirm that we
Expand All @@ -156,6 +167,32 @@ def test_sharding_with_randomization(self):
self.assertEqual(set(first_tests), set(second_tests))
self.assertNotEqual(first_tests, second_tests)

@parameterized.named_parameters(
('total_1_index_0', 1, 0, None),
('total_2_index_0', 2, 0, None),
# The 2nd shard (index=1) should not fail.
('total_2_index_1', 2, 1, 0),
)
def test_no_tests_ran(
self, total_shards, shard_index, override_expected_exit_code
):
if override_expected_exit_code is not None:
expected_exit_code = override_expected_exit_code
elif sys.version_info[:2] >= (3, 12):
expected_exit_code = 5
else:
expected_exit_code = 0
out, exit_code = self._run_sharded(
total_shards,
shard_index,
helper_name='absltest_sharding_test_helper_no_tests',
)
self.assertEqual(
expected_exit_code,
exit_code,
'Unexpected exit code, output:\n{}'.format(out),
)


if __name__ == '__main__':
absltest.main()
Loading

0 comments on commit 6c95d4d

Please sign in to comment.