[PATCH] D122251: [lit] Use sharding for GoogleTest format

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D122251#3401008 <https://reviews.llvm.org/D122251#3401008>, @ychen wrote:

> Do we support using upstream googletest instead of the in-tree copy for the unit test?

LLVM unit tests use the in-tree gtest. If you need more recent functionality, you should look at updating the im-tree copy.

What's with all the deleted files?



================
Comment at: llvm/utils/lit/lit/formats/googletest.py:39
                             litConfig, localConfig):
+        init_shard_size = 512 # number of tests in a shard
+        core_count = lit.util.usable_core_count()
----------------
Why 512?


================
Comment at: llvm/utils/lit/lit/main.py:137
 
+def post_process_gtest_result(selected_tests, discovered_tests):
+    def remove_gtest(tests):
----------------



================
Comment at: llvm/utils/lit/lit/main.py:140
+        idxs = []
+        for idx,t in enumerate(tests):
+            if t.gtest_json_file:
----------------



================
Comment at: llvm/utils/lit/lit/main.py:143
+                idxs.append(idx)
+        count = len(idxs)
+        for i in range(count):
----------------
Just do `len(idxs)` inline


================
Comment at: llvm/utils/lit/lit/main.py:151-152
+    for test in gtests:
+        if test.gtest_json_file is None:
+            continue
+
----------------
This seems redundant given the way `gtests` is created.


================
Comment at: llvm/utils/lit/lit/main.py:154-155
+
+        # Load json file to retrieve results
+        with open(test.gtest_json_file, encoding='utf-8') as file:
+            testsuites = json.load(file)['testsuites']
----------------
Don't use `file` as a variable name since it's a built-in python name.


================
Comment at: llvm/utils/lit/lit/main.py:159
+                for testinfo in testcase['testsuite']:
+                    # Ignore DISABLE'd tests.
+                    if testinfo['result'] == 'SUPPRESSED':
----------------
Since the term appears to be `SUPPRESSED` anyway...


================
Comment at: llvm/utils/lit/lit/main.py:172
+                    if testinfo['result'] == 'SKIPPED':
+                        restcode = lit.Test.SKIPPED
+                    elif 'failures' in testinfo:
----------------
`restcode` -> `resultCode` or `returnCode`, right?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122251/new/

https://reviews.llvm.org/D122251