[Lldb-commits] [lldb] r251862 - Revert "Remove the __import__ hack of lldbtest_config."

[Zachary Turner via lldb-commits]

+Enrico Granata <egranata at apple.com>

On Mon, Nov 2, 2015 at 4:42 PM Zachary Turner <zturner at google.com> wrote:

> Ok, I think I figured out the root of all the problems.
>
> First, in setupSysPath we have this:
>
>     sys.path.insert(0, scriptPath)
>
> This basically adds `lldb/packages/Python/lldbsuite/test` to sys.path.
>
> Then, in the individual tests we write this:
>
> from lldbtest import *
>
> It finds lldbtest at the above path, and imports it.  But this is *not*
> the same copy of lldbtest that has already been imported, because that copy
> was imported as a member of the lldbsuite.test package.  So now we have
> this module in sys.modules twice.  Once as lldbtest, found by looking in
> lldbsuite/test since it was in sys.path.  And once as
> lldbsuite.test.lldbtest, found because lldbsuite was in sys.path, using
> subpackage resolution.
>
> I think the fix for all of these problems is to *remove* scriptPath from
> sys.path.  We shouldn't be mucking with sys.path for stuff in our own test
> suite.  We should resolve the references explicitly with subpackage
> references.  For example, even if I don't write __package__ =
> "lldbsuite.test" in TestMultithreaded.py, I can still fix the problem with
> the below patch:
>
> - from lldbtest import *
> + from lldbsuite.test.lldbtest import *
>
> because now we're referencing the module that has already been imported.
> This is the "correct" way to do it, so if we tighten up sys.path, nobody
> should be able to make this mistake again.
>
> On Mon, Nov 2, 2015 at 4:32 PM Zachary Turner <zturner at google.com> wrote:
>
>> I think what is happening is that when we go through unittest2, the
>> package link is being broken.
>>
>> Inside dotest.py : __package__ = lldbsuite.test
>> Inside unittest2.loadTestsFromName : __package__ = unittest2
>> Inside TestMultithreaded.py : __package__ = None
>>
>> Restoring the link by writing
>>
>> __package__ = "lldbsuite.test"
>>
>> at the top of TestMultithreaded.py seems to fix the problem, although
>> that really bothersome to have to do that in every single file.  I'm still
>> trying to figure out the proper fix.
>>
>> On Mon, Nov 2, 2015 at 3:41 PM Pavel Labath via lldb-commits <
>> lldb-commits at lists.llvm.org> wrote:
>>
>>> Author: labath
>>> Date: Mon Nov  2 17:39:09 2015
>>> New Revision: 251862
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=251862&view=rev
>>> Log:
>>> Revert "Remove the __import__ hack of lldbtest_config."
>>>
>>> The hack still seems to be necessary. Putting it back in until we figure
>>> out why.
>>>
>>> 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=251862&r1=251861&r2=251862&view=diff
>>>
>>> ==============================================================================
>>> --- lldb/trunk/packages/Python/lldbsuite/test/dotest.py (original)
>>> +++ lldb/trunk/packages/Python/lldbsuite/test/dotest.py Mon Nov  2
>>> 17:39:09 2015
>>> @@ -19,11 +19,14 @@ 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
>>> -import lldbsuite
>>>
>>> -import lldbtest_config
>>> +import lldbsuite
>>>
>>>  import atexit
>>>  import commands
>>>
>>>
>>> _______________________________________________
>>> lldb-commits mailing list
>>> lldb-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20151103/f4d57edb/attachment.html>