Sorry for the email spam. I still think the buildbot should run the version in lldb/test though. `test` should be a top-level folder, it makes it easy for people who clone the repo to instantly look and figure out how our test suite works. So I think that's the flow that the buildbot should test. Developer convenience should come second.
On Mon, Nov 2, 2015 at 11:28 AM Zachary Turner <ztur...@google.com> wrote: > If you want to have a way to run the test suite without typing a long > relative path to go into the packages tree, then one thing that might work > is to make a new file packages/Python/dotest.py. > > Then make it a copy of test/dotest.py. Not real crazy about copying code, > but let me know what you think about that. > > On Mon, Nov 2, 2015 at 11:24 AM Zachary Turner <ztur...@google.com> wrote: > >> Don't change your buildbot to not use it. That's an error. >> packages/Python/lldbsuite/test/dotest *must* be imported, as it depends on >> its __init__.py being run. If your buildbot doesn't use it anymore, then I >> think the patch I just submitted (r251819) will break your buildbot, >> because I added this code: >> >> >> if __name__ == "__main__": >> print(__file__ + " is for use as a module only. It should not be run >> as a standalone script.") >> sys.exit(-1) >> >> to packages/Python/lldbsuite/test/dotest/dotest.py >> >> On Mon, Nov 2, 2015 at 11:22 AM Pavel Labath <lab...@google.com> wrote: >> >>> BTW, is lldb/test/dotest.py here to stay? I thought it was there just to >>> avoid breaking anybody who runs dotest directly (instead of ninja >>> check-lldb), and therefore we will remove it once everybody gets a chance >>> to migrate (I have already changed our buildbots to not use it). >>> >>> Is that correct or I am misunderstanding something? >>> >>> pl >>> >>> >>> On 2 November 2015 at 11:11, Zachary Turner via lldb-commits < >>> lldb-commits@lists.llvm.org> wrote: >>> >>>> I looked into this some more, and unfortunately I can't reproduce it. >>>> That being said, I'm not convinced this patch fixes anything. The reason >>>> is that when you import something, whether it be through the `__import__` >>>> function or the `import` statement, the module itself is cached in >>>> `sys.modules`. Whether the *name* for that module is introduced globally >>>> (as is done in this patch) or locally (as is done when you use an `import` >>>> statement) is irrelevant. Because the next time someone else imports it, >>>> it will still find the same instance of the module in `sys.modules` and >>>> just create a new name. It won't import it anew. >>>> >>>> If this patch does actually fix something, I think it must be a >>>> coincidence. That said, I do have an idea as to what the problem might >>>> be. Or at the very least, I know of a problem that would definitely lead >>>> to strange behavior. >>>> >>>> `lldbsuite` is now a package, and it relies on the code in its >>>> `__init__.py` being run. If you run >>>> `packages/python/lldbsuite/test/dotest.py` manually, then `__init__.py` >>>> doesn't get run, and `lldbExec` doesn't get initialized properly. >>>> >>>> Of course, this isn't what you're doing, but it *is* what `dosep` does >>>> internally. `dosep` manually constructs a path to >>>> `packages/python/lldbsuite/test/dotest.py` >>>> and execs it. I have a patch that fixes this and makes `dosep` exec >>>> `lldb/test/dotest.py` instead, which will then lead to the package being >>>> imported, and `__init__.py` being run, and everything being initialized >>>> properly. >>>> >>>> I'm going to commit that patch by itself, and then I will submit a >>>> followup patch that reverts the change from this patch (since I can't >>>> reproduce this problem, I can't check whether or not my patch fixes it). >>>> So if my revert breaks you again, feel free to revert the revert. Although >>>> if there's any way you can investigate a bit to understand what's going on >>>> a little bit more, but I would be very grateful. In particular, I wonder >>>> about the following things: >>>> >>>> 1) When lldbExec is not initialized properly, is this in the same >>>> process instance that you ran from the command line, or is it in the >>>> multiprocessing fork? >>>> 2) If you add code to `lldbsuite/__init__.py` to print the process id >>>> and the value of `lldb_root`, and then add code in `dotest.py` to print the >>>> process id and the value of lldbExec, what does the output look like? >>>> (Each line should be printed up to twice, due to the multiprocessing >>>> fork). >>>> >>>> Anyway, I'll get those 2 patches submitted to fix the dosep issue and >>>> revert this change, and see what happens. >>>> >>>> On Fri, Oct 30, 2015 at 1:53 PM Jim Ingham <jing...@apple.com> wrote: >>>> >>>>> Note, the other important step was that you had to have an lldb >>>>> installed in /usr/bin/lldb that FAILED this test. If you have a more >>>>> recent lldb there, the test will succeed, and you won't notice you aren't >>>>> testing your newly built sources. >>>>> >>>>> Jim >>>>> >>>>> > On Oct 30, 2015, at 1:25 PM, Enrico Granata via lldb-commits < >>>>> lldb-commits@lists.llvm.org> wrote: >>>>> > >>>>> > I think what I was doing is be in lldb/test and do >>>>> > >>>>> > $ ./dotest.py ../packages/python/lldbsuite/functionalities/completion >>>>> > >>>>> >> On Oct 30, 2015, at 12:22 PM, Zachary Turner <ztur...@google.com> >>>>> wrote: >>>>> >> >>>>> >> 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 <egran...@apple.com> >>>>> wrote: >>>>> >>> On Oct 29, 2015, at 11:31 PM, Zachary Turner <ztur...@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 <ztur...@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@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@lists.llvm.org >>>>> >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits >>>>> >> >>>>> >> >>>>> >> Thanks, >>>>> >> - Enrico >>>>> >> 📩 egranata@.com ☎️ 27683 >>>>> >> >>>>> > >>>>> > >>>>> > Thanks, >>>>> > - Enrico >>>>> > 📩 egranata@.com ☎️ 27683 >>>>> > >>>>> > _______________________________________________ >>>>> > lldb-commits mailing list >>>>> > lldb-commits@lists.llvm.org >>>>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits >>>>> >>>>> >>>> _______________________________________________ >>>> lldb-commits mailing list >>>> lldb-commits@lists.llvm.org >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits >>>> >>>> >>>
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
