[lit] Allow for tests to have non-parsed requirements

[CaseyCarter]

Summary

MSVC uses lit for STL testing, running both the libcxx tests and our "native" suite of tests which has feature requirements that are not parsed from the test content, but supplied externally. dac21fd2 broke us by ignoring the initial value of the requires property of tests and using only the result of the REQUIRES: parser.

(This is my first LIT change, apologies if I'm missing some protocol.)

[ldionne]

How do you set test.requires externally? Do you have a custom test format?

This LGTM generally speaking, with some details. Also, can we add some tests? The tests are in llvm/utils/lit/tests.

llvm/utils/lit/lit/TestRunner.py

1466

I think we should have consistent behaviour for test.xfails and test.unsupported`.

1467

I think this can always be just test.requires because test.requires is set to [] on construction. Thus you'd end up adding both the requires set programatically on the test and the parsed ones.

[cbezault]

Yes, we have a custom test format which considers the configured target architecture and the set of flags the test will be compiled with.

[ldionne]

In D81782#2093185, @cbezault wrote:

Yes, we have a custom test format which considers the configured target architecture and the set of flags the test will be compiled with.

Is that test format available on GitHub or somewhere else? I'd love to take a look.

[cbezault]

It can be found here. It was based loosely off of the old libcxx test format. I'm probably going to submit a pretty significant re-working of it and/or see if I can adapt the new format to our needs whenever I have a free minute.

[ldionne]

In D81782#2093428, @cbezault wrote:

It can be found here. It was based loosely off of the old libcxx test format. I'm probably going to submit a pretty significant re-working of it and/or see if I can adapt the new format to our needs whenever I have a free minute.

Nice! I think you should look at https://reviews.llvm.org/D81846 then -- it would allow specifying your own Lit config entirely from libc++'s CMake configuration. This way, you could point Lit to your own configuration made entirely from scratch. I also have examples of how to setup configurations from scratch that I'm going to submit patches for soon.

[CaseyCarter]

Partially address review comments.

[CaseyCarter]

Hack together test.

[CaseyCarter]

I attempted to address everything. The test may be terrible - I've used LIT casually but never peeked inside until yesterday - so please don't refrain from telling me how to improve it.

[ldionne]

llvm/utils/lit/tests/unparsed-requirements.py

1 ↗

(On Diff #270845)

Is 2> %t.err necessary?

8 ↗

(On Diff #270845)

I think this LGTM, but you should also test xfails and unsupported.

15 ↗

(On Diff #270845)

You could also avoid creating a new class CustomTest and instead use:

test = Test(<args...>)
test.requires = ["meow"]

This might be less brittle in case we decide to change the signature of lit.Test's __init__ method.

[CaseyCarter]

• Remove copy-pasta from RUN line in unparsed-requirements.py.
• Simplify code with +=
• Complete test coverage
• Change line endings to LF in new files

[CaseyCarter]

llvm/utils/lit/tests/unparsed-requirements.py

1 ↗

(On Diff #270845)

Nope - it's copy-pasta.

8 ↗

(On Diff #270845)

I was so shocked to get the test working that I forgot it wasn't complete!

[ldionne]

LGTM.

[CaseyCarter]

Thanks for the help, Louis!

llvm/utils/lit/lit/TestRunner.py

def parseIntegratedTestScript(test, additional_parsers=[],
     """
     # Parse the test sources and extract test properties
     try:
         parsed = _parseKeywords(test.getSourcePath(), additional_parsers,
                                 require_script)
     except ValueError as e:
         return lit.Test.Result(Test.UNRESOLVED, str(e))
     script = parsed['RUN:'] or []
     test.xfails = parsed['XFAIL:'] or []
-    test.requires = parsed['REQUIRES:'] or []
+    test.requires = (test.requires or []) + (parsed['REQUIRES:'] or [])
     test.unsupported = parsed['UNSUPPORTED:'] or []
     if parsed['ALLOW_RETRIES:']:
         test.allowed_retries = parsed['ALLOW_RETRIES:'][0]
 
     # Enforce REQUIRES:
     missing_required_features = test.getMissingRequiredFeatures()
     if missing_required_features:
         msg = ', '.join(missing_required_features)