[llvm] gtest sharding (PR #67063)

via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 21 14:02:33 PDT 2023


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-testing-tools

<details>
<summary>Changes</summary>

[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 "use_sharding" in the lit
googletest configuration and it can be configured using
"LLVM_GTEST_USE_SHARDING" cmake option.


---
Full diff: https://github.com/llvm/llvm-project/pull/67063.diff


6 Files Affected:

- (modified) llvm/test/Unit/lit.cfg.py (+6-1) 
- (modified) llvm/test/Unit/lit.site.cfg.py.in (+2) 
- (modified) llvm/utils/lit/lit/formats/googletest.py (+72-27) 
- (added) llvm/utils/lit/tests/Inputs/googletest-no-sharding/DummySubDir/OneTest.py (+105) 
- (added) llvm/utils/lit/tests/Inputs/googletest-no-sharding/lit.cfg (+4) 
- (added) llvm/utils/lit/tests/googletest-no-sharding.py (+43) 


``````````diff
diff --git a/llvm/test/Unit/lit.cfg.py b/llvm/test/Unit/lit.cfg.py
index f15c30dbcdb0aa6..4e815444d339dcd 100644
--- a/llvm/test/Unit/lit.cfg.py
+++ b/llvm/test/Unit/lit.cfg.py
@@ -19,7 +19,12 @@
 config.test_source_root = config.test_exec_root
 
 # testFormat: The test format to use to interpret tests.
-config.test_format = lit.formats.GoogleTest(config.llvm_build_mode, "Tests")
+config.test_format = lit.formats.GoogleTest(
+    config.llvm_build_mode,
+    "Tests",
+    run_under=config.gtest_run_under,
+    use_sharding=config.use_gtest_sharding,
+)
 
 # Propagate the temp directory. Windows requires this because it uses \Windows\
 # if none of these are present.
diff --git a/llvm/test/Unit/lit.site.cfg.py.in b/llvm/test/Unit/lit.site.cfg.py.in
index 1d7d76580149495..523d2bb3817e122 100644
--- a/llvm/test/Unit/lit.site.cfg.py.in
+++ b/llvm/test/Unit/lit.site.cfg.py.in
@@ -7,6 +7,8 @@ config.llvm_obj_root = path(r"@LLVM_BINARY_DIR@")
 config.llvm_tools_dir = lit_config.substitute(path(r"@LLVM_TOOLS_DIR@"))
 config.llvm_build_mode = lit_config.substitute("@LLVM_BUILD_MODE@")
 config.shlibdir = lit_config.substitute(path(r"@SHLIBDIR@"))
+config.gtest_run_under = lit_config.substitute(r"@LLVM_GTEST_RUN_UNDER@")
+config.use_gtest_sharding = lit_config.substitute(r"@LLVM_GTEST_USE_SHARD@")
 
 # Let the main config do the real work.
 lit_config.load_config(
diff --git a/llvm/utils/lit/lit/formats/googletest.py b/llvm/utils/lit/lit/formats/googletest.py
index f8304cbd05453b5..f05e7e351cfefa6 100644
--- a/llvm/utils/lit/lit/formats/googletest.py
+++ b/llvm/utils/lit/lit/formats/googletest.py
@@ -15,7 +15,7 @@
 
 
 class GoogleTest(TestFormat):
-    def __init__(self, test_sub_dirs, test_suffix, run_under=[]):
+    def __init__(self, test_sub_dirs, test_suffix, run_under=[], use_sharding=True):
         self.seen_executables = set()
         self.test_sub_dirs = str(test_sub_dirs).split(";")
 
@@ -27,6 +27,17 @@ def __init__(self, test_sub_dirs, test_suffix, run_under=[]):
         # Also check for .py files for testing purposes.
         self.test_suffixes = {exe_suffix, test_suffix + ".py"}
         self.run_under = run_under
+        if type(use_sharding) is str:
+            use_sharding = use_sharding.lower()
+            if use_sharding in ("0", "off", "false", "no", "n", "ignore", "notfound"):
+                use_sharding = False
+            else:
+                use_sharding = True
+        elif type(use_sharding) is bool:
+            pass
+        else:
+            use_sharding = True
+        self.use_sharding = use_sharding
 
     def get_num_tests(self, path, litConfig, localConfig):
         list_test_cmd = self.prepareCmd(
@@ -68,24 +79,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 self.use_sharding:
+                        # 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 +154,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 self.use_sharding:
+            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/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..5507fe65782dcbc
--- /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", use_sharding="False")
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..3fa7a37cdd7090c
--- /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 --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

``````````

</details>


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


More information about the llvm-commits mailing list