[Lldb-commits] [lldb] r251678 - Some test cases that need the lldbExec path were failing because lldbExec was turning out to be None even though it was being validly set by dotest.py

[Zachary Turner via lldb-commits]

Can you give me a command line which will reproduce the original problem?
Because I ran through the entire test suite and nothing failed, so I want
to make sure we're doing the same thing.  I'm still a little confused about
how this happens, but I plan to look into it when I'm back on Monday and
see if I can understand it better to identify a better fix.

On Fri, Oct 30, 2015 at 11:58 AM Enrico Granata <egranata at apple.com> wrote:

> On Oct 29, 2015, at 11:31 PM, Zachary Turner <zturner at google.com> wrote:
>
> Wow.  That's a weird problem.  Thanks for finding it!
>
> Would it work if we move the definition of the `lldbtest_config` class
> into lldbsuite/test/__init__.py?  This way the configuration should be part
> of the global package state of the lldbsuite.test package, which all the
> tests are already members of the same package, so they wouldn't even need
> to import anything (I think).
>
>
> I think the problem is exactly that we want lldbtest_config to be *global*
> state and not package local state.
> Honestly, I think if we are not content with the fix as it stands, the
> right way would be to change the way unittest2 imports test cases as to use
> the package-level global scope instead of the global global state as it is
> now.
>
> On Oct 30, 2015, at 8:32 AM, Zachary Turner <zturner at google.com> wrote:
>
> I'm also still a little confused why this worked before my patch.  How is
> unittest2 importing the individual tests in a way that behaves differently
> when dotest is a package (now) versus a standalone script (before)?
>
>
> That is a good question. One to which “because Python” is the only answer
> I can think of. I suspect scripts live at the global scope anyway, so we
> were just getting lucky with those imports making it through correctly.
>
> On Thu, Oct 29, 2015 at 6:12 PM Enrico Granata via lldb-commits <
> lldb-commits at lists.llvm.org> wrote:
>
>> Author: enrico
>> Date: Thu Oct 29 20:09:54 2015
>> New Revision: 251678
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=251678&view=rev
>> Log:
>> Some test cases that need the lldbExec path were failing because lldbExec
>> was turning out to be None even though it was being validly set by dotest.py
>>
>> It turns out that lldbtest_config was being imported locally to
>> "lldbsuite.test" instead of globally, so when the test cases got
>> individually brought by a global import via __import__ by unittest2, they
>> did not see the lldbtest_config import, and ended up importing a new
>> separate copy of it, with lldbExec unset
>>
>> This is a simple hackaround that brings lldbtest_config to global
>> visibility and makes sure the configuration data is correctly shared
>>
>>
>> Modified:
>>     lldb/trunk/packages/Python/lldbsuite/test/dotest.py
>>
>> Modified: lldb/trunk/packages/Python/lldbsuite/test/dotest.py
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/dotest.py?rev=251678&r1=251677&r2=251678&view=diff
>>
>> ==============================================================================
>> --- lldb/trunk/packages/Python/lldbsuite/test/dotest.py (original)
>> +++ lldb/trunk/packages/Python/lldbsuite/test/dotest.py Thu Oct 29
>> 20:09:54 2015
>> @@ -21,6 +21,10 @@ for available options.
>>  """
>>
>>  from __future__ import print_function
>> +# this module needs to have global visibility, otherwise test cases
>> +# will import it anew in their local namespace, essentially losing access
>> +# to all the configuration data
>> +globals()['lldbtest_config'] = __import__('lldbtest_config')
>>
>>  import use_lldb_suite
>>
>> @@ -42,7 +46,6 @@ import test_results
>>  from test_results import EventBuilder
>>  import inspect
>>  import unittest2
>> -import lldbtest_config
>>  import test_categories
>>
>>  import six
>>
>>
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>
>
>
> Thanks,
> *- Enrico*
> 📩 egranata@.com ☎️ 27683
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20151030/5fd7b309/attachment.html>