[lit] Use sharding for GoogleTest format

[ychen]

Summary

This helps lit unit test performance by a lot, especially on windows. The performance gain comes from launching one gtest executable for many subtests instead of one (this is the current situation).

The shards are executed by the test runner and the results are stored in the
json format supported by the GoogleTest. Later in the test reporting stage,
all test results in the json file are retrieved to continue the test results
summary etc.

On my Win10 desktop, before this patch: check-clang-unit: 177s, check-llvm-unit: 38s; after this patch: check-clang-unit: 37s, check-llvm-unit: 11s.
On my Linux machine, before this patch: check-clang-unit: 46s, check-llvm-unit: 8s; after this patch: check-clang-unit: 7s, check-llvm-unit: 4s.

[ychen]

• Fix two bugs on Windows.

[ychen]

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

llvm/utils/lit/lit/TestingConfig.py

31

This is the replacement for --filter and --filter-not for unit tests. Since GTEST_FILTER takes a different format of regex than --filter/--filter-not , passing --filter and --filter-not arguments for unit tests to GTEST_FILTER could be confusing to the lit user. IMHO, it would be more clear to just use GTEST_FILTER. The drawback is sacrificing a bit of consistency. Please let me know if you think otherwise.

About --xfail /--xfail-not, tests for D106022 did not cover unit tests, I could add more tests and the handling for sharding in a follow-up patch considering this patch is already big.

[jhenderson]

In 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?

llvm/utils/lit/lit/formats/googletest.py

39

Why 512?

llvm/utils/lit/lit/main.py

138

141

144

Just do len(idxs) inline

152–153

This seems redundant given the way gtests is created.

155–156

Don't use file as a variable name since it's a built-in python name.

160

Since the term appears to be SUPPRESSED anyway...

173

restcode -> resultCode or returnCode, right?

[ychen]

In D122251#3401843, @jhenderson wrote:

In 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?

Hmm, I was asking about the upstream googletest because of the deleted tests. They were introduced by D18606 to handle upstream googletest --gtest_list_tests output. This is not needed for sharding since --gtest_list_tests is only used for estimating the number of sub-testcases which in turn is used to compute the number of shards. The correctness is always preserved.

llvm/utils/lit/lit/formats/googletest.py

39

Why 512?

Admittedly, I chose this with LLVM lit testing in mind. I'm not sure this is a good number for the general use of LIT. The criteria are that the number should be as big as possible to keep the number of shards small (so that the overhead is small) while the number of shards should be big enough to keep all cores busy and not hurts scheduling.

[ychen]

• Address James's comments.

[probinson]

In D122251#3401008, @ychen wrote:

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

The Clang/LLVM unittests use an imported copy of googletest (with some modifications, I believe mainly to allow using LLVM's various string types with gtest macros). However, I think I have seen patches intended to allow lit to work cleanly with upstream googletest. That functionality should be preserved, even though LLVM itself doesn't make use of it.

[ychen]

In D122251#3403337, @probinson wrote:

In D122251#3401008, @ychen wrote:

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

The Clang/LLVM unittests use an imported copy of googletest (with some modifications, I believe mainly to allow using LLVM's various string types with gtest macros). However, I think I have seen patches intended to allow lit to work cleanly with upstream googletest. That functionality should be preserved, even though LLVM itself doesn't make use of it.

Agreed. I think they're preserved. The deleted tests are meant for a piece of deleted code.

[jhenderson]

Re. performance, I can see the benefit if you just run the unit tests, but is that the normal use-case? My suspicion is that most peopke just run check-llvm or check-clang, which includes the regular lit tests, and therefore already ends up with things split across he available cores. Do you have any figures for that case?

llvm/utils/lit/lit/main.py

144–145

Doesn't this invalidate the indexes after the first one? If we need to delete items 2 and 4 in the list, after deleting item 2, item 4 will be at index 3 now.

152

Don't use file as a variable name.

[probinson]

My suspicion is that most peopke just run check-llvm or check-clang, which includes the regular lit tests, and therefore already ends up with things split across he available cores.

Yes, but.

The way lit works now, it runs each unittest program once per test function (plus one, an initial run to retrieve the list of test functions without actually running them). If a unittest has 500 test functions, lit will run the unittest program 500 times, running one test function each time. The total number of test functions is some thousands, across all unittest programs. Even on Linux, the process-creation overhead adds up. Yes, all those processes are distributed across the available cores, but you're still talking many thousands of processes.

By taking advantage of gtest's sharding feature, within each test program we essentially divide the total number of tests-per-program by the number of cores, and run each test program once per core. So, the number of processes created goes from (number-of-test-functions-across-all-unittest-programs) to (number-of-cores X number-of-test-programs). Actually in some cases fewer, because there are a handful of unittest programs that have a very small number of test functions, but for ballparking this is the right calculation. @ychen can correct me if I'm misunderstanding what's going on.

I believe he tried running each unittest program exactly once, but some of them take significant time, and the net wall-time reduction wasn't as good. gtest's sharding seemed like a good compromise.

[ychen]

• Rename file to f

llvm/utils/lit/lit/main.py

144–145

The -i makes the index correct. To delete items 2 and 4 in the list, the idxs is [2,4]. The del statement ends up being del tests[2-0] and del tests[4-1].

[ychen]

In D122251#3405396, @probinson wrote:

My suspicion is that most peopke just run check-llvm or check-clang, which includes the regular lit tests, and therefore already ends up with things split across he available cores.

Yes, but.

The way lit works now, it runs each unittest program once per test function (plus one, an initial run to retrieve the list of test functions without actually running them). If a unittest has 500 test functions, lit will run the unittest program 500 times, running one test function each time. The total number of test functions is some thousands, across all unittest programs. Even on Linux, the process-creation overhead adds up. Yes, all those processes are distributed across the available cores, but you're still talking many thousands of processes.

By taking advantage of gtest's sharding feature, within each test program we essentially divide the total number of tests-per-program by the number of cores, and run each test program once per core. So, the number of processes created goes from (number-of-test-functions-across-all-unittest-programs) to (number-of-cores X number-of-test-programs). Actually in some cases fewer, because there are a handful of unittest programs that have a very small number of test functions, but for ballparking this is the right calculation. @ychen can correct me if I'm misunderstanding what's going on.

I believe he tried running each unittest program exactly once, but some of them take significant time, and the net wall-time reduction wasn't as good. gtest's sharding seemed like a good compromise.

Thanks for the explanation, @probinson. That's exactly where the performance gain comes from. I've updated the patch description to explain this.

[jhenderson]

My knowledge of this area is relatively limited, so I don't feel like I have a good enough understanding of the intricacies to progress this review further. Thanks for the explanations as to why this approach is most appropriate. The real cost sounds like it's due to process spawning overhead primarily (and any startup costs within the program itself). That does make me wonder whether there's a natural way of threading things within the test programs themselves, but I suspect not as I doubt the unit tests are thread-safe. Consequently, this approach seems sound, but hopefully others with knowledge in this area might be able to chime in?

llvm/utils/lit/lit/main.py

144–145

Ah, I misread that as -1 not -i. Thanks!

[ychen]

• Fixed a bug on Windows

[ychen]

In D122251#3407354, @jhenderson wrote:

My knowledge of this area is relatively limited, so I don't feel like I have a good enough understanding of the intricacies to progress this review further. Thanks for the explanations as to why this approach is most appropriate. The real cost sounds like it's due to process spawning overhead primarily (and any startup costs within the program itself). That does make me wonder whether there's a natural way of threading things within the test programs themselves, but I suspect not as I doubt the unit tests are thread-safe. Consequently, this approach seems sound, but hopefully others with knowledge in this area might be able to chime in?

@jhenderson Thanks for taking a look. Adding a few potentially interested reviewers working on Windows (@rnk, @mstorsjo, @compnerd).

[rnk]

I don't have time to review the code, but this seems like a good direction.

What happens if the unit test shard crashes without writing the final JSON file? Does lit handle that? Is or can it be tested, and if not, what gives you confidence that it will work?

[ychen]

In D122251#3414630, @rnk wrote:

I don't have time to review the code, but this seems like a good direction.

What happens if the unit test shard crashes without writing the final JSON file? Does lit handle that? Is or can it be tested, and if not, what gives you confidence that it will work?

It would trigger an assertion failure : assert os.path.exists(test.gtest_json_file). The only way this could happen is the copy of GoogleTest in LLVM has crashing bugs that may be triggered by new tests or new test environments (existing ones, if any, should be found when this patch is landed). I think both cases could be detected and handled reliably.

[rnk]

In D122251#3414840, @ychen wrote:

It would trigger an assertion failure : assert os.path.exists(test.gtest_json_file). The only way this could happen is the copy of GoogleTest in LLVM has crashing bugs that may be triggered by new tests or new test environments (existing ones, if any, should be found when this patch is landed). I think both cases could be detected and handled reliably.

I think I don't agree that the only way this could happen is if there are gtest bugs. One segfault in the middle of a test suite (on Linux, no SEH or signal handler recovery mechanism) will bring down the entire gtest shard process.

llvm/utils/lit/lit/formats/googletest.py

116–118

I don't think it makes sense to use assert to check for things that are not expected invariants. If an exception is desired, IMO it's better to use if ...: raise .... Alternatively, it makes sense to return a FAIL test status in this case.

129

This command is different from the sharded command. It is possible to construct tests that fail when run together but pass when run in isolation, so this command may not always reproduce failures observable with lit. I'm not sure how to signal that to the user.

[rnk]

I should add, I think it's worth adding a test that shows what happens when a shard fails. This is doable, just have the fake gtest process exit without producing a json file.

[ychen]

In D122251#3414948, @rnk wrote:

I should add, I think it's worth adding a test that shows what happens when a shard fails. This is doable, just have the fake gtest process exit without producing a json file.

Sounds good. I'll add the handling for crashes with a test.

[probinson]

llvm/utils/lit/lit/formats/googletest.py

129

Currently, *all* unitttest functions are run in isolation under lit, but would be run in the same process by someone who just runs the program interactively with no arguments. It would be best to report exactly what lit did, but I'm not super worried about weird test-function interactions.

[ychen]

• return test failure for shard crash (added a new test case)
• print the shard command if any tests in the shard fail.

llvm/utils/lit/lit/formats/googletest.py

129

I've added a header script for the sharded command if there are any test failures in the shard. The googletest-format down below has the output check. @rnk @probinson WDYT?

[rnk]

Thanks, I'm happy, but please let other reviewers comment.

[probinson]

I really like these results but I don't feel confident enough about Python to give a final okay.

[yln]

LGTM, with one small comment, thanks!

[yln]

llvm/utils/lit/lit/main.py

112

Can we move this function out into to a gtest-specific file and make it return new (filtered & changed) lists leaving the passed-in ones intact. Something like:

selected_tests, discovered_tests = gtest.post_process_results(selected_tests, discovered_tests)

Thanks!

[ychen]

• move post processing method to class GoogleTest

[ychen]

• format

[ychen]

llvm/utils/lit/lit/main.py

112

Yep, done.

[yln]

llvm/utils/lit/lit/formats/googletest.py

170

This will still change the passed-in lists, right?

[ychen]

llvm/utils/lit/lit/formats/googletest.py

170

That's right. It is by reference.

[fhahn]

Not sure if it is related, but it looks like the output strings for failed unit tests regressed over the last day, it's now something like Clangd Unit Tests :: ./ClangdTests/24/133 instead of the test name.

[thakis]

check-llvm has been failing on my bots since this landed: http://45.33.8.238/linux/72775/step_12.txt

Does the error make sense to you?

[thakis]

(Also fails on my win bot with the same error: http://45.33.8.238/win/55601/step_11.txt

Passes fine on my mac bot, though!)

[thakis]

I don't see it discussed above: An advantage of the previous scheme is that it guarantees that there are no order dependencies between tests and that each tests passes in isolation.

The speedup is nice, but it's also somewhat small relative to total check-llvm / check-clang time. Is that worth sacrificing guaranteed independent tests for?

[thakis]

In D122251#3426058, @thakis wrote:

(Also fails on my win bot with the same error: http://45.33.8.238/win/55601/step_11.txt

Passes fine on my mac bot, though!)

Aha, I think I know what this is: MLAnalysisTests is an old, stale binary that just happens to still be there due to an old build (these bots do incremental builds). I _think_ this change means that all future stale binaries shouldn't have this problem and that this is a one-time thing, not an ongoing, systemic problem for incremental builds every time a test binary is added or removed. So I'll just manually remove this one stale binary from the bots.

[thakis]

In D122251#3426061, @thakis wrote:

I don't see it discussed above: An advantage of the previous scheme is that it guarantees that there are no order dependencies between tests and that each tests passes in isolation.

The speedup is nice, but it's also somewhat small relative to total check-llvm / check-clang time. Is that worth sacrificing guaranteed independent tests for?

FWIW, I don't see a speedup on Win on my bot.
Before: http://45.33.8.238/win/55580/summary.html
After: http://45.33.8.238/win/55605/summary.html

If anything, check-clang got 30s slower.

On linux, check-clang is 16 seconds faster (out of ~5 min total bot cycle time, so a 5.3% speedup) but none of the other test suites changed:
Before: http://45.33.8.238/linux/72772/summary.html
After: http://45.33.8.238/linux/72807/summary.html

Do you see bigger wins on other bots?

[abrachet]

When using result_db output from lit we are seeing

Traceback (most recent call last):
  File "/b/s/w/ir/x/w/staging/llvm_build/./bin/llvm-lit", line 50, in <module>
    main(builtin_parameters)
  File "/b/s/w/ir/x/w/llvm-llvm-project/llvm/utils/lit/lit/main.py", line 121, in main
    report.write_results(tests_for_report, elapsed)
  File "/b/s/w/ir/x/w/llvm-llvm-project/llvm/utils/lit/lit/reports.py", line 208, in write_results
    gen_resultdb_test_entry(
  File "/b/s/w/ir/x/w/llvm-llvm-project/llvm/utils/lit/lit/reports.py", line 163, in gen_resultdb_test_entry
    'start_time': datetime.datetime.fromtimestamp(start_time).isoformat() + 'Z',
TypeError: an integer is required (got type NoneType)

https://github.com/llvm/llvm-project/blob/main/llvm/utils/lit/lit/reports.py#L163

[ychen]

In D122251#3425750, @fhahn wrote:

Not sure if it is related, but it looks like the output strings for failed unit tests regressed over the last day, it's now something like Clangd Unit Tests :: ./ClangdTests/24/133 instead of the test name.

That's expected. 24 is the shard index; 133 is the total number of shards. The failed message would print the failed test name and the reproducing script for the shard and the individual test. If anything is missing, please let me know.

[ychen]

In D122251#3426198, @thakis wrote:

In D122251#3426061, @thakis wrote:

I don't see it discussed above: An advantage of the previous scheme is that it guarantees that there are no order dependencies between tests and that each tests passes in isolation.

The speedup is nice, but it's also somewhat small relative to total check-llvm / check-clang time. Is that worth sacrificing guaranteed independent tests for?

FWIW, I don't see a speedup on Win on my bot.
Before: http://45.33.8.238/win/55580/summary.html
After: http://45.33.8.238/win/55605/summary.html

If anything, check-clang got 30s slower.

Probably there is some one-time cost involved?

http://45.33.8.238/win/55614/summary.html did not regress which is expected since these runs do not include unit tests.

On linux, check-clang is 16 seconds faster (out of ~5 min total bot cycle time, so a 5.3% speedup) but none of the other test suites changed:
Before: http://45.33.8.238/linux/72772/summary.html
After: http://45.33.8.238/linux/72807/summary.html

Do you see bigger wins on other bots?

Yeah, like
before (~270) https://lab.llvm.org/buildbot/#/builders/216/builds/2323/steps/7/logs/stdio
after (~240) https://lab.llvm.org/buildbot/#/builders/216/builds/2356/steps/7/logs/stdio

The time fluctuates to various degrees for bots. bot 216 is relatively stable and the speedup is what I would expect for a windows machine.

[ychen]

In D122251#3426344, @abrachet wrote:

When using result_db output from lit we are seeing

Traceback (most recent call last):
  File "/b/s/w/ir/x/w/staging/llvm_build/./bin/llvm-lit", line 50, in <module>
    main(builtin_parameters)
  File "/b/s/w/ir/x/w/llvm-llvm-project/llvm/utils/lit/lit/main.py", line 121, in main
    report.write_results(tests_for_report, elapsed)
  File "/b/s/w/ir/x/w/llvm-llvm-project/llvm/utils/lit/lit/reports.py", line 208, in write_results
    gen_resultdb_test_entry(
  File "/b/s/w/ir/x/w/llvm-llvm-project/llvm/utils/lit/lit/reports.py", line 163, in gen_resultdb_test_entry
    'start_time': datetime.datetime.fromtimestamp(start_time).isoformat() + 'Z',
TypeError: an integer is required (got type NoneType)

https://github.com/llvm/llvm-project/blob/main/llvm/utils/lit/lit/reports.py#L163

I see you've reverted the patch. Thanks. It seems some output formats depend on start_time which is not synthesized for the shards, and strangely the lit tests for these formats did not trigger. I'll fix it and reland.

[abrachet]

In D122251#3426889, @ychen wrote:

I see you've reverted the patch. Thanks. It seems some output formats depend on start_time which is not synthesized for the shards, and strangely the lit tests for these formats did not trigger. I'll fix it and reland.

Thanks! Sorry about that. It just keeps our bots red and possibly masks other problems

[browneee]

https://github.com/llvm/llvm-project/commit/948f3deca91a66caf4a618f826ff6de8277bed9c

This re-land seems to have broken this buildbot: https://lab.llvm.org/buildbot/#/builders/169

Previous Good: https://lab.llvm.org/buildbot/#/builders/169/builds/7234
Broken: https://lab.llvm.org/buildbot/#/builders/169/builds/7235

[ychen]

In D122251#3426905, @browneee wrote:

https://github.com/llvm/llvm-project/commit/948f3deca91a66caf4a618f826ff6de8277bed9c

This re-land seems to have broken this buildbot: https://lab.llvm.org/buildbot/#/builders/169

Previous Good: https://lab.llvm.org/buildbot/#/builders/169/builds/7234
Broken: https://lab.llvm.org/buildbot/#/builders/169/builds/7235

All x86 tests passed but non-x86 tests failed with

Traceback (most recent call last):
  File "/b/sanitizer-x86_64-linux-qemu/build/llvm_build0/./bin/llvm-lit", line 45, in <module>
    main(builtin_parameters)
  File "/b/sanitizer-x86_64-linux-qemu/build/llvm-project/llvm/utils/lit/lit/main.py", line 45, in main
    opts.indirectlyRunCheck)
  File "/b/sanitizer-x86_64-linux-qemu/build/llvm-project/llvm/utils/lit/lit/discovery.py", line 279, in find_tests_for_inputs
    local_config_cache, indirectlyRunCheck)[1])
  File "/b/sanitizer-x86_64-linux-qemu/build/llvm-project/llvm/utils/lit/lit/discovery.py", line 200, in getTestsInSuite
    litConfig, lc):
  File "/b/sanitizer-x86_64-linux-qemu/build/llvm-project/llvm/utils/lit/lit/formats/googletest.py", line 50, in getTestsInDirectory
    num_tests = self.get_num_tests(execpath, localConfig)
  File "/b/sanitizer-x86_64-linux-qemu/build/llvm-project/llvm/utils/lit/lit/formats/googletest.py", line 32, in get_num_tests
    out, _, exitCode = lit.util.executeCommand(cmd, env=localConfig.environment)
  File "/b/sanitizer-x86_64-linux-qemu/build/llvm-project/llvm/utils/lit/lit/util.py", line 342, in executeCommand
    env=env, close_fds=kUseCloseFDs)
  File "/usr/lib/python3.7/subprocess.py", line 775, in __init__
    restore_signals, start_new_session)
  File "/usr/lib/python3.7/subprocess.py", line 1522, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
OSError: [Errno 8] Exec format error: '/b/sanitizer-x86_64-linux-qemu/build/llvm_build2_aarch64_qemu/lib/scudo/standalone/tests/./ScudoUnitTest-aarch64-Test'

I suspect the bot has some setup issue. How do I find out who is the bot owner?

[yln]

llvm/utils/lit/lit/formats/googletest.py

170

I meant: please change this function so it builds new lists and leaves the passed-in ones unchanged. Thanks!

[vitalybuka]

In D122251#3426909, @ychen wrote:

In D122251#3426905, @browneee wrote:

https://github.com/llvm/llvm-project/commit/948f3deca91a66caf4a618f826ff6de8277bed9c

This re-land seems to have broken this buildbot: https://lab.llvm.org/buildbot/#/builders/169

Previous Good: https://lab.llvm.org/buildbot/#/builders/169/builds/7234
Broken: https://lab.llvm.org/buildbot/#/builders/169/builds/7235

All x86 tests passed but non-x86 tests failed with

Traceback (most recent call last):
  File "/b/sanitizer-x86_64-linux-qemu/build/llvm_build0/./bin/llvm-lit", line 45, in <module>
    main(builtin_parameters)
  File "/b/sanitizer-x86_64-linux-qemu/build/llvm-project/llvm/utils/lit/lit/main.py", line 45, in main
    opts.indirectlyRunCheck)
  File "/b/sanitizer-x86_64-linux-qemu/build/llvm-project/llvm/utils/lit/lit/discovery.py", line 279, in find_tests_for_inputs
    local_config_cache, indirectlyRunCheck)[1])
  File "/b/sanitizer-x86_64-linux-qemu/build/llvm-project/llvm/utils/lit/lit/discovery.py", line 200, in getTestsInSuite
    litConfig, lc):
  File "/b/sanitizer-x86_64-linux-qemu/build/llvm-project/llvm/utils/lit/lit/formats/googletest.py", line 50, in getTestsInDirectory
    num_tests = self.get_num_tests(execpath, localConfig)
  File "/b/sanitizer-x86_64-linux-qemu/build/llvm-project/llvm/utils/lit/lit/formats/googletest.py", line 32, in get_num_tests
    out, _, exitCode = lit.util.executeCommand(cmd, env=localConfig.environment)
  File "/b/sanitizer-x86_64-linux-qemu/build/llvm-project/llvm/utils/lit/lit/util.py", line 342, in executeCommand
    env=env, close_fds=kUseCloseFDs)
  File "/usr/lib/python3.7/subprocess.py", line 775, in __init__
    restore_signals, start_new_session)
  File "/usr/lib/python3.7/subprocess.py", line 1522, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
OSError: [Errno 8] Exec format error: '/b/sanitizer-x86_64-linux-qemu/build/llvm_build2_aarch64_qemu/lib/scudo/standalone/tests/./ScudoUnitTest-aarch64-Test'

I suspect the bot has some setup issue. How do I find out who is the bot owner?

new code probably missed COMPILER_RT_EMULATOR

[ychen]

In D122251#3427536, @vitalybuka wrote:

In D122251#3426909, @ychen wrote:

In D122251#3426905, @browneee wrote:

https://github.com/llvm/llvm-project/commit/948f3deca91a66caf4a618f826ff6de8277bed9c

This re-land seems to have broken this buildbot: https://lab.llvm.org/buildbot/#/builders/169

Previous Good: https://lab.llvm.org/buildbot/#/builders/169/builds/7234
Broken: https://lab.llvm.org/buildbot/#/builders/169/builds/7235

All x86 tests passed but non-x86 tests failed with

Traceback (most recent call last):
  File "/b/sanitizer-x86_64-linux-qemu/build/llvm_build0/./bin/llvm-lit", line 45, in <module>
    main(builtin_parameters)
  File "/b/sanitizer-x86_64-linux-qemu/build/llvm-project/llvm/utils/lit/lit/main.py", line 45, in main
    opts.indirectlyRunCheck)
  File "/b/sanitizer-x86_64-linux-qemu/build/llvm-project/llvm/utils/lit/lit/discovery.py", line 279, in find_tests_for_inputs
    local_config_cache, indirectlyRunCheck)[1])
  File "/b/sanitizer-x86_64-linux-qemu/build/llvm-project/llvm/utils/lit/lit/discovery.py", line 200, in getTestsInSuite
    litConfig, lc):
  File "/b/sanitizer-x86_64-linux-qemu/build/llvm-project/llvm/utils/lit/lit/formats/googletest.py", line 50, in getTestsInDirectory
    num_tests = self.get_num_tests(execpath, localConfig)
  File "/b/sanitizer-x86_64-linux-qemu/build/llvm-project/llvm/utils/lit/lit/formats/googletest.py", line 32, in get_num_tests
    out, _, exitCode = lit.util.executeCommand(cmd, env=localConfig.environment)
  File "/b/sanitizer-x86_64-linux-qemu/build/llvm-project/llvm/utils/lit/lit/util.py", line 342, in executeCommand
    env=env, close_fds=kUseCloseFDs)
  File "/usr/lib/python3.7/subprocess.py", line 775, in __init__
    restore_signals, start_new_session)
  File "/usr/lib/python3.7/subprocess.py", line 1522, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
OSError: [Errno 8] Exec format error: '/b/sanitizer-x86_64-linux-qemu/build/llvm_build2_aarch64_qemu/lib/scudo/standalone/tests/./ScudoUnitTest-aarch64-Test'

I suspect the bot has some setup issue. How do I find out who is the bot owner?

new code probably missed COMPILER_RT_EMULATOR

Thanks @vitalybuka. I'll update the patch here.

[vitalybuka]

llvm/utils/lit/lit/formats/googletest.py

38

prepareCmd is gone?

[ychen]

llvm/utils/lit/lit/formats/googletest.py

38

yep. I'll put it back with a test.

[thakis]

http://45.33.8.238/win/55614/summary.html did not regress which is expected since these runs do not include unit tests.

check-llvm includes llvm's unit tests, check-lld includes lld's unit tests, check-clang includes clang's unit tests. That build does include unit tests.

If you compare the stdout:
http://45.33.8.238/linux/72772/step_7.txt
http://45.33.8.238/linux/72807/step_7.txt

then one says "30362 tests" while the other says "16250 tests" (which I think is expected since we now count number of unit test processes instead of number of tests?), so it does seem to have an effect. It's just not (much) faster.

before (~270) https://lab.llvm.org/buildbot/#/builders/216/builds/2323/steps/7/logs/stdio
after (~240) https://lab.llvm.org/buildbot/#/builders/216/builds/2356/steps/7/logs/stdio

The time fluctuates to various degrees for bots. bot 216 is relatively stable and the speedup is what I would expect for a windows machine.

Even on that it it's a 30s improvement on a bit with a 9-15 min cycle time, so a 3-5% speedup. Is that worth giving up the property that we're guaranteed we don't have ordering dependencies between tests?

[ychen]

In D122251#3428173, @thakis wrote:

http://45.33.8.238/win/55614/summary.html did not regress which is expected since these runs do not include unit tests.

check-llvm includes llvm's unit tests, check-lld includes lld's unit tests, check-clang includes clang's unit tests. That build does include unit tests.

If you compare the stdout:
http://45.33.8.238/linux/72772/step_7.txt
http://45.33.8.238/linux/72807/step_7.txt

then one says "30362 tests" while the other says "16250 tests" (which I think is expected since we now count number of unit test processes instead of number of tests?), so it does seem to have an effect. It's just not (much) faster.

before (~270) https://lab.llvm.org/buildbot/#/builders/216/builds/2323/steps/7/logs/stdio
after (~240) https://lab.llvm.org/buildbot/#/builders/216/builds/2356/steps/7/logs/stdio

The time fluctuates to various degrees for bots. bot 216 is relatively stable and the speedup is what I would expect for a windows machine.

Even on that it it's a 30s improvement on a bit with a 9-15 min cycle time, so a 3-5% speedup. Is that worth giving up the property that we're guaranteed we don't have ordering dependencies between tests?

https://github.com/google/googletest/blob/main/docs/advanced.md says that Remember that the test order is undefined, so your code can't depend on a test preceding or following another.. And https://github.com/google/googletest/blob/main/docs/advanced.md#shuffling-the-tests seems to suggest random order is that default. If we're paranoid, I could pass in GTEST_SHUFFLE all the time. WDYT?

[ychen]

In D122251#3428173, @thakis wrote:

http://45.33.8.238/win/55614/summary.html did not regress which is expected since these runs do not include unit tests.

check-llvm includes llvm's unit tests, check-lld includes lld's unit tests, check-clang includes clang's unit tests. That build does include unit tests.

If you compare the stdout:
http://45.33.8.238/linux/72772/step_7.txt
http://45.33.8.238/linux/72807/step_7.txt

Agreed that it does not help much for this bot. It is pretty fast already. TBH, I think the patch helps local development more than CI, especially for people working on Windows.

then one says "30362 tests" while the other says "16250 tests" (which I think is expected since we now count number of unit test processes instead of number of tests?), so it does seem to have an effect. It's just not (much) faster.

before (~270) https://lab.llvm.org/buildbot/#/builders/216/builds/2323/steps/7/logs/stdio
after (~240) https://lab.llvm.org/buildbot/#/builders/216/builds/2356/steps/7/logs/stdio

The time fluctuates to various degrees for bots. bot 216 is relatively stable and the speedup is what I would expect for a windows machine.

Even on that it it's a 30s improvement on a bit with a 9-15 min cycle time, so a 3-5% speedup. Is that worth giving up the property that we're guaranteed we don't have ordering dependencies between tests?

[ychen]

Thanks, everyone for the valuable feedback. I believe I addressed all of them in the updated patch.

• @abrachet: makes sure synthesize result.pid and result.start that are used by --resultdb-output and --time-trace-output. (with a test case)
• @vitalybuka: makes sure prepareCmd is used during the test discovery phase. (added a test case for the GTest wrapper command)
• @thakis: hook up lit --order=random to GTEST_SHUFFLE=1 and when a shard fails, include GTEST_RANDOM_SEED in the reproducer script. (this is checked in the updated test)
• @yln: compose new lists instead of modifying existing lists.
• formatting

[abrachet]

In D122251#3431337, @ychen wrote:

Thanks, everyone for the valuable feedback. I believe I addressed all of them in the updated patch.

• @abrachet: makes sure synthesize result.pid and result.start that are used by --resultdb-output and --time-trace-output. (with a test case)

I checked this out locally and it works for --resultdb-output. Thanks. Hopefully others can say if the fixes worked for them too before resubmitting.

[rprichard]

I think this change broke libc++ tests. _executeScriptInternal in libcxx/utils/libcxx/test/dsl.py does not pass an argument for LitConfig's new order parameter:

rprichard@cashew:/x/llvm-upstream/out$ cmake -GNinja -S ../llvm-project/runtimes -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" -DLIBCXXABI_USE_LLVM_UNWINDER=ON
...
rprichard@cashew:/x/llvm-upstream/out$ ninja
...
rprichard@cashew:/x/llvm-upstream/out$ ninja check-cxx
[0/1] Running libcxx tests
llvm-lit: /x/llvm-upstream/llvm-project/libcxx/utils/libcxx/test/newconfig.py:23: note: Applied 'add Lit feature target=x86_64-unknown-linux-gnu' as a result of parameter 'target_triple=x86_64-unknown-linux-gnu'
llvm-lit: /x/llvm-upstream/llvm-project/llvm/utils/lit/lit/TestingConfig.py:103: fatal: unable to parse config file '/x/llvm-upstream/out/libcxx/test/lit.site.cfg', traceback: Traceback (most recent call last):
  File "/x/llvm-upstream/llvm-project/llvm/utils/lit/lit/TestingConfig.py", line 92, in load_from_path
    exec(compile(data, path, 'exec'), cfg_globals, None)
  File "/x/llvm-upstream/out/libcxx/test/lit.site.cfg", line 21, in <module>
    libcxx.test.newconfig.configure(
  File "/x/llvm-upstream/llvm-project/libcxx/utils/libcxx/test/newconfig.py", line 22, in configure
    action.applyTo(config)
  File "/x/llvm-upstream/llvm-project/libcxx/utils/libcxx/test/dsl.py", line 400, in applyTo
    if hasCompileFlag(config, flag):
  File "/x/llvm-upstream/llvm-project/libcxx/utils/libcxx/test/dsl.py", line 64, in f
    cache[cacheKey] = function(config, *args, **kwargs)
  File "/x/llvm-upstream/llvm-project/libcxx/utils/libcxx/test/dsl.py", line 203, in hasCompileFlag
    out, err, exitCode, timeoutInfo = _executeScriptInternal(test, [
  File "/x/llvm-upstream/llvm-project/libcxx/utils/libcxx/test/dsl.py", line 80, in _executeScriptInternal
    litConfig = lit.LitConfig.LitConfig(
TypeError: __init__() missing 1 required positional argument: 'order'

FAILED: libcxx/test/CMakeFiles/check-cxx /x/llvm-upstream/out/libcxx/test/CMakeFiles/check-cxx 
cd /x/llvm-upstream/out/libcxx/test && /usr/bin/python3.9 /x/llvm-upstream/out/bin/llvm-lit -sv --show-xfail --show-unsupported /x/llvm-upstream/out/libcxx/test
ninja: build stopped: subcommand failed.

[ychen]

In D122251#3446870, @rprichard wrote:

I think this change broke libc++ tests. _executeScriptInternal in libcxx/utils/libcxx/test/dsl.py does not pass an argument for LitConfig's new order parameter:

rprichard@cashew:/x/llvm-upstream/out$ cmake -GNinja -S ../llvm-project/runtimes -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" -DLIBCXXABI_USE_LLVM_UNWINDER=ON
...
rprichard@cashew:/x/llvm-upstream/out$ ninja
...
rprichard@cashew:/x/llvm-upstream/out$ ninja check-cxx
[0/1] Running libcxx tests
llvm-lit: /x/llvm-upstream/llvm-project/libcxx/utils/libcxx/test/newconfig.py:23: note: Applied 'add Lit feature target=x86_64-unknown-linux-gnu' as a result of parameter 'target_triple=x86_64-unknown-linux-gnu'
llvm-lit: /x/llvm-upstream/llvm-project/llvm/utils/lit/lit/TestingConfig.py:103: fatal: unable to parse config file '/x/llvm-upstream/out/libcxx/test/lit.site.cfg', traceback: Traceback (most recent call last):
  File "/x/llvm-upstream/llvm-project/llvm/utils/lit/lit/TestingConfig.py", line 92, in load_from_path
    exec(compile(data, path, 'exec'), cfg_globals, None)
  File "/x/llvm-upstream/out/libcxx/test/lit.site.cfg", line 21, in <module>
    libcxx.test.newconfig.configure(
  File "/x/llvm-upstream/llvm-project/libcxx/utils/libcxx/test/newconfig.py", line 22, in configure
    action.applyTo(config)
  File "/x/llvm-upstream/llvm-project/libcxx/utils/libcxx/test/dsl.py", line 400, in applyTo
    if hasCompileFlag(config, flag):
  File "/x/llvm-upstream/llvm-project/libcxx/utils/libcxx/test/dsl.py", line 64, in f
    cache[cacheKey] = function(config, *args, **kwargs)
  File "/x/llvm-upstream/llvm-project/libcxx/utils/libcxx/test/dsl.py", line 203, in hasCompileFlag
    out, err, exitCode, timeoutInfo = _executeScriptInternal(test, [
  File "/x/llvm-upstream/llvm-project/libcxx/utils/libcxx/test/dsl.py", line 80, in _executeScriptInternal
    litConfig = lit.LitConfig.LitConfig(
TypeError: __init__() missing 1 required positional argument: 'order'

FAILED: libcxx/test/CMakeFiles/check-cxx /x/llvm-upstream/out/libcxx/test/CMakeFiles/check-cxx 
cd /x/llvm-upstream/out/libcxx/test && /usr/bin/python3.9 /x/llvm-upstream/out/bin/llvm-lit -sv --show-xfail --show-unsupported /x/llvm-upstream/out/libcxx/test
ninja: build stopped: subcommand failed.

Fixed in 81b51b61f849fa91bdf5d069591

[rprichard]

In D122251#3446881, @ychen wrote:

In D122251#3446870, @rprichard wrote:

  File "/x/llvm-upstream/llvm-project/libcxx/utils/libcxx/test/dsl.py", line 80, in _executeScriptInternal
    litConfig = lit.LitConfig.LitConfig(
TypeError: __init__() missing 1 required positional argument: 'order'

Fixed in 81b51b61f849fa91bdf5d069591

Thanks, looks fixed to me.

[ldionne]

In D122251#3446908, @rprichard wrote:

In D122251#3446881, @ychen wrote:

In D122251#3446870, @rprichard wrote:

  File "/x/llvm-upstream/llvm-project/libcxx/utils/libcxx/test/dsl.py", line 80, in _executeScriptInternal
    litConfig = lit.LitConfig.LitConfig(
TypeError: __init__() missing 1 required positional argument: 'order'

Fixed in 81b51b61f849fa91bdf5d069591

Thanks, looks fixed to me.

Yup, thanks. And sorry @ychen for this piece of tech debt that we carry around in libc++. I'll try removing it in the future when/if I get some time.

[vitalybuka]

llvm/utils/lit/lit/formats/googletest.py

97

Why it uses env variables when gtest has has command line flags for that? Seems flags are more portable and easier to work with emulators.

106

'Script' used to be convenient oneliner which can be copy/pasted into shell for debugging, now it's not.
also GTEST_OUTPUT and GTEST_COLOR are not needed for that purpose.

[ychen]

llvm/utils/lit/lit/formats/googletest.py

106

GTEST_SHARD_INDEX/GTEST_TOTAL_SHARDS only have env var version. To be consistent, I made them all env vars. This script is for the failing shard (there is another one for the individual tests down below). If an error could only be reproduced this way, there are some hidden dependencies among tests that are not expected. In practice, I think this script is less used than the one below.

One option is to output them in one line. At least this is user-friendly on Linux. Does that sound good to you?

[vitalybuka]

llvm/utils/lit/lit/formats/googletest.py

106

I am sorry, you are correct, only env of them.
One liner would be nice
Probably we should drop GTEST_COLOR, GTEST_SHUFFLE and GTEST_OUTPUT that case to make it shorter.

[ahatanak]

It looks like this commit is causing https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-expensive/22234/testReport/junit/ThreadSanitizer-Unit/rtl___TsanRtlTest-x86_64-Test_ThreadSanitizer/RaceWithDeadThread/ to fail. Can you take a look?

[ychen]

In D122251#3462980, @ahatanak wrote:

It looks like this commit is causing https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-expensive/22234/testReport/junit/ThreadSanitizer-Unit/rtl___TsanRtlTest-x86_64-Test_ThreadSanitizer/RaceWithDeadThread/ to fail. Can you take a look?

Sure. I'll take a look today.

[sammccall]

After this change, failure details from unittests are not printed both locally and on bots: https://lab.llvm.org/buildbot/#/builders/109/builds/37071
(Assuming this is a bug that can be fixed)

Also the summary of "failed tests" at the end is now not useful:

Failed Tests (6):
  Clangd Unit Tests :: ./ClangdTests/114/134
  Clangd Unit Tests :: ./ClangdTests/115/134
  Clangd Unit Tests :: ./ClangdTests/128/134
  Clangd Unit Tests :: ./ClangdTests/42/134
  Clangd Unit Tests :: ./ClangdTests/78/134
  Clangd Unit Tests :: ./ClangdTests/87/134

This is critical information (e.g. to pick the simplest test to start debugging) and having to scan through the test outputs to try to find the actual failing test names is a large regression.
Is there a plan to address this?

[sammccall]

In D122251#3465326, @sammccall wrote:

After this change, failure details from unittests are not printed both locally and on bots: https://lab.llvm.org/buildbot/#/builders/109/builds/37071
(Assuming this is a bug that can be fixed)

Having read the change more carefully, it sounds like it's *expected* that a crashing unittest no longer prints a stack trace on stdout.

I think this is an unacceptable regression. Crashes/asserts are a common way that tests fail and the stack trace from a debug build is how we investigate them.
The rerun commands are hard to find, require manually rebuilding to iterate on, and don't filter output to the test in question as lit did (prior to this change).
Moreover, it's not possible to manually rerun the test on a bot, and many times the crash trace/assertion messages from buildbots have been *critical* to debugging failures that only occur in certain buildbot configurations (e.g. platforms I don't have access to).
Finally, some crashes are rare/nondeterministic, and so rerunning is not always a good option.

I'm not familiar enough with this part of lit to suggest solutions here.
But can this be reverted or made an off-by-default option until it can provide equivalent information & convenience?

[ychen]

In D122251#3465359, @sammccall wrote:

In D122251#3465326, @sammccall wrote:

After this change, failure details from unittests are not printed both locally and on bots: https://lab.llvm.org/buildbot/#/builders/109/builds/37071
(Assuming this is a bug that can be fixed)

Having read the change more carefully, it sounds like it's *expected* that a crashing unittest no longer prints a stack trace on stdout.

I think this is an unacceptable regression. Crashes/asserts are a common way that tests fail and the stack trace from a debug build is how we investigate them.
The rerun commands are hard to find, require manually rebuilding to iterate on, and don't filter output to the test in question as lit did (prior to this change).
Moreover, it's not possible to manually rerun the test on a bot, and many times the crash trace/assertion messages from buildbots have been *critical* to debugging failures that only occur in certain buildbot configurations (e.g. platforms I don't have access to).
Finally, some crashes are rare/nondeterministic, and so rerunning is not always a good option.

I'm not familiar enough with this part of lit to suggest solutions here.
But can this be reverted or made an off-by-default option until it can provide equivalent information & convenience?

Thanks for the feedback. I think there is a way to address this. Give me one day or two.

[sammccall]

In D122251#3465599, @ychen wrote:

I'm not familiar enough with this part of lit to suggest solutions here.
But can this be reverted or made an off-by-default option until it can provide equivalent information & convenience?

Thanks for the feedback. I think there is a way to address this. Give me one day or two.

Thanks for looking into it.
And the test speed impressive & fantastic, thanks for working on it!

[michaelplatings]

But can this be reverted or made an off-by-default option until it can provide equivalent information & convenience?

I appreciate the speed improvement on Windows, but I'd like to second @sammccall's request.
Currently I'm intermittently getting the output below from my CI, and it's too opaque.

Script(shard):
--
GTEST_COLOR=no
GTEST_SHUFFLE=0
GTEST_TOTAL_SHARDS=83
GTEST_SHARD_INDEX=69
GTEST_OUTPUT=json:/path/to/clang/unittests/Tooling/./ToolingTests-Clang-Unit-54664-69-83.json
/path/to/clang/unittests/Tooling/./ToolingTests
--
shard JSON output does not exist: /path/to/clang/unittests/Tooling/./ToolingTests-Clang-Unit-54664-69-83.json

Something, somewhere is sometimes, somehow causing a crash but the information the test system provides is not helpful to figuring out what that is.
Given the choice between faster opaque tests and slower, more helpful ones I'd like the slower ones back please.

[ychen]

In D122251#3472718, @michaelplatings wrote:

But can this be reverted or made an off-by-default option until it can provide equivalent information & convenience?

I appreciate the speed improvement on Windows, but I'd like to second @sammccall's request.
Currently I'm intermittently getting the output below from my CI, and it's too opaque.

Script(shard):
--
GTEST_COLOR=no
GTEST_SHUFFLE=0
GTEST_TOTAL_SHARDS=83
GTEST_SHARD_INDEX=69
GTEST_OUTPUT=json:/path/to/clang/unittests/Tooling/./ToolingTests-Clang-Unit-54664-69-83.json
/path/to/clang/unittests/Tooling/./ToolingTests
--
shard JSON output does not exist: /path/to/clang/unittests/Tooling/./ToolingTests-Clang-Unit-54664-69-83.json

Something, somewhere is sometimes, somehow causing a crash but the information the test system provides is not helpful to figuring out what that is.
Given the choice between faster opaque tests and slower, more helpful ones I'd like the slower ones back please.

The patch to address this has just been committed (d3efa577f549760a42ba68bda95d1e). I'm still waiting for the CI results. Looks good so far. :-)

[michaelplatings]

That's working perfectly for me. Great improvement, thank you very much!

[nigelp-xmos]

This has changed test totals reported on the buildbot dashboard, so I would be interested in a comment on that.

As others have mentioned, some tests are now reported as a kind of summary. For example I used to get 1332 lines of "ADTTests" like:

LLVM-Unit :: ADT/./ADTTests/AnyTest.BadAnyCast

and now get three lines in total:

LLVM-Unit :: ADT/./ADTTests/0/3
LLVM-Unit :: ADT/./ADTTests/1/3
LLVM-Unit :: ADT/./ADTTests/2/3
(3-core VM)

Before this change, the XCore buildbot totals on the buildbot staging dashboard matched the totals at the end of the "check-all" log:

https://lab.llvm.org/staging/#/builders/145/builds/5438

But after this change, the dashboard totals do not match the totals at the end of the log. The passed total is down by 20,000 and "skipped" (skipped Google unit tests) is not reported at all:

https://lab.llvm.org/staging/#/builders/145/builds/5439

I think the buildbot dashboard report counts log lines, rather than just displaying the totals at the end of the log.

A green buildbot with the change is clang-ppc64le-linux-multistage on main buildbot master:

https://lab.llvm.org/buildbot/#/builders/121/builds/18347
https://lab.llvm.org/buildbot/#/builders/121/builds/18350

Again the number of "expected passes" does not match the end of the log, "skipped" is not on the dashboard, and the log has numbers instead of test names.

(I tried the follow-up commit but it does not give test names of passing tests.)

Is this all intended and/or expected? What do you think about the buildbot totals?

Thanks,
Nigel

[ychen]

In D122251#3490585, @nigelp-xmos wrote:

This has changed test totals reported on the buildbot dashboard, so I would be interested in a comment on that.

As others have mentioned, some tests are now reported as a kind of summary. For example I used to get 1332 lines of "ADTTests" like:

LLVM-Unit :: ADT/./ADTTests/AnyTest.BadAnyCast

and now get three lines in total:

LLVM-Unit :: ADT/./ADTTests/0/3
LLVM-Unit :: ADT/./ADTTests/1/3
LLVM-Unit :: ADT/./ADTTests/2/3
(3-core VM)

Before this change, the XCore buildbot totals on the buildbot staging dashboard matched the totals at the end of the "check-all" log:

https://lab.llvm.org/staging/#/builders/145/builds/5438

But after this change, the dashboard totals do not match the totals at the end of the log. The passed total is down by 20,000 and "skipped" (skipped Google unit tests) is not reported at all:

https://lab.llvm.org/staging/#/builders/145/builds/5439

I think the buildbot dashboard report counts log lines, rather than just displaying the totals at the end of the log.

A green buildbot with the change is clang-ppc64le-linux-multistage on main buildbot master:

https://lab.llvm.org/buildbot/#/builders/121/builds/18347
https://lab.llvm.org/buildbot/#/builders/121/builds/18350

Again the number of "expected passes" does not match the end of the log, "skipped" is not on the dashboard, and the log has numbers instead of test names.

(I tried the follow-up commit but it does not give test names of passing tests.)

Is this all intended and/or expected? What do you think about the buildbot totals?

Thanks,
Nigel

This is all intended. The buildbot totals are counting the number of shards across all unit tests. The summary at the end of the log is counting the number of test cases like before. I think it is possible to let the buildbot show the summary but I'd understand the use case before doing that. Are you blocked in some way due to the mismatch?

[probinson]

The buildbot totals are counting the number of shards across all unit tests. The summary at the end of the log is counting the number of test cases like before.

lit used to report (or include in the report) the number of individual test functions across all the unittests. Now I believe it is reporting the number of shards, instead? I should think the code in lit that parses googletest output should be able to extract the number of tests executed by each shard (and if not, it could be upgraded to do so). That would recover the number that people have been expecting to see (both directly from lit, from buildbots, and possibly from metrics recorded elsewhere).

I wouldn't think it was a big deal, except that total number is in the tens of thousands. I'm sure it is very surprising to people to have that huge drop.

[ychen]

In D122251#3491815, @probinson wrote:

The buildbot totals are counting the number of shards across all unit tests. The summary at the end of the log is counting the number of test cases like before.

lit used to report (or include in the report) the number of individual test functions across all the unittests. Now I believe it is reporting the number of shards, instead? I should think the code in lit that parses googletest output should be able to extract the number of tests executed by each shard (and if not, it could be upgraded to do so). That would recover the number that people have been expecting to see (both directly from lit, from buildbots, and possibly from metrics recorded elsewhere).

Yeah, I attempted this before posting the patch. It is not trivial. There is one shared results vector across all individual jobs in the thread pool and the LIT thread pool is multiprocess instead of multithreaded. Extracting the results inside a shard earlier would incur some nontrivial overhead due to IPC and from my initial attempt, the required source code changes are pretty intrusive so I ended up deferring the shard results extraction until the test reporting phase (which is after the thread pool execution is done.)

[nigelp-xmos]

Are you blocked in some way due to the mismatch?

No, not blocked at all.

[thakis]

Another effect of this: If a unit test hangs forever, killing the process now no longer prints the hanging test name in the results. (Example: http://45.33.8.238/linux/76575/step_9.txt , where clangd unit tests ran for a very long time, until I killed that process.) I'm guessing it's https://github.com/llvm/llvm-project/issues/48455 since we have this known, rare, hang in those tests -- but if I didn't know about that issue, I'd be stumped.

What's the recommendation for finding out which test a test process is hanging in now?

[ychen]

In D122251#3528961, @thakis wrote:

Another effect of this: If a unit test hangs forever, killing the process now no longer prints the hanging test name in the results. (Example: http://45.33.8.238/linux/76575/step_9.txt , where clangd unit tests ran for a very long time, until I killed that process.) I'm guessing it's https://github.com/llvm/llvm-project/issues/48455 since we have this known, rare, hang in those tests -- but if I didn't know about that issue, I'd be stumped.

What's the recommendation for finding out which test a test process is hanging in now?

The last test is TUSchedulerTests.Cancellation in the shard. Do you mean it is not `TUSchedulerTests.Cancellation that hangs?

[thakis]

The last test is TUSchedulerTests.Cancellation in the shard. Do you mean it is not `TUSchedulerTests.Cancellation that hangs?

I'm just bad at reading. Thanks, that _is_ easy to see :) Sorry for the noise.

[vitalybuka]

Not sure when it's started but, but likely related to sharding

ninja check-all exits with 0 if only sharded test fail

LIT_FILTER=Unit ninja check-all || echo 1

FAIL: LLVM-Unit :: Analysis/./AnalysisTests/29/157 (1459 of 6195)
******************** TEST 'LLVM-Unit :: Analysis/./AnalysisTests/29/157' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:/usr/local/google/home/vitalybuka/tmp/bot/llvm_build_asan/unittests/Analysis/./AnalysisTests-LLVM-Unit-1704385-29-157.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=157 GTEST_SHARD_INDEX=29 /usr/local/google/home/vitalybuka/tmp/bot/llvm_build_asan/unittests/Analysis/./AnalysisTests
--


********************

Testing Time: 21.39s
  Excluded   : 62250
  Skipped    :    38
  Unsupported:     4
  Passed     : 24625
vvv:~/tmp/bot/llvm_build_asan$

check stage2/asan check of https://lab.llvm.org/buildbot/#/builders/5/builds/26144
This bot is broken for a while, but we didn't see that

[vitalybuka]

Also there is not output of failure
previously we had something

GTEST_OUTPUT=json:/usr/local/google/home/vitalybuka/tmp/bot/llvm_build_asan/unittests/Analysis/./AnalysisTests-LLVM-Unit-1704385-29-157.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=157 GTEST_SHARD_INDEX=29 /usr/local/google/home/vitalybuka/tmp/bot/llvm_build_asan/unittests/Analysis/./AnalysisTests
Note: This is test shard 30 of 157.
[==========] Running 4 tests from 4 test suites.
[----------] Global test environment set-up.
[----------] 1 test from IsPotentiallyReachableTest
[ RUN      ] IsPotentiallyReachableTest.SimpleLoop1
[       OK ] IsPotentiallyReachableTest.SimpleLoop1 (0 ms)
[----------] 1 test from IsPotentiallyReachableTest (0 ms total)

[----------] 1 test from IRSimilarityIdentifier
[ RUN      ] IRSimilarityIdentifier.IntrinsicCommutative
[       OK ] IRSimilarityIdentifier.IntrinsicCommutative (1 ms)
[----------] 1 test from IRSimilarityIdentifier (1 ms total)

[----------] 1 test from ScalarEvolutionsTest
[ RUN      ] ScalarEvolutionsTest.ComputeMaxTripCountFromMultiDemArray
[       OK ] ScalarEvolutionsTest.ComputeMaxTripCountFromMultiDemArray (1 ms)
[----------] 1 test from ScalarEvolutionsTest (1 ms total)

[----------] 1 test from ComputeKnownBitsTest
[ RUN      ] ComputeKnownBitsTest.ComputeKnownBitsAddWithRangeNoOverlap
[       OK ] ComputeKnownBitsTest.ComputeKnownBitsAddWithRangeNoOverlap (0 ms)
[----------] 1 test from ComputeKnownBitsTest (0 ms total)

[----------] Global test environment tear-down
[==========] 4 tests from 4 test suites ran. (3 ms total)
[  PASSED  ] 4 tests.

=================================================================
==1767424==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 80 byte(s) in 1 object(s) allocated from:
    #0 0xc9f0b7 in operator new(unsigned long) /usr/local/google/home/vitalybuka/tmp/bot/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:95:3
    #1 0xd1e63f in (anonymous namespace)::IsPotentiallyReachableTest::ExpectPath(bool)::IsPotentiallyReachableTestPass::initialize() /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/llvm/unittests/Analysis/CFGTest.cpp:80:25
    #2 0xd1c5df in (anonymous namespace)::IsPotentiallyReachableTest::ExpectPath(bool) /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/llvm/unittests/Analysis/CFGTest.cpp:117:29
    #3 0x230b53c in HandleExceptionsInMethodIfSupported<testing::Test, void> /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc
    #4 0x230b53c in testing::Test::Run() /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:2508:5
    #5 0x230fbab in testing::TestInfo::Run() /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:2684:11
    #6 0x231103f in testing::TestSuite::Run() /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:2816:28
    #7 0x233d3ea in testing::internal::UnitTestImpl::RunAllTests() /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:5338:44
    #8 0x233be4c in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc
    #9 0x233be4c in testing::UnitTest::Run() /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:4925:10
    #10 0x22eea08 in RUN_ALL_TESTS /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:2472:46
    #11 0x22eea08 in main /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/llvm/utils/unittest/UnitTestMain/TestMain.cpp:55:10
    #12 0x7f5876dd37fc in __libc_start_main csu/../csu/libc-start.c:332:16

SUMMARY: AddressSanitizer: 80 byte(s) leaked in 1 allocation(s).

[sammccall]

In D122251#3654458, @vitalybuka wrote:

Also there is not output of failure
previously we had something

I've also seen this recently when trying to debug a tsan failure: we see that the test shard fails, but no failure details (and so no idea which test *case* to associate with).

https://lab.llvm.org/buildbot/#/builders/131/builds/30528/steps/6/logs/stdio

I also don't know whether it's this patch or a more recent regression, as I haven't had cause to investigate a santitizer failure in a while.

[ychen]

In D122251#3654496, @sammccall wrote:

In D122251#3654458, @vitalybuka wrote:

Also there is not output of failure
previously we had something

I've also seen this recently when trying to debug a tsan failure: we see that the test shard fails, but no failure details (and so no idea which test *case* to associate with).

https://lab.llvm.org/buildbot/#/builders/131/builds/30528/steps/6/logs/stdio

I also don't know whether it's this patch or a more recent regression, as I haven't had cause to investigate a santitizer failure in a while.

@vitalybuka Thanks for the link. I'll take a look today.

[ychen]

In D122251#3655769, @ychen wrote:

In D122251#3654496, @sammccall wrote:

In D122251#3654458, @vitalybuka wrote:

Also there is not output of failure
previously we had something

I've also seen this recently when trying to debug a tsan failure: we see that the test shard fails, but no failure details (and so no idea which test *case* to associate with).

https://lab.llvm.org/buildbot/#/builders/131/builds/30528/steps/6/logs/stdio

I also don't know whether it's this patch or a more recent regression, as I haven't had cause to investigate a santitizer failure in a while.

@vitalybuka Thanks for the link. I'll take a look today.

Should've been fixed by 4cd1c96d375aaaf8b48f0fddc8bd0a6619ff061c

[ychen]

In D122251#3654496, @sammccall wrote:

In D122251#3654458, @vitalybuka wrote:

Also there is not output of failure
previously we had something

I've also seen this recently when trying to debug a tsan failure: we see that the test shard fails, but no failure details (and so no idea which test *case* to associate with).

https://lab.llvm.org/buildbot/#/builders/131/builds/30528/steps/6/logs/stdio

I also don't know whether it's this patch or a more recent regression, as I haven't had cause to investigate a santitizer failure in a while.

@sammccall Looks like a data race in [ RUN ] TUSchedulerTests.PreambleThrottle (https://lab.llvm.org/buildbot/#/builders/131/builds/30569/steps/6/logs/FAIL__Clangd_Unit_Tests__18).

[yln]

@ychen Is there a nob to disable the sharding for unit tests?

[ychen]

In D122251#3730557, @yln wrote:

@ychen Is there a nob to disable the sharding for unit tests?

Not at the moment. Two approaches are possible: 1. (easy) force a shard size of 1 so tests do not affect each other; 2. (more involved) add a knob to have the original behavior. I hope we choose 2 as the last the resort.

llvm/unittests/Support/CrashRecoveryTest.cpp

std::string Executable =
       sys::fs::getMainExecutable(TestMainArgv0, &CrashTestStringArg1);
   StringRef argv[] = {
       Executable, "--gtest_filter=CrashRecoveryTest.UnixCRCReturnCode"};
 
   // Add LLVM_CRC_UNIXCRCRETURNCODE to the environment of the child process.
   int Res = setenv("LLVM_CRC_UNIXCRCRETURNCODE", "1", 0);
   ASSERT_EQ(Res, 0);
 
+  Res = unsetenv("GTEST_SHARD_INDEX");
+  ASSERT_EQ(Res, 0);
+  Res = unsetenv("GTEST_TOTAL_SHARDS");
+  ASSERT_EQ(Res, 0);
+
   std::string Error;
   bool ExecutionFailed;
   int RetCode = ExecuteAndWait(Executable, argv, {}, {}, 0, 0, &Error,
                                &ExecutionFailed);
   ASSERT_EQ(-2, RetCode);
 }
 #endif

llvm/unittests/Support/ProgramTest.cpp

#if defined(_WIN32)
       return EnvStorage.back();
 #else
       (void)this;
       return StringRef(Var);
 #endif
     };
 
     while (*EnvP != nullptr) {
-      EnvTable.emplace_back(prepareEnvVar(*EnvP));
+      auto S = prepareEnvVar(*EnvP);
+      if (!StringRef(S).startswith("GTEST_"))
+        EnvTable.emplace_back(S);
       ++EnvP;
     }
   }
 
   void TearDown() override {
     EnvTable.clear();
     EnvStorage.clear();
   }

llvm/utils/lit/lit/Test.py

def getSourcePath(self, components):
         return os.path.join(self.source_root, *components)
 
     def getExecPath(self, components):
         return os.path.join(self.exec_root, *components)
 
 class Test:
     """Test - Information on a single test instance."""
 
-    def __init__(self, suite, path_in_suite, config, file_path = None):
+    def __init__(self, suite, path_in_suite, config, file_path = None, gtest_json_file = None):
         self.suite = suite
         self.path_in_suite = path_in_suite
         self.config = config
         self.file_path = file_path
+        self.gtest_json_file = gtest_json_file
 
         # A list of conditions under which this test is expected to fail.
         # Each condition is a boolean expression of features and target
         # triple parts. These can optionally be provided by test format
         # handlers, and will be honored when the test result is supplied.
         self.xfails = []
 
         # If true, ignore all items in self.xfails.
def __init__(self, suite, path_in_suite, config, file_path = None, gtest_json_file = None):
         self.result = None
 
         # The previous test failure state, if applicable.
         self.previous_failure = False
 
         # The previous test elapsed time, if applicable.
         self.previous_elapsed = 0.0
 
-        if '/'.join(path_in_suite) in suite.test_times:
+        if suite.test_times and '/'.join(path_in_suite) in suite.test_times:
             time = suite.test_times['/'.join(path_in_suite)]
             self.previous_elapsed = abs(time)
             self.previous_failure = time < 0
 
 
     def setResult(self, result):
         assert self.result is None, "result already set"
         assert isinstance(result, Result), "unexpected result type"

llvm/utils/lit/lit/TestingConfig.py

def fromdefaults(litConfig):
 
         pass_vars = ['LIBRARY_PATH', 'LD_LIBRARY_PATH', 'SYSTEMROOT', 'TERM',
                      'CLANG', 'LLDB', 'LD_PRELOAD', 'ASAN_OPTIONS',
                      'UBSAN_OPTIONS', 'LSAN_OPTIONS', 'ADB', 'ANDROID_SERIAL',
                      'SSH_AUTH_SOCK', 'SANITIZER_IGNORE_CVE_2016_2143',
                      'TMPDIR', 'TMP', 'TEMP', 'TEMPDIR', 'AVRLIT_BOARD',
                      'AVRLIT_PORT', 'FILECHECK_OPTS', 'VCINSTALLDIR',
                      'VCToolsinstallDir', 'VSINSTALLDIR', 'WindowsSdkDir',
-                     'WindowsSDKLibVersion', 'SOURCE_DATE_EPOCH']
+                     'WindowsSDKLibVersion', 'SOURCE_DATE_EPOCH','GTEST_FILTER']
 
         if sys.platform == 'win32':
             pass_vars.append('COMSPEC')
             pass_vars.append('INCLUDE')
             pass_vars.append('LIB')
             pass_vars.append('PATHEXT')
             pass_vars.append('USERPROFILE')
             environment['PYTHONBUFFERED'] = '1'

llvm/utils/lit/lit/formats/googletest.py

 from __future__ import absolute_import
+import json
+import math
 import os
-import re
 import shlex
-import subprocess
 import sys
 
 import lit.Test
 import lit.TestRunner
 import lit.util
 from .base import TestFormat
 
 kIsWindows = sys.platform in ['win32', 'cygwin']
 
 class GoogleTest(TestFormat):
     def __init__(self, test_sub_dirs, test_suffix, run_under = []):
         self.test_sub_dirs = str(test_sub_dirs).split(';')
 
         # On Windows, assume tests will also end in '.exe'.
         exe_suffix = str(test_suffix)
         if kIsWindows:
             exe_suffix += '.exe'
 
         # Also check for .py files for testing purposes.
         self.test_suffixes = {exe_suffix, test_suffix + '.py'}
         self.run_under = run_under
 
-    def getGTestTests(self, path, litConfig, localConfig):
-        """getGTestTests(path) - [name]
-
-        Return the tests available in gtest executable.
-
-        Args:
-          path: String path to a gtest executable
-          litConfig: LitConfig instance
-          localConfig: TestingConfig instance"""
-
-        list_test_cmd = self.prepareCmd([path, '--gtest_list_tests'])
-
-        try:
-            output = subprocess.check_output(list_test_cmd,
-                                             env=localConfig.environment)
-        except subprocess.CalledProcessError as exc:
-            litConfig.warning(
-                "unable to discover google-tests in %r: %s. Process output: %s"
-                % (path, sys.exc_info()[1], exc.output))
-            # This doesn't look like a valid gtest file.  This can
-            # have a number of causes, none of them good.  For
-            # instance, we could have created a broken executable.
-            # Alternatively, someone has cruft in their test
-            # directory.  If we don't return a test here, then no
-            # failures will get reported, so return a dummy test name
-            # so that the failure is reported later.
-            yield 'failed_to_discover_tests_from_gtest'
-            return
-
-        upstream_prefix = re.compile('Running main\(\) from .*gtest_main\.cc')
-        nested_tests = []
-        for ln in output.splitlines(False):  # Don't keep newlines.
-            ln = lit.util.to_string(ln)
-
-            if upstream_prefix.fullmatch(ln):
-                # Upstream googletest prints this to stdout prior to running
-                # tests. LLVM removed that print statement in r61540, but we
-                # handle it here in case upstream googletest is being used.
-                continue
-
-            # The test name list includes trailing comments beginning with
-            # a '#' on some lines, so skip those. We don't support test names
-            # that use escaping to embed '#' into their name as the names come
-            # from C++ class and method names where such things are hard and
-            # uninteresting to support.
-            ln = ln.split('#', 1)[0].rstrip()
-            if not ln.lstrip():
-                continue
-
-            index = 0
-            while ln[index*2:index*2+2] == '  ':
-                index += 1
-            while len(nested_tests) > index:
-                nested_tests.pop()
-
-            ln = ln[index*2:]
-            if ln.endswith('.'):
-                nested_tests.append(ln)
-            elif any([name.startswith('DISABLED_')
-                      for name in nested_tests + [ln]]):
-                # Gtest will internally skip these tests. No need to launch a
-                # child process for it.
-                continue
-            else:
-                yield ''.join(nested_tests) + ln
+    def get_num_tests(self, path, localConfig):
+        cmd = [path, '--gtest_list_tests', '--gtest_filter=-*DISABLED_*']
+        if cmd[0].endswith('.py'):
+            cmd = [sys.executable] + cmd
+        out, _, exitCode = lit.util.executeCommand(cmd, env=localConfig.environment)
+        if exitCode == 0:
+            return sum(map(lambda line: line.startswith('  '), out.splitlines()))
+        return None
 
     def getTestsInDirectory(self, testSuite, path_in_suite,
                             litConfig, localConfig):
+        init_shard_size = 512 # number of tests in a shard
+        core_count = lit.util.usable_core_count()
         source_path = testSuite.getSourcePath(path_in_suite)
         for subdir in self.test_sub_dirs:
             dir_path = os.path.join(source_path, subdir)
             if not os.path.isdir(dir_path):
                 continue
             for fn in lit.util.listdir_files(dir_path,
                                              suffixes=self.test_suffixes):
                 # Discover the tests in this executable.
                 execpath = os.path.join(source_path, subdir, fn)
-                testnames = self.getGTestTests(execpath, litConfig, localConfig)
-                for testname in testnames:
-                    testPath = path_in_suite + (subdir, fn, testname)
+                num_tests = self.get_num_tests(execpath, 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
+                        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)
+                                            file_path=execpath,
+                                            gtest_json_file=json_file)
+                else:
+                    # This doesn't look like a valid gtest file.  This can
+                    # have a number of causes, none of them good.  For
+                    # instance, we could have created a broken executable.
+                    # Alternatively, someone has cruft in their test
+                    # directory.  If we don't return a test here, then no
+                    # failures will get reported, so return a dummy test name
+                    # so that the failure is reported later.
+                    testPath = path_in_suite + (subdir, fn, 'failed_to_discover_tests_from_gtest')
+                    yield lit.Test.Test(testSuite, testPath, localConfig, file_path=execpath)
 
     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
 
-        cmd = [testPath, '--gtest_filter=' + testName]
+        testName,total_shards = os.path.split(testName)
+        testName,shard_idx = os.path.split(testName)
+        shard_env = {'GTEST_COLOR':'no','GTEST_TOTAL_SHARDS':total_shards, 'GTEST_SHARD_INDEX':shard_idx, 'GTEST_OUTPUT':'json:'+test.gtest_json_file}
+        test.config.environment.update(shard_env)
+
+        cmd = [testPath]
         cmd = self.prepareCmd(cmd)
         if litConfig.useValgrind:
             cmd = litConfig.valgrindArgs + cmd
 
         if litConfig.noExecute:
             return lit.Test.PASS, ''
 
-        header = f"Script:\n--\n{' '.join(cmd)}\n--\n"
+        shard_envs= '\n'.join([k + '=' + v for k, v in shard_env.items()])
+        shard_header = f"Script(shard):\n--\n{shard_envs}\n{' '.join(cmd)}\n--\n"
 
         try:
-            out, err, exitCode = lit.util.executeCommand(
+            _, _, exitCode = lit.util.executeCommand(
                 cmd, env=test.config.environment,
                 timeout=litConfig.maxIndividualTestTime)
         except lit.util.ExecuteCommandTimeoutException:
             return (lit.Test.TIMEOUT,
-                    f'{header}Reached timeout of '
+                    f'{shard_header}Reached timeout of '
                     f'{litConfig.maxIndividualTestTime} seconds')
 
-        if exitCode:
-            return lit.Test.FAIL, header + out + err
-
-        if '[  SKIPPED ] 1 test,' in out:
-            return lit.Test.SKIPPED, ''
-
-        passing_test_line = '[  PASSED  ] 1 test.'
-        if passing_test_line not in out:
-            return (lit.Test.UNRESOLVED,
-                    f'{header}Unable to find {passing_test_line} '
-                    f'in gtest output:\n\n{out}{err}')
+        if not os.path.exists(test.gtest_json_file):
+            errmsg = f"shard JSON output does not exist: %s" % (test.gtest_json_file)
+            return lit.Test.FAIL, shard_header + errmsg
 
-        return lit.Test.PASS,''
+        if exitCode:
+            output = shard_header + '\n'
+            with open(test.gtest_json_file, encoding='utf-8') as f:
+                testsuites = json.load(f)['testsuites']
+                for testcase in testsuites:
+                    for testinfo in testcase['testsuite']:
+                        if testinfo['result'] == 'SUPPRESSED' or testinfo['result'] == 'SKIPPED':
+                            continue
+                        testname = testcase['name'] + '.' + testinfo['name']
+                        header = f"Script:\n--\n{' '.join(cmd)} --gtest_filter={testname}\n--\n"
+                        if 'failures' in testinfo:
+                            output += header
+                            for fail in testinfo['failures']:
+                                output += fail['failure'] + '\n'
+                            output += '\n'
+                        elif testinfo['result'] != 'COMPLETED':
+                            output += header
+                            output += 'unresolved test result\n'
+            return lit.Test.FAIL, output
+        else:
+            return lit.Test.PASS, ''
 
     def prepareCmd(self, cmd):
         """Insert interpreter if needed.
 
         It inserts the python exe into the command if cmd[0] ends in .py or caller
         specified run_under.
         We cannot rely on the system to interpret shebang lines for us on
         Windows, so add the python executable to the command if this is a .py
         script.
         """
         if cmd[0].endswith('.py'):
             cmd = [sys.executable] + cmd
         if self.run_under:
             if isinstance(self.run_under, list):
                 cmd = self.run_under + cmd
             else:
                 cmd = shlex.split(self.run_under) + cmd
         return cmd
+
+    @staticmethod
+    def post_process_shard_results(selected_tests, discovered_tests):
+        def remove_gtest(tests):
+            idxs = []
+            for idx, t in enumerate(tests):
+                if t.gtest_json_file:
+                    idxs.append(idx)
+            for i in range(len(idxs)):
+                del tests[idxs[i]-i]
+
+        remove_gtest(discovered_tests)
+        gtests = [t for t in selected_tests if t.gtest_json_file]
+        remove_gtest(selected_tests)
+        for test in gtests:
+            # In case gtest has bugs such that no JSON file was emitted.
+            if not os.path.exists(test.gtest_json_file):
+                selected_tests.append(test)
+                discovered_tests.append(test)
+                continue
+
+            # Load json file to retrieve results.
+            with open(test.gtest_json_file, encoding='utf-8') as f:
+                testsuites = json.load(f)['testsuites']
+                for testcase in testsuites:
+                    for testinfo in testcase['testsuite']:
+                        # Ignore disabled tests.
+                        if testinfo['result'] == 'SUPPRESSED':
+                            continue
+
+                        testPath = test.path_in_suite[:-2] + (testcase['name'], testinfo['name'])
+                        subtest = lit.Test.Test(test.suite, testPath,
+                                                test.config, test.file_path)
+
+                        testname = testcase['name'] + '.' + testinfo['name']
+                        header = f"Script:\n--\n{test.file_path} --gtest_filter={testname}\n--\n"
+
+                        output = ''
+                        if testinfo['result'] == 'SKIPPED':
+                            returnCode = lit.Test.SKIPPED
+                        elif 'failures' in testinfo:
+                            returnCode = lit.Test.FAIL
+                            output = header
+                            for fail in testinfo['failures']:
+                                output += fail['failure'] + '\n'
+                        elif testinfo['result'] == 'COMPLETED':
+                            returnCode = lit.Test.PASS
+                        else:
+                            returnCode = lit.Test.UNRESOLVED
+                            output = header + 'unresolved test result\n'
+
+                        subtest.setResult(lit.Test.Result(returnCode, output, float(testinfo['time'][:-1])))
+
+                        selected_tests.append(subtest)
+                        discovered_tests.append(subtest)
+            os.remove(test.gtest_json_file)
+
+        return selected_tests, discovered_tests

llvm/utils/lit/lit/main.py

 import lit.cl_arguments
 import lit.discovery
 import lit.display
 import lit.LitConfig
 import lit.reports
 import lit.run
 import lit.Test
 import lit.util
+from lit.formats.googletest import GoogleTest
 from lit.TestTimes import record_test_times
 
 
 def main(builtin_params={}):
     opts = lit.cl_arguments.parse_args()
     params = create_params(builtin_params, opts.user_params)
     is_windows = platform.system() == 'Windows'
 
def main(builtin_params={}):
     mark_excluded(discovered_tests, selected_tests)
 
     start = time.time()
     run_tests(selected_tests, lit_config, opts, len(discovered_tests))
     elapsed = time.time() - start
 
     record_test_times(selected_tests, lit_config)
 
+    selected_tests, discovered_tests = GoogleTest.post_process_shard_results(
+        selected_tests, discovered_tests)
+
     if opts.time_tests:
         print_histogram(discovered_tests)
 
     print_results(discovered_tests, elapsed, opts)
 
     for report in opts.reports:
         report.write_results(tests_for_report, elapsed)
 
     if lit_config.numErrors:
         sys.stderr.write('\n%d error(s) in tests\n' % lit_config.numErrors)
         sys.exit(2)
 
     if lit_config.numWarnings:
         sys.stderr.write('\n%d warning(s) in tests\n' % lit_config.numWarnings)
 
     has_failure = any(t.isFailure() for t in discovered_tests)
     if has_failure:
         if opts.ignoreFail:
             sys.stderr.write("\nExiting with status 0 instead of 1 because "
                              "'--ignore-fail' was specified.\n")
         else:
             sys.exit(1)
 
 def create_params(builtin_params, user_params):
     def parse(p):
         return p.split('=', 1) if '=' in p else (p, '')
 
     params = dict(builtin_params)
     params.update([parse(p) for p in user_params])
     return params
 
 
 def print_discovered(tests, show_suites, show_tests):
     tests.sort(key=lit.reports.by_suite_and_test_path)
 
     if show_suites:
         tests_by_suite = itertools.groupby(tests, lambda t: t.suite)
         print('-- Test Suites --')
         for suite, test_iter in tests_by_suite:
             test_count = sum(1 for _ in test_iter)
             print('  %s - %d tests' % (suite.name, test_count))
             print('    Source Root: %s' % suite.source_root)
             print('    Exec Root  : %s' % suite.exec_root)
             features = ' '.join(sorted(suite.config.available_features))
             print('    Available Features: %s' % features)
             substitutions = sorted(suite.config.substitutions)
             substitutions = ('%s => %s' % (x, y) for (x, y) in substitutions)
             substitutions = '\n'.ljust(30).join(substitutions)
             print('    Available Substitutions: %s' % substitutions)
 
     if show_tests:
         print('-- Available Tests --')
         for t in tests:
             print('  %s' % t.getFullName())
 
 
 def determine_order(tests, order):
     from lit.cl_arguments import TestOrder
     enum_order = TestOrder(order)
     if enum_order == TestOrder.RANDOM:
         import random
         random.shuffle(tests)
     elif enum_order == TestOrder.LEXICAL:
         tests.sort(key=lambda t: t.getFullName())
     else:
         assert enum_order == TestOrder.SMART, 'Unknown TestOrder value'
         tests.sort(key=lambda t: (not t.previous_failure, -t.previous_elapsed, t.getFullName()))

llvm/utils/lit/tests/Inputs/googletest-crash/DummySubDir/OneTest.py

+#!/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_TOTAL_SHARDS', 'GTEST_SHARD_INDEX', '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'])
+
+dummy_output = """\
+{
+"testsuites": [
+]
+}"""
+
+if os.environ['GTEST_SHARD_INDEX'] == '0':
+    exit_code = 1
+else:
+    json_filename = os.environ['GTEST_OUTPUT'].split(':', 1)[1]
+    with open(json_filename, 'w') as f:
+        f.write(dummy_output)
+    exit_code = 0
+
+sys.exit(exit_code)

llvm/utils/lit/tests/Inputs/googletest-crash/lit.cfg

 import lit.formats
-config.name = 'googletest-upstream-format'
+config.name = 'googletest-crash'
 config.test_format = lit.formats.GoogleTest('DummySubDir', 'Test')

llvm/utils/lit/tests/Inputs/googletest-format/DummySubDir/OneTest.py

 #!/usr/bin/env python
 
+import os
 import sys
 
-if len(sys.argv) != 2:
-    raise ValueError("unexpected number of args")
-
-if sys.argv[1] == "--gtest_list_tests":
+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 not sys.argv[1].startswith("--gtest_filter="):
-    raise ValueError("unexpected argument: %r" % (sys.argv[1]))
+elif len(sys.argv) != 1:
+    # sharding and json output are specified using environment variables
+    raise ValueError("unexpected argument: %r" % (' '.join(sys.argv[1:])))
 
-test_name = sys.argv[1].split('=',1)[1]
-if test_name == 'FirstTest.subTestA':
-    print('I am subTest A, I PASS')
-    print('[  PASSED  ] 1 test.')
-    sys.exit(0)
-elif test_name == 'FirstTest.subTestB':
-    print('I am subTest B, I FAIL')
-    print('And I have two lines of output')
-    sys.exit(1)
-elif test_name == 'FirstTest.subTestC':
-    print('I am subTest C, I am SKIPPED')
-    print('[  PASSED  ] 0 tests.')
-    print('[  SKIPPED ] 1 test, listed below:')
-    print('[  SKIPPED ] FirstTest.subTestC')
-    sys.exit(0)
-elif test_name == 'FirstTest.subTestD':
-    print('I am subTest D, I am UNRESOLVED')
-    sys.exit(0)
-elif test_name in ('ParameterizedTest/0.subTest',
-                   'ParameterizedTest/1.subTest'):
-    print('I am a parameterized test, I also PASS')
-    print('[  PASSED  ] 1 test.')
-    sys.exit(0)
+for e in ['GTEST_TOTAL_SHARDS', 'GTEST_SHARD_INDEX', '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 = """\
+{
+"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') as f:
+    if os.environ['GTEST_SHARD_INDEX'] == '0':
+        f.write(output)
+        exit_code = 1
     else:
-    raise SystemExit("error: invalid test name: %r" % (test_name,))
+        f.write(dummy_output)
+        exit_code = 0
+
+sys.exit(exit_code)

llvm/utils/lit/tests/Inputs/googletest-upstream-format/DummySubDir/OneTest.py

-#!/usr/bin/env python
-
-import os
-import sys
-
-if len(sys.argv) != 2:
-    raise ValueError("unexpected number of args")
-
-if sys.argv[1] == "--gtest_list_tests":
-    print(f"""\
-Running main() from {os.getcwd()}/gtest_main.cc
-FirstTest.
-  subTestA
-  subTestB
-  subTestC
-  subTestD
-ParameterizedTest/0.
-  subTest
-ParameterizedTest/1.
-  subTest""")
-    sys.exit(0)
-elif not sys.argv[1].startswith("--gtest_filter="):
-    raise ValueError("unexpected argument: %r" % (sys.argv[1]))
-
-test_name = sys.argv[1].split('=',1)[1]
-print('Running main() from gtest_main.cc')
-if test_name == 'FirstTest.subTestA':
-    print('I am subTest A, I PASS')
-    print('[  PASSED  ] 1 test.')
-    sys.exit(0)
-elif test_name == 'FirstTest.subTestB':
-    print('I am subTest B, I FAIL')
-    print('And I have two lines of output')
-    sys.exit(1)
-elif test_name == 'FirstTest.subTestC':
-    print('I am subTest C, I am SKIPPED')
-    print('[  PASSED  ] 0 tests.')
-    print('[  SKIPPED ] 1 test, listed below:')
-    print('[  SKIPPED ] FirstTest.subTestC')
-    sys.exit(0)
-elif test_name == 'FirstTest.subTestD':
-    print('I am subTest D, I am UNRESOLVED')
-    sys.exit(0)
-elif test_name in ('ParameterizedTest/0.subTest',
-                   'ParameterizedTest/1.subTest'):
-    print('I am a parameterized test, I also PASS')
-    print('[  PASSED  ] 1 test.')
-    sys.exit(0)
-else:
-    raise SystemExit("error: invalid test name: %r" % (test_name,))

llvm/utils/lit/tests/googletest-crash.py

+# Check GoogleTest shard test crashes are handled.
+
+# RUN: not %{lit} -v %{inputs}/googletest-crash | FileCheck %s
+
+# CHECK: -- Testing:
+# CHECK: FAIL: googletest-crash :: [[PATH:[Dd]ummy[Ss]ub[Dd]ir/]][[FILE:OneTest\.py]]/0
+# CHECK: *** TEST 'googletest-crash :: [[PATH]][[FILE]]/0{{.*}} FAILED ***
+# CHECK-NEXT: Script(shard):
+# CHECK-NEXT: --
+# CHECK-NEXT: GTEST_COLOR=no
+# CHECK-NEXT: GTEST_TOTAL_SHARDS=6
+# CHECK-NEXT: GTEST_SHARD_INDEX=0
+# CHECK-NEXT: GTEST_OUTPUT=json:[[JSON:.*\.json]]
+# CHECK-NEXT: [[FILE]]
+# CHECK-NEXT: --
+# CHECK-NEXT: shard JSON output does not exist: [[JSON]]
+# CHECK-NEXT: ***
+# CHECK: Failed Tests (1):
+# CHECK-NEXT:   googletest-crash :: [[PATH]][[FILE]]/0/6
+# CHECK: Failed{{ *}}: 1
 No newline at end of file

llvm/utils/lit/tests/googletest-format.py

 # Check the various features of the GoogleTest format.
 
 # RUN: not %{lit} -v %{inputs}/googletest-format > %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: PASS: googletest-format :: [[PATH:[Dd]ummy[Ss]ub[Dd]ir/]][[FILE:OneTest\.py]]/FirstTest.subTestA
-# CHECK: FAIL: googletest-format :: [[PATH]][[FILE]]/[[TEST:FirstTest\.subTestB]]
-# CHECK-NEXT: *** TEST 'googletest-format :: [[PATH]][[FILE]]/[[TEST]]' FAILED ***
+# CHECK: FAIL: googletest-format :: [[PATH:[Dd]ummy[Ss]ub[Dd]ir/]][[FILE:OneTest\.py]]/0
+# CHECK: *** TEST 'googletest-format :: [[PATH]][[FILE]]/0{{.*}} FAILED ***
+# CHECK-NEXT: Script(shard):
+# CHECK-NEXT: --
+# CHECK-NEXT: GTEST_COLOR=no
+# CHECK-NEXT: GTEST_TOTAL_SHARDS=6
+# CHECK-NEXT: GTEST_SHARD_INDEX=0
+# CHECK-NEXT: GTEST_OUTPUT=json:{{.*\.json}}
+# CHECK-NEXT: [[FILE]]
+# CHECK-NEXT: --
+# CHECK-EMPTY:
 # CHECK-NEXT: Script:
 # CHECK-NEXT: --
-# CHECK-NEXT: [[FILE]] --gtest_filter=[[TEST]]
+# CHECK-NEXT: [[FILE]] --gtest_filter=FirstTest.subTestB
 # CHECK-NEXT: --
 # CHECK-NEXT: I am subTest B, I FAIL
 # CHECK-NEXT: And I have two lines of output
-# CHECK: ***
-# CHECK: SKIPPED: googletest-format :: [[PATH]][[FILE]]/FirstTest.subTestC
-# CHECK: UNRESOLVED: googletest-format :: [[PATH]][[FILE]]/[[TEST:FirstTest\.subTestD]]
-# CHECK-NEXT: *** TEST 'googletest-format :: [[PATH]][[FILE]]/[[TEST]]' FAILED ***
-# CHECK-NEXT: Script:
+# CHECK-EMPTY:
+# CHECK: Script:
 # CHECK-NEXT: --
-# CHECK-NEXT: [[FILE]] --gtest_filter=[[TEST]]
+# CHECK-NEXT: [[FILE]] --gtest_filter=FirstTest.subTestD
 # CHECK-NEXT: --
-# CHECK-NEXT: Unable to find [ PASSED ] 1 test. in gtest output
-# CHECK: I am subTest D, I am UNRESOLVED
-# CHECK: PASS: googletest-format :: [[PATH]][[FILE]]/ParameterizedTest/0.subTest
-# CHECK: PASS: googletest-format :: [[PATH]][[FILE]]/ParameterizedTest/1.subTest
-# CHECK: Failed Tests (1)
+# CHECK-NEXT: unresolved test result
+# CHECK: ***
+# CHECK: Unresolved Tests (1):
+# CHECK-NEXT:   googletest-format :: [[PATH]][[FILE]]/FirstTest/subTestD
+# CHECK: ***
+# CHECK-NEXT: Failed Tests (1):
+# CHECK-NEXT:   googletest-format :: [[PATH]][[FILE]]/FirstTest/subTestB
 # CHECK: Skipped{{ *}}: 1
 # CHECK: Passed{{ *}}: 3
 # CHECK: Unresolved{{ *}}: 1
 # CHECK: Failed{{ *}}: 1

llvm/utils/lit/tests/googletest-upstream-format.py

-# Check the various features of the GoogleTest format.
-
-# RUN: not %{lit} -v %{inputs}/googletest-upstream-format > %t.out
-# RUN: FileCheck < %t.out %s
-#
-# END.
-
-# CHECK: -- Testing:
-# CHECK: PASS: googletest-upstream-format :: [[PATH:[Dd]ummy[Ss]ub[Dd]ir/]][[FILE:OneTest\.py]]/FirstTest.subTestA
-# CHECK: FAIL: googletest-upstream-format :: [[PATH]][[FILE]]/[[TEST:FirstTest\.subTestB]]
-# CHECK-NEXT: *** TEST 'googletest-upstream-format :: [[PATH]][[FILE]]/[[TEST]]' FAILED ***
-# CHECK-NEXT: Script:
-# CHECK-NEXT: --
-# CHECK-NEXT: [[FILE]] --gtest_filter=[[TEST]]
-# CHECK-NEXT: --
-# CHECK-NEXT: Running main() from gtest_main.cc
-# CHECK-NEXT: I am subTest B, I FAIL
-# CHECK-NEXT: And I have two lines of output
-# CHECK: SKIPPED: googletest-upstream-format :: [[PATH]][[FILE]]/FirstTest.subTestC
-# CHECK: UNRESOLVED: googletest-upstream-format :: [[PATH]][[FILE]]/[[TEST:FirstTest\.subTestD]]
-# CHECK-NEXT: *** TEST 'googletest-upstream-format :: [[PATH]][[FILE]]/[[TEST]]' FAILED ***
-# CHECK-NEXT: Script:
-# CHECK-NEXT: --
-# CHECK-NEXT: [[FILE]] --gtest_filter=[[TEST]]
-# CHECK-NEXT: --
-# CHECK-NEXT: Unable to find [ PASSED ] 1 test. in gtest output
-# CHECK: I am subTest D, I am UNRESOLVED
-# CHECK: ***
-# CHECK: PASS: googletest-upstream-format :: [[PATH]][[FILE]]/ParameterizedTest/0.subTest
-# CHECK: PASS: googletest-upstream-format :: [[PATH]][[FILE]]/ParameterizedTest/1.subTest
-# CHECK: Failed Tests (1)
-# CHECK: Skipped{{ *}}: 1
-# CHECK: Passed{{ *}}: 3
-# CHECK: Unresolved{{ *}}: 1
-# CHECK: Failed{{ *}}: 1