New interactive test framework based on pexpect

[ridiculousfish]

This adds a new interactive test framework, intended to eventually supplant the expect-based interactive tests.

Recall that "interactive tests" simulate a tty. They're for testing key bindings, signal handling, and other UI stuff. Existing fish interactive tests are based on expect. Despite heroics from @zanchey and others, expect is still hard to work with. TCL is an idiosyncratic language. It uses dollar-sign variables, which collides with fish. The diff-based framework makes authoring and debugging tests tedious.

I propose incrementally migrating expect based tests to a framework based on pexpect, which is a tty emulator written in Python. This allows us to write tests in Python instead of TCL, and also provides better authoring and debugging niceties.

Design

pexpect tests will use the same vocabulary as expect-based tests:

• send() or sendline() to send text to fish.
• expect_str() and expect_re() to match against fish's output. If no matching output is produced before a timeout, this fails the test.
• expect_prompt() optionally matches a RE, then a prompt.

See bind.py for an example (compare to bind.expect).

Failures

A test failure looks like this:

this shows the line number bind.py:188 where the failure occurred.

Note the table of "messages" which is text that was successfully sent or matched:

RECV 3193.86 (bind.py:177): \r\ndefault\r\ninsert\r\npaste\r\nreplace\r\nreplace_one\r\nvisual\r\n
RECV 3198.16 (bind.py:177): \r[I] prompt 19>
SENT 3198.57 (bind.py:181): set -g fish_key_bindings fish_default_key_bindings\n
RECV 3225.20 (bind.py:182): \rprompt 20>
SENT 3225.66 (bind.py:185): echo fish_escape_delay_cms=$fish_escape_delay_ms\n

This shows us the direction, timestamp (ms since start), and the location in the test file which initiated the send or receive.

Dependency Changes

Tests will run under Python 3 only. Python 2 support is not worth the hassle. Note this is a test-time only dependency; we'll continue to support python2 for user-facing scripts.

Beyond that the pexpect module must be installed: pip3 install pexpect. Note that pexpect uses the pty module, which is untested except on Linux - this is I think the biggest risk. In practice it appears to work fine on Macs, and course our expect tests have historically been flaky on non Linux/Mac platforms.

[faho]

This is quite cool!

I love how this uses one file per test. I feel that's the single biggest improvement in littlecheck over our previous system. Switching files got old quick, especially between ".err" and ".out" files.

I also like how this uses python, which we also use elsewhere and which I know and like, over tcl which I don't know and only ever used for these tests.

The dependency seems acceptable, since it seems to be widely available (also the other name), especially via pip. I would love confirmation that it works everywhere (e.g. the BSDs, OpenIndiana/Illumos, cygwin?), but since it's just a testing dependency and we don't currently have CI on any of these I would be okay without.

---

What I don't really love is the output. There is a lot of information here, and it's quite dense, especially considering there's escaped characters in here.

There's a few suggestions I'd make:

• Add some color. This output is all yellow (I have been informed it is apparently "green". You people and your silly colors), which appears to be your normal text color. It should distinguish between out/input and "meta" text like "RECV"

• Ditch the "RECV". That's super jargony. Make it "Output"

• Separate the "Buffer" from the rest. It's not super useful in this case, but I think I can see how it can sometimes be useful. Maybe just add an empty line between these?

Also it's currently a bit weird how you have to read top to bottom and then jump to the top again. I.e. you follow the messages, and then the thing that happens is the TIMEOUT, mentioned before the messages. Maybe add a TIMEOUT message to the "table"? Or maybe not, since that's the important thing that happened? But on the other hand the "unmatched" message is now printed in two locations already.

---

Tests will run under Python 3 only. Python 2 support is not worth the hassle. Note this is a test-time only dependency; we'll continue to support python2 for user-facing scripts.

Supporting py3 only is acceptable. See also #6537.

[faho]

I'm not entirely up on idiomatic python. Should this not be __str__? Then you could remove the explicit call later.

[faho]

So I've done some work on this, and I've found it nicer to remove the formatting responsibility from this class, so you can do relative timestamps.

[faho]

A rough sketch of what I think the output should look like:

I don't want to overdo it, but I feel coloring in the important bits ("this was the failure, this kind of thing happened, this is the interaction") really aids readibility.

[faho]

Also it is entirely too ready to call everything a "TIMEOUT" or is that on purpose?

When I e.g. change a "send" but not the corresponding "except", should it not complain that it's gotten the wrong thing?

[faho]

Okay, if I'm reading this correctly pexpect ignores any non-matching output.

I'm not a fan of that. It means you can just add more output and the test won't fail. In certain situations our prompt counter could save us, tho.

[krobelus]

I love this! it really takes away most of the pain from writing these tests for me, because it's much easier to learn and debug with a bit of python experience.

Okay, if I'm reading this correctly pexpect ignores any non-matching output.

Yep, this makes it easy to ignore extra redraws; for example:

control_a = '\x01'
sp.send("XYZ" + control_a + "echo ")

Here fish has to redraw the line once for each character of echo . I'm not sure how expect behaves here.

[faho]

Yep, this makes it easy to ignore extra redraws; for example:

That's neat, but what about unintentional output?

If you cause another prompt to appear we'll still catch it with the prompt counting, and if you change a line that's bracketed with \r or similar you'll break it, but e.g. change

sp.send("echo mno pqr")

to

sp.send("echo AAAAAAA; echo mno pqr")

or e.g. unintentionally change the output of bind, this won't catch it! The expect tests would.

[krobelus]

I see, so with expect, the very next line has to match? I assume with lines separated by either of \n or \r.
It seems reasonable to keep that behavior, similar to littlecheck (as opposed to LLVM's FileCheck which ignores non-matching output).

[zanchey]

I really can't take any of the blame, er, kudos for the expect-based framework - that was @lilyball's hard work and @faho has done lots to keep it running. I agree that it is time to move to more modern tooling.

pexpect is packaged in just about every platform we care about, although sometimes the versions are quite old.

[faho]

Okay, if I'm reading this correctly pexpect ignores any non-matching output.

So I've looked some more into this, and I have no idea how expect does it.

You can technically use spawn.readline() to read a line or spawn.read(int) to read some characters (with timeout exceptions if you don't get enough and such). But always reading back all input becomes boring quickly, so you'd add reading it back to send. Only... sometimes you don't get all the input back. And that isn't just a rare case, it's what happens for any non-inserted character, including execute, so you'd basically have to adjust every send again, or figure out some way to figure out what fish inserts (and now send any escape sequence).

So: Yeah, it's not all that important. Let's just use it without.

[zanchey]

Expect also just drops unmatched output until the timeout arrives - that's one of its strengths, arguably.

[zanchey]

This is great! I've tested re-enabling the expect/pexpect tests on all the OBS platforms, and they run and pass just fine with the exception of RHEL 6 & 7 where getting Python 3 installed is not straightforward (CentOS 7 is fine).

The expect-based tests were disabled on OBS because of how flaky they were in aa32fc9, but perhaps we can turn them back on once they are migrated to pexpect. There's probably diminishing returns on running them on every known platform, of course.

[zanchey]

Just make this python3-pexpect, and drop the pip lines on everything except macOS - the system packages install faster and are fine (or I can do it after merge).

[ridiculousfish]

Awesome, I'll do it

[ridiculousfish]

Thank you all for awesome review and feedback. @faho's output suggestions are a huge improvement, I will certainly implement those before merging.

[faho]

@faho's output suggestions are a huge improvement, I will certainly implement those before merging.

https://github.com/faho/fish-shell/tree/pexpect-output

[ridiculousfish]

Merged as 8a27873

@zanchey on Travis, I tried your suggestion of switching from pip3 install pexpect to python3-pexpect; see this build. But the .deb was very old (python3-pexpect_4.0.1-1_all.deb) and I wish to use a more recent release. So I merged this with the pip3, but feel free to modify this of course.

@faho I think I incorporated your suggestions; of course feel free to modify it further as you please. You have a great eye for this stuff.