[llvm] [unittest] Add option to allow disabling sharding in unittest (PR #67063)

via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 17 14:16:01 PDT 2023


https://github.com/zeroomega updated https://github.com/llvm/llvm-project/pull/67063

>From 0299421c640227e15550b6e4d48abfdecca93bbb Mon Sep 17 00:00:00 2001
From: Haowei Wu <haowei at google.com>
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 d7e79b60f385bd3..c7703f15f9e3f71 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 132476fb2a36783..7f12f833afe5995 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 f8304cbd05453b5..7c427e0106ae5f0 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 3cb47c605ad5ac7..19851723cd2955b 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 000000000000000..8fcab0081a21209
--- /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 000000000000000..fd75709efad3cd0
--- /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 000000000000000..ccf2fe0d9d31d32
--- /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 5e24e6f6da4ad860ded2e85991fb6371940107c1 Mon Sep 17 00:00:00 2001
From: Haowei Wu <haowei at google.com>
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 7c427e0106ae5f0..df7fc650f351f7a 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 8fcab0081a21209..38d76cd637fb5dd 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 4cb204f28b95353081dbe3d3db1ceeb96783f8ed Mon Sep 17 00:00:00 2001
From: Haowei Wu <haowei at google.com>
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 df7fc650f351f7a..16f411b25607a99 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



More information about the llvm-commits mailing list