[PATCH] D77819: [lit] Add SKIPPED test result category

[Joel E. Denny via Phabricator via llvm-commits]

jdenny added a comment.

In D77819#1975150 <https://reviews.llvm.org/D77819#1975150>, @yln wrote:

> In D77819#1974995 <https://reviews.llvm.org/D77819#1974995>, @jdenny wrote:
>
> > It think it would be messy: run the lit test suite in the background redirecting to a file, record the PID, poll that file in the foreground until expected passes appear, and then kill -SIGINT.  Very slow tests should be skipped.  It works from my bash shell, but I'm not sure about portability.
> >
> > For now, your `-max-failures` tests are a race-free, portable way to exercise at least one code path for skipped tests, so I would not object if you felt that was sufficient.
>
>
> Yes, I would like to spend my time on other improvements ;)


So, if your `-max-time` test proves race-free in practice, my recipe above can be simplified a bit: just run lit in the background and record the PID, sleep long enough for expected passes, and then `kill -SIGINT`.  Well, maybe one day.  :-)



================
Comment at: llvm/utils/lit/tests/max-time.py:3
+#
+# RUN: %{lit} %{inputs}/max-time --max-time=1 2>&1  |  FileCheck %s
+
----------------
yln wrote:
> jdenny wrote:
> > yln wrote:
> > > jdenny wrote:
> > > > On heavily loaded test systems, is there a chance of a race here?
> > > Yes, race is between `--max-time=1` and `sleep 5` in [slow.txt].  I will increase 5 to 60 when landing.  That should work for all practical purposes.
> > Isn't there theoretically a race between `--max-time=1` and fast.txt as well?  I'm not saying you need to change it, but I at least want to be sure I'm not misunderstanding something.
> Yes, you are right! I forgot to consider this.  I would like to keep this at 1 for now (because it increases the actual running time of the test) and only increase it if we discover that it is a problem in practice.
I think that's fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77819