[PATCH] D153967: [lit] Remove the --no-indirectly-run-check option

Joel E. Denny via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 17 14:17:44 PDT 2023


jdenny accepted this revision.
jdenny added a comment.

Other than the comment I just requested, LGTM.  Thanks for the cleanup.



================
Comment at: llvm/utils/lit/lit/discovery.py:170-171
+        # tests.
+        tests = [Test.Test(ts, path_in_suite, lc)] if lc.test_format is None or lc.standalone_tests else \
                 lc.test_format.getTestsForPath(ts, path_in_suite, litConfig, lc)
 
----------------
ldionne wrote:
> jdenny wrote:
> > If I understand correctly, if a directory is in standalone mode because tests are always run individually, it is not possible to use a test format that overrides getTestsForPaths in order to generate multiple tests from each of those individual tests.  Is that correct?  I'm not saying that's problematic.  I just want to be sure I understand.
> Yes, that is correct. TBH I am also skeptical about the usefulness of "standalone" tests in the first place, it seems like they were introduced as a clutch for people not wanting to have appropriate suffixes and lit excludes in the first place.
> Yes, that is correct.

I feel like the inability to use standalone mode with `getTestsForPaths` in that manner ought to be documented somewhere.  What about at a `getTestsForPaths` definition in  `llvm/utils/lit/lit/formats/base.py`?

> TBH I am also skeptical about the usefulness of "standalone" tests in the first place, it seems like they were introduced as a clutch for people not wanting to have appropriate suffixes and lit excludes in the first place.

My understanding is that it's for use cases where lit is not doing test discovery, so it seems pointless to require configuring test suffixes and exclusions for lit.  However, it's not a use case I directly have experience with, so I might be misunderstanding.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153967



More information about the llvm-commits mailing list