This is an archive of the discontinued LLVM Phabricator instance.

Move TestCommandScriptImmediateOutput from PExpectTest to TestBase
AbandonedPublic

Authored by fjricci on Apr 27 2016, 3:14 PM.

Details

Summary

This should make TestCommandScriptImmediateOutput more consistent
with the rest of the test suite.

Diff Detail

Event Timeline

fjricci updated this revision to Diff 55337.Apr 27 2016, 3:14 PM
fjricci retitled this revision from to Move TestCommandScriptImmediateOutput from PExpectTest to TestBase.
fjricci updated this object.
fjricci added reviewers: granata.enrico, zturner.
fjricci added subscribers: sas, lldb-commits.
zturner edited edge metadata.Apr 27 2016, 3:17 PM
zturner added a subscriber: zturner.

You can probably remove the expected failure windows if this is no longer
going to be a pexpect test.

fjricci updated this revision to Diff 55340.Apr 27 2016, 3:22 PM
fjricci edited edge metadata.

Remove windows expected failure

Why are we trying to do this in the first place?

The entire purpose of this test is to verify the immediate output to the console.

If you want to test the output to a file instead, make a separate test case, that would be fine. But I'd rather much leave this test alone.

As of r264351, this test already writes to both the console and to files (since the file writing is testing the same python file io bugs tested by the console writing). I can make another patch to split it out if that's preferable.

However, this patch is intended to move the test off of PExpect, because I noticed that sendline() has some issues when running with older versions of lldb (like 3.8, for example), regardless of whether we're testing files or the console.

My personal preference would be to split up the test cases. I'd really like to keep testing the immediate output to console scenario since it is interesting for - e.g. - kernel debugging at Apple.

fjricci abandoned this revision.Apr 28 2016, 8:11 AM

Ok, I'll put up a new revision to split out the test cases.

Would it be acceptable to split it into two test methods inside the same test case? Both are testing writing immediate output from a command script, just one is immediate output to the console and the other is immediate output to a file.

Sure, that works. My only concern is that I'd like to keep the current PExpect-let's test writing to the console and see what happens behavior

Whatever you need to do to split the test is fine by me

Oh I see. I don't think the file-writing is going to work in the PExpect mode (for some reason, it looks like the sendline calls aren't always being received/processed correctly by lldb, while runCmd works fine), so it's probably better to split out into a different test case then.

I don't think we are seeing this behavior on OS X (the failure to process sendline() calls) - but Todd Fiala might know more about that since he keeps a careful eye over our bots.

Longer term, it would be interesting to find out *why* those issues are happening. For now, a test case split sounds reasonable.

I do observe the sendline() behavior on OSX, but only when I cherry-pick these bug-fixes onto the 3.8 branch. It seems to work fine on master.

fjricci added a subscriber: tfiala.Apr 28 2016, 9:37 AM