> 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 > <mailto:egran...@apple.com>> wrote: >> On Jan 13, 2016, at 10:21 AM, Zachary Turner <ztur...@google.com >> <mailto:ztur...@google.com>> wrote: >> >> >> On Wed, Jan 13, 2016 at 10:15 AM Enrico Granata via lldb-commits >> <lldb-commits@lists.llvm.org <mailto: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 >> <http://llvm.org/pr24813> >> + @expectedFlakeyFreeBSD("llvm.org/pr25172 <http://llvm.org/pr25172> >> fails rarely on the buildbot") >> + @expectedFlakeyLinux("llvm.org/pr25172 <http://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
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
