[lit] Move llvm-test-suite result codes into llvm/lit

[jdoerfert]

Summary

Without this lnt runtest test-suite instructed to use lit will fail
if tests result in the SKIPPED or NOEXE result codes. A patch to the
llvm-test-suite will remove the definitions there (from
litsupport/test.py) and reuse these ones instead.

[cmatthews]

Lgtm. Thanks!

[jdenny]

llvm/utils/lit/lit/Test.py

39

This has been added already: D77819.

[yln]

Hi Johannes,

As Joel mentioned: I already added a SKIPPED category: tests that should have been executed but weren't.
I am planning to add one more: FILTERED: tests that are discovered but, are filtered out.

Please explain what benefit having a separate category for NOEXE offers; and why it is considered a failure? My understanding is that --no-execute is mostly used for benchmarking lit itself (pass/fail doesn't matter there?).

Thanks!

[jdoerfert]

In D77986#1978333, @yln wrote:

Hi Johannes,

As Joel mentioned: I already added a SKIPPED category: tests that should have been executed but weren't.
I am planning to add one more: FILTERED: tests that are discovered but, are filtered out.

Please explain what benefit having a separate category for NOEXE offers; and why it is considered a failure? My understanding is that --no-execute is mostly used for benchmarking lit itself (pass/fail doesn't matter there?).

Thanks!

As I mentioned in the commit message, without this patch running lnt runtest test-suite can fail with an obscure message and not produce a result. It is considered a failure because this is the definition that I moved from the test-suite (D77987).

[yln]

Ah okay, I didn't realize that NOEXE had nothing to do with lit's --no-execute option. I think you can use UNRESOLVED instead of creating a specific category for it and use the already existing SKIPPED. Would that work?

[jdoerfert]

In D77986#1979080, @yln wrote:

Ah okay, I didn't realize that NOEXE had nothing to do with lit's --no-execute option. I think you can use UNRESOLVED instead of creating a specific category for it and use the already existing SKIPPED. Would that work?

I guess one could use UNRESOLVED but I'm unsure why and I think there are obvious drawbacks: It would introduce a weird split between the llvm-test-suite and llvm/lit as NOEXE in the former would then map to something else in the latter. I don't think we should "rename" it in the test-suite as it is not "unresolved" (whatever that then means) but we just failed to produce an executable so the compile stage failed not the run stage (which would be FAIL).

Using the SKIPPED category has similar problems (conceptually, I don't know if it would actually show right now): We didn't skip the test, we compiled, failed to produce an executable, and consequently couldn't run anything. Skipped implies something else.

[jdenny]

I don't think we should "rename" it in the test-suite as it is not "unresolved" (whatever that then means) but we just failed to produce an executable so the compile stage failed not the run stage (which would be FAIL).

Based on that description, I agree with @jdoerfert that UNRESOLVED or SKIPPED doesn't make sense for NOEXE. NOEXE sounds like a special kind of FAIL. I don't have enough experience with lnt to know why it's worthwhile to distinguish NOEXE from FAIL, but it's not a new distinction in lnt. If it's worthwhile there, maybe it will be worthwhile in other lit-based test suites too.

[yln]

In D77986#1980779, @jdenny wrote:

I don't think we should "rename" it in the test-suite as it is not "unresolved" (whatever that then means) but we just failed to produce an executable so the compile stage failed not the run stage (which would be FAIL).

Based on that description, I agree with @jdoerfert that UNRESOLVED or SKIPPED doesn't make sense for NOEXE. NOEXE sounds like a special kind of FAIL.

I mixed this up as well. Thanks for explaining @jdoerfert!

I don't have enough experience with lnt to know why it's worthwhile to distinguish NOEXE from FAIL, but it's not a new distinction in lnt. If it's worthwhile there, maybe it will be worthwhile in other lit-based test suites too.

This is the question then. Are there plans (or a desire) to adopt this notion in other lit tests as well and should this become a supported lit feature? Is not having these categories in "vanilla" lit blocking improvements in lnt?

[jdoerfert]

In D77986#1981385, @yln wrote:

In D77986#1980779, @jdenny wrote:

I don't have enough experience with lnt to know why it's worthwhile to distinguish NOEXE from FAIL, but it's not a new distinction in lnt. If it's worthwhile there, maybe it will be worthwhile in other lit-based test suites too.

This is the question then. Are there plans (or a desire) to adopt this notion in other lit tests as well and should this become a supported lit feature? Is not having these categories in "vanilla" lit blocking improvements in lnt?

To make this clear: Right now this is needed for me to run lnt with lit in the first place. Without these two patches lnt crashes as it provides lit with NOEXE result codes that are not handled in tests_by_code (and potentially other places).

[yln]

In D77986#1981550, @jdoerfert wrote:

To make this clear: Right now this is needed for me to run lnt with lit in the first place. Without these two patches lnt crashes as it provides lit with NOEXE result codes that are not handled in tests_by_code (and potentially other places).

Ah, I now understand the reason and urgency! I didn't realize that there were "custom" categories in derived projects. Apologies for breaking things. Let me get back to you with a patch that accounts for this.

[yln]

Would the solution proposed in D78164 work for you? @jdoerfert

[yln]

Accidental accept. I mean to resign as a reviewer.

[jdoerfert]

@Meinersbur will take a look at this using the registration mechanism @ynl provided.

llvm/utils/lit/lit/Test.py

 PASS        = ResultCode('PASS', False)
 FLAKYPASS   = ResultCode('FLAKYPASS', False)
 XFAIL       = ResultCode('XFAIL', False)
 FAIL        = ResultCode('FAIL', True)
 XPASS       = ResultCode('XPASS', True)
 UNRESOLVED  = ResultCode('UNRESOLVED', True)
 UNSUPPORTED = ResultCode('UNSUPPORTED', False)
 TIMEOUT     = ResultCode('TIMEOUT', True)
+SKIPPED     = ResultCode('SKIPPED', False)
+NOEXE       = ResultCode('NOEXE', True)
+
 
 # Test metric values.
 
 class MetricValue(object):
     def format(self):
         """
         format() -> str
 

llvm/utils/lit/lit/main.py

def print_histogram(tests):
     lit.util.printHistogram(test_times, title='Tests')
 
 
 # Status code, summary label, group label
 failure_codes = [
     (lit.Test.UNRESOLVED,  'Unresolved Tests',    'Unresolved'),
     (lit.Test.TIMEOUT,     'Individual Timeouts', 'Timed Out'),
     (lit.Test.FAIL,        'Unexpected Failures', 'Failing'),
-    (lit.Test.XPASS,       'Unexpected Passes',   'Unexpected Passing')
+    (lit.Test.XPASS,       'Unexpected Passes',   'Unexpected Passing'),
+    (lit.Test.NOEXE,       'No Executable',       'No Executable'),
 ]
 
 all_codes = [
     (lit.Test.UNSUPPORTED, 'Unsupported Tests', 'Unsupported'),
     (lit.Test.PASS,        'Expected Passes',   ''),
     (lit.Test.FLAKYPASS,   'Passes With Retry', ''),
     (lit.Test.XFAIL,       'Expected Failures', 'Expected Failing'),
+    (lit.Test.SKIPPED,     'Skipped',           'Skipped'),
 ] + failure_codes
 
 
 def print_results(tests, elapsed, opts):
     tests_by_code = {code: [] for (code, _, _) in all_codes}
     for test in tests:
         tests_by_code[test.result.code].append(test)