Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[unittest] Refactoring the gtest sharding option. #69537

Merged
merged 1 commit into from
Oct 19, 2023
Merged

Conversation

zeroomega
Copy link
Contributor

This patch addresses the missed review comment from PR #67063. It
renames LIT flag "--disable-gtest-sharding" to "--no-gtest-sharding"
and corrects the code style issue.

This patch addresses the missed review comment from PR llvm#67063. It
renames LIT flag "--disable-gtest-sharding" to "--no-gtest-sharding"
and corrects the code style issue.
@zeroomega zeroomega marked this pull request as ready for review October 18, 2023 23:55
@llvmbot
Copy link

llvmbot commented Oct 18, 2023

@llvm/pr-subscribers-testing-tools

Author: Haowei (zeroomega)

Changes

This patch addresses the missed review comment from PR #67063. It
renames LIT flag "--disable-gtest-sharding" to "--no-gtest-sharding"
and corrects the code style issue.


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

5 Files Affected:

  • (modified) llvm/utils/lit/lit/LitConfig.py (+2-2)
  • (modified) llvm/utils/lit/lit/cl_arguments.py (+9-3)
  • (modified) llvm/utils/lit/lit/formats/googletest.py (+2-2)
  • (modified) llvm/utils/lit/lit/main.py (+1-1)
  • (modified) llvm/utils/lit/tests/googletest-no-sharding.py (+1-1)
diff --git a/llvm/utils/lit/lit/LitConfig.py b/llvm/utils/lit/lit/LitConfig.py
index c7703f15f9e3f71..5dc712ae28370cc 100644
--- a/llvm/utils/lit/lit/LitConfig.py
+++ b/llvm/utils/lit/lit/LitConfig.py
@@ -37,7 +37,7 @@ def __init__(
         maxIndividualTestTime=0,
         parallelism_groups={},
         per_test_coverage=False,
-        disableGTestSharding=False,
+        gtest_sharding=True,
     ):
         # The name of the test runner.
         self.progname = progname
@@ -88,7 +88,7 @@ def __init__(
         self.maxIndividualTestTime = maxIndividualTestTime
         self.parallelism_groups = parallelism_groups
         self.per_test_coverage = per_test_coverage
-        self.disableGTestSharding = bool(disableGTestSharding)
+        self.gtest_sharding = bool(gtest_sharding)
 
     @property
     def maxIndividualTestTime(self):
diff --git a/llvm/utils/lit/lit/cl_arguments.py b/llvm/utils/lit/lit/cl_arguments.py
index 7f12f833afe5995..ba3706659550b67 100644
--- a/llvm/utils/lit/lit/cl_arguments.py
+++ b/llvm/utils/lit/lit/cl_arguments.py
@@ -119,10 +119,16 @@ 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",
+        "--gtest-sharding",
+        help="Enable sharding for GoogleTest format",
         action="store_true",
+        default=True,
+    )
+    execution_group.add_argument(
+        "--no-gtest-sharding",
+        dest="gtest_sharding",
+        help="Disable sharding for GoogleTest format",
+        action="store_false",
     )
     execution_group.add_argument(
         "--path",
diff --git a/llvm/utils/lit/lit/formats/googletest.py b/llvm/utils/lit/lit/formats/googletest.py
index 16f411b25607a99..8037094a910678a 100644
--- a/llvm/utils/lit/lit/formats/googletest.py
+++ b/llvm/utils/lit/lit/formats/googletest.py
@@ -68,7 +68,7 @@ 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:
-                    if not litConfig.disableGTestSharding:
+                    if litConfig.gtest_sharding:
                         # Compute the number of shards.
                         shard_size = init_shard_size
                         nshard = int(math.ceil(num_tests / shard_size))
@@ -151,7 +151,7 @@ def execute(self, test, litConfig):
             "GTEST_OUTPUT": "json:" + test.gtest_json_file,
             "GTEST_SHUFFLE": "1" if use_shuffle else "0",
         }
-        if not litConfig.disableGTestSharding:
+        if litConfig.gtest_sharding:
             testPath, testName = os.path.split(test.getSourcePath())
             while not os.path.exists(testPath):
                 # Handle GTest parameterized and typed tests, whose name includes
diff --git a/llvm/utils/lit/lit/main.py b/llvm/utils/lit/lit/main.py
index 4580dbc96667948..1d0d6bb2682993d 100755
--- a/llvm/utils/lit/lit/main.py
+++ b/llvm/utils/lit/lit/main.py
@@ -41,7 +41,7 @@ def main(builtin_params={}):
         params=params,
         config_prefix=opts.configPrefix,
         per_test_coverage=opts.per_test_coverage,
-        disableGTestSharding=opts.disableGTestSharding,
+        gtest_sharding=opts.gtest_sharding,
     )
 
     discovered_tests = lit.discovery.find_tests_for_inputs(
diff --git a/llvm/utils/lit/tests/googletest-no-sharding.py b/llvm/utils/lit/tests/googletest-no-sharding.py
index ccf2fe0d9d31d32..bb008effb831533 100644
--- a/llvm/utils/lit/tests/googletest-no-sharding.py
+++ b/llvm/utils/lit/tests/googletest-no-sharding.py
@@ -1,6 +1,6 @@
 # Check the various features of the GoogleTest format.
 
-# RUN: not %{lit} -v --disable-gtest-sharding --order=random %{inputs}/googletest-no-sharding > %t.out
+# RUN: not %{lit} -v --no-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

@zeroomega zeroomega merged commit 44d4b30 into llvm:main Oct 19, 2023
6 checks passed
kendalharland pushed a commit to kendalharland/llvm-project that referenced this pull request Aug 8, 2024
This patch addresses the missed review comment from PR llvm#67063. It
renames LIT flag "--disable-gtest-sharding" to "--no-gtest-sharding"
and corrects the code style issue.

(cherry picked from commit 44d4b30)
kendalharland pushed a commit to kendalharland/llvm-project that referenced this pull request Aug 8, 2024
This patch addresses the missed review comment from PR llvm#67063. It
renames LIT flag "--disable-gtest-sharding" to "--no-gtest-sharding"
and corrects the code style issue.

(cherry picked from commit 44d4b30)
kendalharland pushed a commit to kendalharland/llvm-project that referenced this pull request Aug 8, 2024
This patch addresses the missed review comment from PR llvm#67063. It
renames LIT flag "--disable-gtest-sharding" to "--no-gtest-sharding"
and corrects the code style issue.

(cherry picked from commit 44d4b30)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants