Support testsuite builds without LNT #245
Conversation
This adds support in Clang builder to run LLVM testsuite without LNT. This will help us enable LLVM WoA testsuite bot and also help us run LLVM testsuite without LNT on Linux.
|
@gkistanova @DavidSpickett friendly ping! |
|
I seem to recall that a major reason to do this was that LNT would not run on the version of Python that first got a native Windows on Arm build. Now that llvm/llvm-lnt@f987c12 has landed, is that a relevant concern? On Linux we have been able to run the test suite with Python 3.10.12 since that change went in. If there are other supporting reasons please list them. (reproducing failures would be a bit easier certainly) |
| logfiles={'configure.log' : 'build/configure.log', | ||
| 'build-tools.log' : 'build/build-tools.log', | ||
| 'test.log' : 'build/test.log', | ||
| 'report.json' : 'build/report.json'}, |
There was a problem hiding this comment.
What logs do we get from the cmake only test suite build? Is it just like a normal ninja check-all, stdio with the output, ending with a table of results?
There was a problem hiding this comment.
As most buildbots run these test for validation I have kept the log to mimic logs output generated by llvm-project's ninja check-all
If we run ninja check it will actually run llvm-lit -sv . which generates logs like we get in llvm-project ninja check-all.
For detailed logs We should run llvm-lit . with sv to get logs like following:
compile_time: 0.0323 exec_time: 0.0000 hash: "a787a4de011d3c645d10ccb5efa9df57" link_time: 0.0561 size: 25984 size..bss: 8 size..comment: 151 size..data: 24 size..data.rel.ro: 568 size..dynamic: 528 size..dynstr: 412 size..dynsym: 456 size..eh_frame: 1652 size..eh_frame_hdr: 404 size..fini: 20 size..fini_array: 8 size..gcc_except_table: 124 size..gnu.hash: 28 size..gnu.version: 38 size..gnu.version_r: 144 size..got: 48 size..got.plt: 120 size..init: 24 size..init_array: 8 size..interp: 27 size..note.ABI-tag: 32 size..plt: 224 size..rela.dyn: 1800 size..rela.plt: 288 size..rodata: 61 size..text: 5128 **********
There was a problem hiding this comment.
That part is fine, 99% of the time I'm looking at the "stdio" file anyway.
Is buildbot still able to parse the failures correctly and show the various failure types in the UI? (I forget if it parses the stdio or the json results file)
|
This patch looks quite invasive. Did you consider using a different build factory for your purpose? Like UnifiedTreeBuilder for example? |
The diff looks a bit invasive but if we examine the code there are not many changes to existing LNT based builder logic. It is just a if condition separation between LNT and No LNT logic. The if condition indentation presents like a lot has changed. Also I did consider other builders but ClangBuilder aligns well with our existing buildbots and infrastructure. |
| @@ -154,7 +154,7 @@ def getClangCMakeBuildFactory( | |||
| testsuite_flags=None, | |||
| submitURL=None, | |||
| testerName=None, | |||
|
|
|||
| testWithLNT=True, | |||
There was a problem hiding this comment.
test could mean ninja check or llvm-test-suite, perhaps change this to testsuite_use_lnt to match the other parameter? Then it's more clear what it changes.
| 'test.log' : 'build/test.log', | ||
| 'report.json' : 'build/report.json'}, | ||
| env=test_suite_env)) | ||
| # Add -k option to ninja command for keep building even if a test fails. |
|
For me reading this it's not that it's that invasive but that it adds to an already gigantic function. While there are advantages to having it all in one place like that, this code adding I would try breaking the if and else parts into standalone functions like |
|
I just stumbled onto https://github.com/llvm/llvm-zorg/blob/main/zorg/buildbot/builders/TestSuiteBuilder.py. Perhaps that's what you were referring to Galina. |
This adds support in Clang builder to run LLVM testsuite without LNT. This will help us enable LLVM WoA testsuite bot and also help us run LLVM testsuite without LNT on Linux.