From d980b45191335c4a7eedc267f6dfc4acffe97764 Mon Sep 17 00:00:00 2001 From: Haowei Wu Date: Thu, 21 Sep 2023 13:51:12 -0700 Subject: [PATCH 1/3] [unittest] Add option to allow disabling sharding in unittest By default, googletest based unit tests uses sharding to speed up the testing. However, when these unit tests are through wrapper program on a remote platform with large round trip time, the sharding will increase the time cost dramatically. This patch adds an "--disable-gtest-sharding" option in the LLVM LIT to disable sharding on googletest based unittests. --- llvm/utils/lit/lit/LitConfig.py | 2 + llvm/utils/lit/lit/cl_arguments.py | 6 + llvm/utils/lit/lit/formats/googletest.py | 86 +++++++++----- llvm/utils/lit/lit/main.py | 1 + .../DummySubDir/OneTest.py | 105 ++++++++++++++++++ .../Inputs/googletest-no-sharding/lit.cfg | 4 + .../utils/lit/tests/googletest-no-sharding.py | 43 +++++++ 7 files changed, 221 insertions(+), 26 deletions(-) create mode 100644 llvm/utils/lit/tests/Inputs/googletest-no-sharding/DummySubDir/OneTest.py create mode 100644 llvm/utils/lit/tests/Inputs/googletest-no-sharding/lit.cfg create mode 100644 llvm/utils/lit/tests/googletest-no-sharding.py diff --git a/llvm/utils/lit/lit/LitConfig.py b/llvm/utils/lit/lit/LitConfig.py index d7e79b60f385b..c7703f15f9e3f 100644 --- a/llvm/utils/lit/lit/LitConfig.py +++ b/llvm/utils/lit/lit/LitConfig.py @@ -37,6 +37,7 @@ def __init__( maxIndividualTestTime=0, parallelism_groups={}, per_test_coverage=False, + disableGTestSharding=False, ): # The name of the test runner. self.progname = progname @@ -87,6 +88,7 @@ def __init__( self.maxIndividualTestTime = maxIndividualTestTime self.parallelism_groups = parallelism_groups self.per_test_coverage = per_test_coverage + self.disableGTestSharding = bool(disableGTestSharding) @property def maxIndividualTestTime(self): diff --git a/llvm/utils/lit/lit/cl_arguments.py b/llvm/utils/lit/lit/cl_arguments.py index 132476fb2a367..7f12f833afe59 100644 --- a/llvm/utils/lit/lit/cl_arguments.py +++ b/llvm/utils/lit/lit/cl_arguments.py @@ -118,6 +118,12 @@ def parse_args(): ) execution_group = parser.add_argument_group("Test Execution") + execution_group.add_argument( + "--disable-gtest-sharding", + dest="disableGTestSharding", + help="Disable sharding for GoogleTest format", + action="store_true", + ) execution_group.add_argument( "--path", help="Additional paths to add to testing environment", diff --git a/llvm/utils/lit/lit/formats/googletest.py b/llvm/utils/lit/lit/formats/googletest.py index f8304cbd05453..7c427e0106ae5 100644 --- a/llvm/utils/lit/lit/formats/googletest.py +++ b/llvm/utils/lit/lit/formats/googletest.py @@ -68,24 +68,49 @@ def getTestsInDirectory(self, testSuite, path_in_suite, litConfig, localConfig): self.seen_executables.add(execpath) num_tests = self.get_num_tests(execpath, litConfig, localConfig) if num_tests is not None: - # Compute the number of shards. - shard_size = init_shard_size - nshard = int(math.ceil(num_tests / shard_size)) - while nshard < core_count and shard_size > 1: - shard_size = shard_size // 2 + if not litConfig.disableGTestSharding: + # Compute the number of shards. + shard_size = init_shard_size nshard = int(math.ceil(num_tests / shard_size)) - - # Create one lit test for each shard. - for idx in range(nshard): - testPath = path_in_suite + (subdir, fn, str(idx), str(nshard)) + while nshard < core_count and shard_size > 1: + shard_size = shard_size // 2 + nshard = int(math.ceil(num_tests / shard_size)) + + # Create one lit test for each shard. + for idx in range(nshard): + testPath = path_in_suite + ( + subdir, + fn, + str(idx), + str(nshard), + ) + json_file = ( + "-".join( + [ + execpath, + testSuite.config.name, + str(os.getpid()), + str(idx), + str(nshard), + ] + ) + + ".json" + ) + yield lit.Test.Test( + testSuite, + testPath, + localConfig, + file_path=execpath, + gtest_json_file=json_file, + ) + else: + testPath = path_in_suite + (subdir, fn) json_file = ( "-".join( [ execpath, testSuite.config.name, str(os.getpid()), - str(idx), - str(nshard), ] ) + ".json" @@ -118,24 +143,33 @@ def execute(self, test, litConfig): if test.gtest_json_file is None: return lit.Test.FAIL, "" - testPath, testName = os.path.split(test.getSourcePath()) - while not os.path.exists(testPath): - # Handle GTest parametrized and typed tests, whose name includes - # some '/'s. - testPath, namePrefix = os.path.split(testPath) - testName = namePrefix + "/" + testName - - testName, total_shards = os.path.split(testName) - testName, shard_idx = os.path.split(testName) + testPath = test.getSourcePath() from lit.cl_arguments import TestOrder use_shuffle = TestOrder(litConfig.order) == TestOrder.RANDOM - shard_env = { - "GTEST_OUTPUT": "json:" + test.gtest_json_file, - "GTEST_SHUFFLE": "1" if use_shuffle else "0", - "GTEST_TOTAL_SHARDS": os.environ.get("GTEST_TOTAL_SHARDS", total_shards), - "GTEST_SHARD_INDEX": os.environ.get("GTEST_SHARD_INDEX", shard_idx), - } + if not litConfig.disableGTestSharding: + testPath, testName = os.path.split(test.getSourcePath()) + while not os.path.exists(testPath): + # Handle GTest parametrized and typed tests, whose name includes + # some '/'s. + testPath, namePrefix = os.path.split(testPath) + testName = namePrefix + "/" + testName + + testName, total_shards = os.path.split(testName) + testName, shard_idx = os.path.split(testName) + shard_env = { + "GTEST_OUTPUT": "json:" + test.gtest_json_file, + "GTEST_SHUFFLE": "1" if use_shuffle else "0", + "GTEST_TOTAL_SHARDS": os.environ.get( + "GTEST_TOTAL_SHARDS", total_shards + ), + "GTEST_SHARD_INDEX": os.environ.get("GTEST_SHARD_INDEX", shard_idx), + } + else: + shard_env = { + "GTEST_OUTPUT": "json:" + test.gtest_json_file, + "GTEST_SHUFFLE": "1" if use_shuffle else "0", + } test.config.environment.update(shard_env) cmd = [testPath] diff --git a/llvm/utils/lit/lit/main.py b/llvm/utils/lit/lit/main.py index b2a578e0ae6d9..4580dbc966679 100755 --- a/llvm/utils/lit/lit/main.py +++ b/llvm/utils/lit/lit/main.py @@ -41,6 +41,7 @@ def main(builtin_params={}): params=params, config_prefix=opts.configPrefix, per_test_coverage=opts.per_test_coverage, + disableGTestSharding=opts.disableGTestSharding, ) discovered_tests = lit.discovery.find_tests_for_inputs( diff --git a/llvm/utils/lit/tests/Inputs/googletest-no-sharding/DummySubDir/OneTest.py b/llvm/utils/lit/tests/Inputs/googletest-no-sharding/DummySubDir/OneTest.py new file mode 100644 index 0000000000000..8fcab0081a212 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/googletest-no-sharding/DummySubDir/OneTest.py @@ -0,0 +1,105 @@ +#!/usr/bin/env python + +import os +import sys + +if len(sys.argv) == 3 and sys.argv[1] == "--gtest_list_tests": + if sys.argv[2] != "--gtest_filter=-*DISABLED_*": + raise ValueError("unexpected argument: %s" % (sys.argv[2])) + print( + """\ +FirstTest. + subTestA + subTestB + subTestC + subTestD +ParameterizedTest/0. + subTest +ParameterizedTest/1. + subTest""" + ) + sys.exit(0) +elif len(sys.argv) != 1: + # sharding and json output are specified using environment variables + raise ValueError("unexpected argument: %r" % (" ".join(sys.argv[1:]))) + +for e in ["GTEST_OUTPUT"]: + if e not in os.environ: + raise ValueError("missing environment variables: " + e) + +if not os.environ["GTEST_OUTPUT"].startswith("json:"): + raise ValueError("must emit json output: " + os.environ["GTEST_OUTPUT"]) + +output = """\ +{ +"random_seed": 123, +"testsuites": [ + { + "name": "FirstTest", + "testsuite": [ + { + "name": "subTestA", + "result": "COMPLETED", + "time": "0.001s" + }, + { + "name": "subTestB", + "result": "COMPLETED", + "time": "0.001s", + "failures": [ + { + "failure": "I am subTest B, I FAIL\\nAnd I have two lines of output", + "type": "" + } + ] + }, + { + "name": "subTestC", + "result": "SKIPPED", + "time": "0.001s" + }, + { + "name": "subTestD", + "result": "UNRESOLVED", + "time": "0.001s" + } + ] + }, + { + "name": "ParameterizedTest/0", + "testsuite": [ + { + "name": "subTest", + "result": "COMPLETED", + "time": "0.001s" + } + ] + }, + { + "name": "ParameterizedTest/1", + "testsuite": [ + { + "name": "subTest", + "result": "COMPLETED", + "time": "0.001s" + } + ] + } +] +}""" + +dummy_output = """\ +{ +"testsuites": [ +] +}""" + +json_filename = os.environ["GTEST_OUTPUT"].split(":", 1)[1] +with open(json_filename, "w", encoding="utf-8") as f: + print("[ RUN ] FirstTest.subTestB", flush=True) + print("I am subTest B output", file=sys.stderr, flush=True) + print("[ FAILED ] FirstTest.subTestB (8 ms)", flush=True) + f.write(output) + exit_code = 1 + +sys.exit(exit_code) diff --git a/llvm/utils/lit/tests/Inputs/googletest-no-sharding/lit.cfg b/llvm/utils/lit/tests/Inputs/googletest-no-sharding/lit.cfg new file mode 100644 index 0000000000000..fd75709efad3c --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/googletest-no-sharding/lit.cfg @@ -0,0 +1,4 @@ +import lit.formats + +config.name = "googletest-no-sharding" +config.test_format = lit.formats.GoogleTest("DummySubDir", "Test") diff --git a/llvm/utils/lit/tests/googletest-no-sharding.py b/llvm/utils/lit/tests/googletest-no-sharding.py new file mode 100644 index 0000000000000..ccf2fe0d9d31d --- /dev/null +++ b/llvm/utils/lit/tests/googletest-no-sharding.py @@ -0,0 +1,43 @@ +# Check the various features of the GoogleTest format. + +# RUN: not %{lit} -v --disable-gtest-sharding --order=random %{inputs}/googletest-no-sharding > %t.out +# FIXME: Temporarily dump test output so we can debug failing tests on +# buildbots. +# RUN: cat %t.out +# RUN: FileCheck < %t.out %s +# +# END. + +# CHECK: -- Testing: +# CHECK: FAIL: googletest-no-sharding :: [[PATH:[Dd]ummy[Ss]ub[Dd]ir/]][[FILE:OneTest\.py]] +# CHECK: *** TEST 'googletest-no-sharding :: [[PATH]][[FILE]]' FAILED *** +# CHECK-NEXT: Script(shard): +# CHECK-NEXT: -- +# CHECK-NEXT: GTEST_OUTPUT=json:{{[^[:space:]]*}} GTEST_SHUFFLE=1 GTEST_RANDOM_SEED=123 {{.*}}[[FILE]] +# CHECK-NEXT: -- +# CHECK-EMPTY: +# CHECK-NEXT: Script: +# CHECK-NEXT: -- +# CHECK-NEXT: [[FILE]] --gtest_filter=FirstTest.subTestB +# CHECK-NEXT: -- +# CHECK-NEXT: I am subTest B output +# CHECK-EMPTY: +# CHECK-NEXT: I am subTest B, I FAIL +# CHECK-NEXT: And I have two lines of output +# CHECK-EMPTY: +# CHECK: Script: +# CHECK-NEXT: -- +# CHECK-NEXT: [[FILE]] --gtest_filter=FirstTest.subTestD +# CHECK-NEXT: -- +# CHECK-NEXT: unresolved test result +# CHECK: *** +# CHECK: *** +# CHECK: Unresolved Tests (1): +# CHECK-NEXT: googletest-no-sharding :: FirstTest/subTestD +# CHECK: *** +# CHECK-NEXT: Failed Tests (1): +# CHECK-NEXT: googletest-no-sharding :: FirstTest/subTestB +# CHECK: Skipped{{ *}}: 1 +# CHECK: Passed{{ *}}: 3 +# CHECK: Unresolved{{ *}}: 1 +# CHECK: Failed{{ *}}: 1 From 0f42e939b950b1a7b6e2646eb7ee0989082cff98 Mon Sep 17 00:00:00 2001 From: Haowei Wu Date: Mon, 25 Sep 2023 11:10:08 -0700 Subject: [PATCH 2/3] amend! [unittest] Add option to allow disabling sharding in unittest Use for f-strings for error messages. Use common env key/values. --- llvm/utils/lit/lit/formats/googletest.py | 25 +++++++++---------- .../DummySubDir/OneTest.py | 8 +++--- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/llvm/utils/lit/lit/formats/googletest.py b/llvm/utils/lit/lit/formats/googletest.py index 7c427e0106ae5..df7fc650f351f 100644 --- a/llvm/utils/lit/lit/formats/googletest.py +++ b/llvm/utils/lit/lit/formats/googletest.py @@ -147,6 +147,10 @@ def execute(self, test, litConfig): from lit.cl_arguments import TestOrder use_shuffle = TestOrder(litConfig.order) == TestOrder.RANDOM + shard_env = { + "GTEST_OUTPUT": "json:" + test.gtest_json_file, + "GTEST_SHUFFLE": "1" if use_shuffle else "0", + } if not litConfig.disableGTestSharding: testPath, testName = os.path.split(test.getSourcePath()) while not os.path.exists(testPath): @@ -157,19 +161,14 @@ def execute(self, test, litConfig): testName, total_shards = os.path.split(testName) testName, shard_idx = os.path.split(testName) - shard_env = { - "GTEST_OUTPUT": "json:" + test.gtest_json_file, - "GTEST_SHUFFLE": "1" if use_shuffle else "0", - "GTEST_TOTAL_SHARDS": os.environ.get( - "GTEST_TOTAL_SHARDS", total_shards - ), - "GTEST_SHARD_INDEX": os.environ.get("GTEST_SHARD_INDEX", shard_idx), - } - else: - shard_env = { - "GTEST_OUTPUT": "json:" + test.gtest_json_file, - "GTEST_SHUFFLE": "1" if use_shuffle else "0", - } + shard_env.update( + { + "GTEST_TOTAL_SHARDS": os.environ.get( + "GTEST_TOTAL_SHARDS", total_shards + ), + "GTEST_SHARD_INDEX": os.environ.get("GTEST_SHARD_INDEX", shard_idx), + } + ) test.config.environment.update(shard_env) cmd = [testPath] diff --git a/llvm/utils/lit/tests/Inputs/googletest-no-sharding/DummySubDir/OneTest.py b/llvm/utils/lit/tests/Inputs/googletest-no-sharding/DummySubDir/OneTest.py index 8fcab0081a212..38d76cd637fb5 100644 --- a/llvm/utils/lit/tests/Inputs/googletest-no-sharding/DummySubDir/OneTest.py +++ b/llvm/utils/lit/tests/Inputs/googletest-no-sharding/DummySubDir/OneTest.py @@ -5,7 +5,7 @@ if len(sys.argv) == 3 and sys.argv[1] == "--gtest_list_tests": if sys.argv[2] != "--gtest_filter=-*DISABLED_*": - raise ValueError("unexpected argument: %s" % (sys.argv[2])) + raise ValueError(f"unexpected argument: {sys.argv[2]}") print( """\ FirstTest. @@ -21,14 +21,14 @@ sys.exit(0) elif len(sys.argv) != 1: # sharding and json output are specified using environment variables - raise ValueError("unexpected argument: %r" % (" ".join(sys.argv[1:]))) + raise ValueError(f"unexpected argument: {' '.join(sys.argv[1:])!r}") for e in ["GTEST_OUTPUT"]: if e not in os.environ: - raise ValueError("missing environment variables: " + e) + raise ValueError(f"missing environment variables: {e}") if not os.environ["GTEST_OUTPUT"].startswith("json:"): - raise ValueError("must emit json output: " + os.environ["GTEST_OUTPUT"]) + raise ValueError(f"must emit json output: {os.environ['GTEST_OUTPUT']}") output = """\ { From 35095cf98e341cece45ef05fee8f1047378586ed Mon Sep 17 00:00:00 2001 From: Haowei Wu Date: Tue, 17 Oct 2023 14:15:17 -0700 Subject: [PATCH 3/3] amend! amend! [unittest] Add option to allow disabling sharding in unittest Fix a typo. --- llvm/utils/lit/lit/formats/googletest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/utils/lit/lit/formats/googletest.py b/llvm/utils/lit/lit/formats/googletest.py index df7fc650f351f..16f411b25607a 100644 --- a/llvm/utils/lit/lit/formats/googletest.py +++ b/llvm/utils/lit/lit/formats/googletest.py @@ -154,7 +154,7 @@ def execute(self, test, litConfig): if not litConfig.disableGTestSharding: testPath, testName = os.path.split(test.getSourcePath()) while not os.path.exists(testPath): - # Handle GTest parametrized and typed tests, whose name includes + # Handle GTest parameterized and typed tests, whose name includes # some '/'s. testPath, namePrefix = os.path.split(testPath) testName = namePrefix + "/" + testName