This is an archive of the discontinued LLVM Phabricator instance.

[lit] Provide extension API for custom result categories
ClosedPublic

Authored by yln on Apr 14 2020, 3:56 PM.

Details

Summary

The lnt test suite defines custom result codes [1]. Support those via
an extension API instead of "by accident", which should offer the
advantage of properly handling them when we print test results.

[1] https://reviews.llvm.org/D77986

Diff Detail

Event Timeline

yln created this revision.Apr 14 2020, 3:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 14 2020, 3:56 PM
yln edited the summary of this revision. (Show Details)Apr 14 2020, 4:11 PM

Thanks, I'll check if I can run add_result_category from the test suite to resolve my crash :)

yln added a comment.EditedApr 17 2020, 2:37 PM

Did you have an opportunity to try this? @jdoerfert

AFAIU, test-suite would need to call add_result_category for its categories to resolve this? Is there anything more blocking this than being tried-out using test-suite?

Note that this occurs problem occurs in our buildbots such as: http://lab.llvm.org:8011/builders/clang-native-arm-lnt-perf

yln added a comment.Apr 27 2020, 2:41 PM

AFAIU, test-suite would need to call add_result_category for its categories to resolve this? Is there anything more blocking this than being tried-out using test-suite?

No, I am only waiting for confirmation that this patch will work for test-suite. Friendly ping to @jdoerfert

Confirming this together with D79064 works with the test suite.

yln added a comment.Apr 28 2020, 10:09 PM

Can I get an official "Accept" so I can land this without a "Landed without approval" warning (and without accepting my own change). Thanks!

Meinersbur accepted this revision.Apr 29 2020, 7:05 PM
This revision is now accepted and ready to land.Apr 29 2020, 7:05 PM
This revision was automatically updated to reflect the committed changes.
abhinavgaba added inline comments.
llvm/utils/lit/tests/Inputs/custom-result-category/lit.cfg
8

Using MyFormat() here is causing check-lit to hang indefinitely on WIndows x86-64 (Python 2.7.6).

Changing this to use config.test_format = lit.format/ShTest() "fixes" the hang (although then the test fails, which is expected).

Is that a known issue?

yln marked an inline comment as done.May 4 2020, 1:15 PM
yln added inline comments.
llvm/utils/lit/tests/Inputs/custom-result-category/lit.cfg
8

I disabled the test on Windows in 3610fd8c5c6039c096d61c376b37a724a81c4d35. Would you be able to help me debug this? (I don't have access to a Windows machine.)

My best guess is that this has something to do with loading the class from the SITE_DIR:

import site
site.addsitedir(os.path.dirname(__file__))

https://github.com/llvm/llvm-project/blob/6cecd3c3dbef48eca6c4cf2dcc2df3290ab91488/llvm/utils/lit/tests/lit.cfg#L46
https://docs.python.org/3.9/library/site.html#site.USER_SITE

Can you also try Python3?

abhinavgaba added inline comments.May 4 2020, 2:17 PM
llvm/utils/lit/tests/Inputs/custom-result-category/lit.cfg
8

Thanks. It does seem like a configuration issue. I'll try with python 3 (and a newer version of 2.7).

Another data point: with everything as is, this change fixes the hang as well:

diff --git a/llvm/utils/lit/tests/Inputs/custom-result-category/format.py b/llvm/utils/lit/tests/Inputs/custom-result-category/format.py
index b0c97ec7..a3258d3 100644
--- a/llvm/utils/lit/tests/Inputs/custom-result-category/format.py
+++ b/llvm/utils/lit/tests/Inputs/custom-result-category/format.py
@@ -8,11 +8,4 @@ lit.main.add_result_category(CUSTOM_PASS, "My Passed")
 lit.main.add_result_category(CUSTOM_FAILURE, "My Failed")


-class MyFormat(lit.formats.ShTest):
-    def execute(self, test, lit_config):
-        result = super(MyFormat, self).execute(test, lit_config)
-        if result.code.isFailure:
-            result.code = CUSTOM_FAILURE
-        else:
-            result.code = CUSTOM_PASS
-        return result
+MyFormat = lit.formats.ShTest
abhinavgaba added inline comments.May 5 2020, 6:11 PM
llvm/utils/lit/tests/Inputs/custom-result-category/lit.cfg
8

I tried Python 2.7.12 and 3.7.4. It passes with 3.7.4 but hangs with 2.7.12.

yln marked an inline comment as done.May 5 2020, 8:08 PM
yln added inline comments.
llvm/utils/lit/tests/Inputs/custom-result-category/lit.cfg
8

The "fix" is interesting. This means that's it is not the error I suspected: loading the separate Python file via site.addsitedir() generally works. But then defining a class in it doesn't?! Strange.

I don't really have an idea how to narrow this down further. Considering that this is pretty obscure (maybe even a Python bug?) and seems to work in Python 3, I propose to keep this test disabled on Windows until we require Python3 to run lit itself.

abhinavgaba added inline comments.May 6 2020, 12:06 PM
llvm/utils/lit/tests/Inputs/custom-result-category/lit.cfg
8

Considering that this is pretty obscure (maybe even a Python bug?) and seems to work in Python 3, I propose to keep this test disabled on Windows until we require Python3 to run lit itself.

That sounds reasonable to me. Thanks.