I don't really understand the purpose of the test, but if the purpose of it is to check whether something appears on stdout, then a pexpect test does seem like the right tool for the job.
I have tried entering the commands from the test manually, and the required text does *not* appear when using the lldb built on the buildbot, while it *does* appear when using the lldb I build locally. So, this appears to be a genuine bug caused by something in the buildbot environment, but it's not clear to me now what it could be. That said, there is currently a big problem with the way this test is written, and that's the fact that the prompt expectation does not play well with the lldb's colored prompt feature. LLDB colors the prompt by overwriting the text which was already written by editline, resulting in something like "(lldb) \33[2m(lldb) \33[22m" written to stdout. This throws pexpect completely off scent, as the prompt substring appears twice, causing the commands and their expectations to misalign. Right now it does not matter, because as soon as you start looking for the pattern "this is a test string", the streams will align up, but it can cause you a lot of problems if you execute a command and then try to observe some external side effects of it, because pexpect will return before the command has even been given a chance to complete. BTW: the skipIfRemote decorator is not needed. It was needed on TestBatchMode, because that one actually runs executables, which needs some plumbing to work remotely, but since this test just plays with python commands it will execute just fine in the remote mode. pl On 14 January 2016 at 19:06, Jim Ingham via lldb-commits <lldb-commits@lists.llvm.org> wrote: > Well, actually on Unix MOST things are files, it was Plan 9 in which that all > things are files, IIRC... > > Jim > >> On Jan 14, 2016, at 11:04 AM, Jim Ingham <jing...@apple.com> wrote: >> >> Yes, on unix all things are files and all files work the same except when >> they don't. I'd rather test the thing we ACTUALLY care about, rather than >> testing something else and assuming that it is going to work in the case we >> care about as well. >> >> Jim >> >>> On Jan 14, 2016, at 10:58 AM, Zachary Turner <ztur...@google.com> wrote: >>> >>> Wouldn't testing with output redirecxted to a file still test that it is >>> being streamed as it is obtained rather than a big dump at the end? I mean >>> that's what stdout is right? Just a file. If you use a file on the >>> filesystem instead, just check the contents of the file after each >>> operation. >>> >>> On Thu, Jan 14, 2016 at 10:42 AM Jim Ingham <jing...@apple.com> wrote: >>> I worry giving up on testing using Python's stdout for the immediate output >>> stream. This is a very useful feature, allowing users to make Python based >>> commands that produce a bunch of output, and stream the output as it is >>> being gathered rather than having the command stall and then dump a bunch >>> of text at the end. This isn't speculative, that's how many of the >>> commands that the OS X kernel team ships for inspecting kernel state work. >>> >>> So we really should be testing this feature. Maybe the flakiness on Linux >>> is just a pexpect bug, and streaming to stdout would always work correctly >>> hooked up to a "real" terminal. But until we know that we should presume >>> that it is something in LLDB->Python or in the way we use Python, and keep >>> testing it. >>> >>> If you want to separate the two issues, then it would be fine to write >>> another test that just tests the type maps for FILE *, but I still think >>> this one is valuable. >>> >>> Jim >>> >>>> On Jan 14, 2016, at 10:16 AM, Zachary Turner via lldb-commits >>>> <lldb-commits@lists.llvm.org> wrote: >>>> >>>> How much time do you think it would take to determine whether or not using >>>> the file-based approach would work? Because on the surface it sounds >>>> fairly straightforward, and fixing it that way would allow us to not have >>>> to xfail this on more platforms for reasons that we don't understand. >>>> >>>> On Thu, Jan 14, 2016 at 10:15 AM Enrico Granata via lldb-commits >>>> <lldb-commits@lists.llvm.org> wrote: >>>> The log just shows a timeout is happening in pexpect() - which I don’t >>>> have a ready explanation for >>>> >>>> X-failing for now is the proper recourse. But you might want to debug this >>>> at some point, since it’s weird behavior. It looks like the command is not >>>> even just silently erroring out - from the log you sent it looked stuck >>>> somewhere.. >>>> >>>> Is there any chance you can step through and see where the hang is >>>> happening? >>>> >>>>> On Jan 14, 2016, at 3:03 AM, Tamas Berghammer <tbergham...@google.com> >>>>> wrote: >>>>> >>>>> I XFAIL-ed the test for Linux to get the build bot green again and filed >>>>> a bug at https://llvm.org/bugs/show_bug.cgi?id=26139 >>>>> >>>>> On Thu, Jan 14, 2016 at 2:18 AM Ying Chen via lldb-commits >>>>> <lldb-commits@lists.llvm.org> wrote: >>>>> Please see attached log file. >>>>> >>>>> Thanks, >>>>> Ying >>>>> >>>>> On Wed, Jan 13, 2016 at 5:39 PM, Enrico Granata <egran...@apple.com> >>>>> wrote: >>>>> From the buildbot log it’s quite hard to tell what could be going on >>>>> >>>>> Is there any chance you guys could run the test by hand with the “-t -v” >>>>> flags to the dotest.py driver and attach the output of the run? >>>>> >>>>> That might help figure out where the issue lies >>>>> >>>>>> On Jan 13, 2016, at 5:35 PM, Ying Chen <chy...@google.com> wrote: >>>>>> >>>>>> Hello Enrico, >>>>>> >>>>>> The new test has been failing on Ubuntu buildbot. But it's passing on >>>>>> some offline Ubuntu machines, I don't understand what caused the >>>>>> difference. >>>>>> Could you please help to take a look? >>>>>> >>>>>> http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/10299 >>>>>> >>>>>> Thanks, >>>>>> Ying >>>>>> >>>>>> On Wed, Jan 13, 2016 at 11:32 AM, Enrico Granata via lldb-commits >>>>>> <lldb-commits@lists.llvm.org> wrote: >>>>>> >>>>>>> On Jan 13, 2016, at 11:26 AM, Zachary Turner <ztur...@google.com> wrote: >>>>>>> >>>>>>> Thanks! btw would having the command write its output to a file >>>>>>> instead of stdout solve the pexpect problme as well? >>>>>>> >>>>>> >>>>>> That’s possible - but I would need to play with it a little bit to >>>>>> convince myself that it really is a faithful reproduction of my original >>>>>> issue >>>>>> It’ll take me a little while to get to it - stay tuned. >>>>>> >>>>>>> On Wed, Jan 13, 2016 at 11:22 AM Enrico Granata <egran...@apple.com> >>>>>>> wrote: >>>>>>> >>>>>>>> On Jan 13, 2016, at 10:34 AM, Zachary Turner <ztur...@google.com> >>>>>>>> wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Wed, Jan 13, 2016 at 10:25 AM Enrico Granata <egran...@apple.com> >>>>>>>> wrote: >>>>>>>>> On Jan 13, 2016, at 10:21 AM, Zachary Turner <ztur...@google.com> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> On Wed, Jan 13, 2016 at 10:15 AM Enrico Granata via lldb-commits >>>>>>>>> <lldb-commits@lists.llvm.org> wrote: >>>>>>>>> + >>>>>>>>> +class CommandScriptImmediateOutputTestCase (PExpectTest): >>>>>>>>> Does the bug that you were trying to fix occur only when using the >>>>>>>>> command_script.py file from the lldb command line? If you load it >>>>>>>>> from within lldb via an LLDB command, does the problem still occur? >>>>>>>>> If the problem you are fixing is not specific to the LLDB command >>>>>>>>> line, I would prefer if you write this not as a pexpect test. >>>>>>>> >>>>>>>> I would love to not touch pexpect :-) But in this case, I can’t see a >>>>>>>> way around it. I am trying to detect whether some text is “physically” >>>>>>>> printed to stdout. And pexpect seems the most obvious straightforward >>>>>>>> way to get that to happen. Note that, in this bug, the result object >>>>>>>> is filled in correctly even if nothing gets printed, so looking at the >>>>>>>> result won’t quite cut it - it really needs to detect output to stdout. >>>>>>>> You're calling result.SetImmediateOutputFile(sys.__stdout__). >>>>>>>> Wouldn't it work to use a file on the file system here, and then you >>>>>>>> open that file and look at it after running the commands? It wouldn't >>>>>>>> work in remote cases, but it already doesn't work on remote cases >>>>>>>> anyway (as you point out below) >>>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>> + >>>>>>>>> + mydir = TestBase.compute_mydir(__file__) >>>>>>>>> + >>>>>>>>> + def setUp(self): >>>>>>>>> + # Call super's setUp(). >>>>>>>>> + PExpectTest.setUp(self) >>>>>>>>> + >>>>>>>>> + @skipIfRemote # test not remote-ready llvm.org/pr24813 >>>>>>>>> + @expectedFlakeyFreeBSD("llvm.org/pr25172 fails rarely on the >>>>>>>>> buildbot") >>>>>>>>> + @expectedFlakeyLinux("llvm.org/pr25172") >>>>>>>>> Are these necessary? The windows one is necessary (but not if you >>>>>>>>> change this to not being a pexpect test as I've requested above), but >>>>>>>>> why are the other ones necessary? >>>>>>>> >>>>>>>> Do we support remote pexpect? As for FreeBSD and Linux, they might not >>>>>>>> be necessary, but I’d rather much remove them (or let the relevant >>>>>>>> platform owners) remove them in a separate commit >>>>>>>> >>>>>>>> No remote pexpect, so the @skipIfRemote probably needs to be there. >>>>>>>> But I think everyone should be checking in tests enabled by default in >>>>>>>> the widest set of environments possible that you aren't completely >>>>>>>> sure are broken. It should be up to the platform holders to disable >>>>>>>> broken tests, not to enable working tests. Because it's much easier >>>>>>>> to notice a broken test going in than it is to notice a working test >>>>>>>> went in disabled (because who's going to think to test it out?). >>>>>>> >>>>>>> This is a fair point. I’ll enable those platforms in a subsequent >>>>>>> commit ASAP >>>>>>> >>>>>>> >>>>>>> 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 >>>>>> >>>>>> >>>>> >>>>> >>>>> Thanks, >>>>> - Enrico >>>>> 📩 egranata@.com ☎️ 27683 >>>>> >>>>> >>>>> _______________________________________________ >>>>> lldb-commits mailing list >>>>> lldb-commits@lists.llvm.org >>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits >>>> >>>> >>>> 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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
