[Lldb-commits] [lldb] r257644 - Fix an issue where scripted commands would not actually print any of their output if an immediate output file was set in the result object via a Python file object

[Zachary Turner via lldb-commits]

On Wed, Jan 13, 2016 at 10:25 AM Enrico Granata <egranata at apple.com> wrote:

> On Jan 13, 2016, at 10:21 AM, Zachary Turner <zturner at google.com> wrote:
>
>
> On Wed, Jan 13, 2016 at 10:15 AM Enrico Granata via lldb-commits <
> lldb-commits at 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?).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20160113/6612604c/attachment.html>