[libcxx-commits] [PATCH] D154987: [lit] Implement PYTHON directive and config.prologue

[Vlad Serebrennikov via Phabricator via libcxx-commits]

Endill added a comment.

Thank you for coming up with the idea, and then implementing it!
Unfortunately, I can't review your implementation from lit standpoint, but it looks solid judging by documentation.



================
Comment at: llvm/docs/TestingGuide.rst:317-318
+commands in a single test file (:program:`lit` parallelism is only across test
+files), this approach can produce very slow tests. Unless full coverage is
+critical, it might be better to specify only a cross-section of such a problem
+space. Python functions can be useful for this purpose. For example, we might
----------------
I think we can provide an example for this case as well:
```
from itertools import product

stds = (98, 11, 14, 17, 20, 23)
triples = (
  'x86_64-apple-darwin10.6.0',
  'x86_64-unknown-linux-gnu'
)
for std, triple in product(stds, triples):
  lit.run(f'%clang_cc1 -std=c++{std} -triple {triple} %s')
```
But it could be better to hold it until we come up with a variant of `run` that runs in parallel with other runs, in order to avoid the issue you're describing.


================
Comment at: llvm/docs/TestingGuide.rst:377
+
+    ; PYTHON: exampleModule.lit = lit # exampleModule.py has: lit = None
+
----------------
I suggest to provide complete example, because description is somewhat difficult to follow, especially for people not too well-versed in Python.


================
Comment at: llvm/utils/lit/lit/TestRunner.py:2243-2244
+
+        # TODO: In the future, extend the 'lit' API to be able to *write*
+        # substitutions?
+
----------------
I don't see why we shouldn't provide such an API to PYTHON directives, but it's out of scope of this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154987