[PATCH] D76829: [lit] Introduce setup and teardown routines

[Julian Lettner via Phabricator via llvm-commits]

yln added a comment.

I can already think of at least one other use case that currently doesn't have a good solution: setting up iOS simulator/devices before and after compiler-rt tests.

The only concern that I have is that in lit we have this concept of "suites" defined by lit configs.  These lit config now also register setup/teardown functions, but the functions get executed at the beginning/end of the overall lit command line invocation.  This is a mismatch. But maybe this is not important?! Do scenarios where setup matters already equate "lit invocation" with "test suite"?

This is what I am trying to depict:

  lit command line invocation - start
    suite 1 setup
    suite 2 setup
    suite 1
      test 1 in suite 1
      test 2 in suite 1
    suite 2
      test 1 in suite 2
      test 2 in suite 2
    suite 1 teardown
    suite 2 teardown
  lit command line invocation - end

Ideally, setup/teardown would execute around the suite that defined it.  We can't easily accomplish this because after test discovery, a "lit run" (run.py) is just the list of all discovered tests.

Still, from my perspective this is a good feature to have.  LGTM, with nits from my side.  I would like someone else to agree with me before landing though.



================
Comment at: llvm/utils/lit/lit/LitConfig.py:73
+    
+    def __getstate__(self):
+        # Don't pickle callbacks.
----------------
I think I can guess why this is required, but please explain it just to make sure I understand.


================
Comment at: llvm/utils/lit/lit/LitConfig.py:178
+    
+    def setup_callback(self, callback):
+        '''
----------------
Can we name this `suite_setup` to drive home the point that this is executed once, and not for every test?


================
Comment at: llvm/utils/lit/lit/LitConfig.py:208
+        for callback in self._setup_callbacks:
+            callback()
+    
----------------
Should we provide self (the lit_config) or is this implicitly captured on the caller side?


================
Comment at: llvm/utils/lit/lit/run.py:73
+    def _execute(self, deadline):
+        raise NotImplementedError("Should be implemented in a subclass")
 
----------------
My apologies for this.  The explicit modeling of serial and parallel runs is my fault.  That abstraction has not carried its weight.  I want to remove it in the future.


================
Comment at: llvm/utils/lit/tests/Inputs/setup-teardown/lit.cfg:10
+def setup():
+    print("Running setup code...")
+
----------------
Can this access the surrounding context?


================
Comment at: llvm/utils/lit/tests/setup-teardown.py:1
+# RUN: not %{lit} -j2 %{inputs}/setup-teardown | FileCheck %s
+
----------------
If you provide `-j1`, then you can skip the `-DAG` below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76829