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

[Zachary Turner via lldb-commits]

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/730bc72a/attachment.html>