[PATCH] D78164: [lit] Provide extension API for custom result categories

[Abhinav Gaba via Phabricator via llvm-commits]

abhinavgaba added inline comments.


================
Comment at: llvm/utils/lit/tests/Inputs/custom-result-category/lit.cfg:8
+config.suffixes = ['.txt']
+config.test_format = format.MyFormat()
+config.test_source_root = None
----------------
yln wrote:
> abhinavgaba wrote:
> > abhinavgaba wrote:
> > > yln wrote:
> > > > abhinavgaba wrote:
> > > > > 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?
> > > > 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?
> > > 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
> > > ```
> > I tried Python 2.7.12 and 3.7.4. It passes with 3.7.4 but hangs with 2.7.12.
> 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.
> 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78164