[Lldb-commits] [PATCH] D67018: [lldb][NFC] Add basic test for GUI command

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

In D67018#1653138 <https://reviews.llvm.org/D67018#1653138>, @teemperor wrote:

> Given how much fun Pavel had fixing my previous pexpect tests, I thought: "Why stop the fun after just two tests?". So here we go!


It looks like Jonas had a bit of fun too. ;)

Thank you for writing this, I think it's important to start experimenting with this. This test alone will increase our coverage of the gui command by a factor of infinity. :D

I do see some problems ahead. Like, this method is sufficient to test that some text is displayed on the screen, but it would be pretty hard to use it to check that the text is graphically laid out if a way that makes sense. Theoretically, we might assert entire ansi escape sequences, but that would be: a) clunky; b) susceptible to flakyness due to different curses versions choosing different (but equivalent) escape sequences. For that, we might need some sort of "terminal emulator" to process the escape sequences and produce the final window contents. Then, we could assert that, similarly to how some gui test frameworks talk to the X server to to get the actual window contents, click buttons, etc.

But that's just thinking about future. This looks fine to me right now.

As for the terminal size, the reason the other test sets the screen size so wide, is so that it avoids wrapping of file names, as that is what the test is asserting. That isn't the case here, though it's true that the test could be susceptible to window-size-related problems. If we don't specify the dimensions argument then pexpect is supposed to choose some sensible default itself. If we find that the default pexpect chooses is not suitable for us (perhaps because it picks the size of the parent terminal), then we can hardcode some reasonable default of our own into the launch method. However, I also don't think that's something we have to do now, as we don't have enough experience to know what that default is.

Anyway, LGMT, with one question: Given the recent test suite restructure, shouldn't this go somewhere under commands/gui or something?



================
Comment at: lldb/packages/Python/lldbsuite/test/functionalities/gui_command/main.c:2
+int main(int argc, char **argv) {
+  lldb_enable_attach();
+  int funky_var_name_that_should_be_rendered = 22;
----------------
You only need this if you actually intend to attach to the program (which, it seems, you don't).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67018/new/

https://reviews.llvm.org/D67018