[lldb][test] Prevent \n in calls to lldb's expect() test helper.

[rupprecht]

Summary

expect() forwards its command to sendline(). This can be problematic if the command already contains a newline: sendline() unconditionally adds a newline to the command, which causes the command to run twice (hitting enter in lldb runs the previous command). The expect() helper looks for the prompt and finds the first one, but because the command has run a second time, the buffer will contain the contents of the second time the command ran, causing potential erroneous matching.

Simplify the editline test, which was using different commands to workaround this misunderstanding.

[merge_guards_bot]

Build result: pass - 60084 tests passed, 0 failed and 729 were skipped.
Log files: console-log.txt, CMakeCache.txt

[labath]

Thanks for getting to the bottom of this. Adding the \n check is fine. The "expect" function was meant to be an pexpect equivalent of the "expect" function in regular tests (which also just handles a single command). If anyone is doing something fancier than that, he should probably interact with pexpect directly.

lldb/packages/Python/lldbsuite/test/terminal/TestEditline.py

26–33

Maybe update the example to use the command "print" too?

[JDevlieghere]

Hey Jordan, the test is failing on the sanitized bot on GreenDragon. The error is Could not terminate the child. Could you have a look?

http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-sanitized/541/console

[davide]

Hey Jordan, it looks like some of the changes to TestEditLine [or adjacent to it] broke the sanitized build on macOS.
Can I ask you to take a look? Thanks

http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-sanitized/539/console

[JDevlieghere]

Looks like we happened to be looking at this at the same time :D

[labath]

[This is probably not the right patch, as the last run on that bot still doesn't have this commit.]

My understanding is that all pexpect tests are failing on that bot and are skipped with @skipIfAsan. Probably this one needs to be skipped too. @teemperor was trying to reproduce some of those failures locally and failed... Maybe that bot is cursed...

[davide]

In D70324#1755704, @labath wrote:

[This is probably not the right patch, as the last run on that bot still doesn't have this commit.]

My understanding is that all pexpect tests are failing on that bot and are skipped with @skipIfAsan. Probably this one needs to be skipped too. @teemperor was trying to reproduce some of those failures locally and failed... Maybe that bot is cursed...

Fair.

[rupprecht]

In D70324#1755712, @davide wrote:

In D70324#1755704, @labath wrote:

[This is probably not the right patch, as the last run on that bot still doesn't have this commit.]

-> D70137 is the correct patch that actually adds this test -- this is testonly cleanup

My understanding is that all pexpect tests are failing on that bot and are skipped with @skipIfAsan. Probably this one needs to be skipped too. @teemperor was trying to reproduce some of those failures locally and failed... Maybe that bot is cursed...

Fair.

I looked at the failure in some detail. (I don't have hardware to repro it on though). The test itself runs fine but the cleanup (I guess the self.quit()) at the end seems to fail: pexpect.exceptions.ExceptionPexpect: Could not terminate the child. Is that in line with the failures that @skipIfAsan is intended for?

[teemperor]

In D70324#1757070, @rupprecht wrote:

In D70324#1755712, @davide wrote:

In D70324#1755704, @labath wrote:

[This is probably not the right patch, as the last run on that bot still doesn't have this commit.]

-> D70137 is the correct patch that actually adds this test -- this is testonly cleanup

My understanding is that all pexpect tests are failing on that bot and are skipped with @skipIfAsan. Probably this one needs to be skipped too. @teemperor was trying to reproduce some of those failures locally and failed... Maybe that bot is cursed...

Fair.

I looked at the failure in some detail. (I don't have hardware to repro it on though). The test itself runs fine but the cleanup (I guess the self.quit()) at the end seems to fail: pexpect.exceptions.ExceptionPexpect: Could not terminate the child. Is that in line with the failures that @skipIfAsan is intended for?

Yeah that's the issue all expect tests have with Asan. It's related to all the timeouts in pexpect probably.

lldb/packages/Python/lldbsuite/test/lldbpexpect.py

class PExpectTest(TestBase):
                 self.child.expect_exact(cmd)
                 self.expect_prompt()
             if executable is not None:
                 self.child.expect_exact("target create")
                 self.child.expect_exact("Current executable set to")
                 self.expect_prompt()
 
         def expect(self, cmd, substrs=None):
+            self.assertNotIn('\n', cmd)
             self.child.sendline(cmd)
             if substrs is not None:
                 for s in substrs:
                     self.child.expect_exact(s)
             self.expect_prompt()
 
         def quit(self, gracefully=True):
             self.child.sendeof()
             self.child.close(force=not gracefully)
             self.child = None

lldb/packages/Python/lldbsuite/test/terminal/TestEditline.py

class EditlineTest(PExpectTest):
 
     def test_left_right_arrow(self):
         """Test that ctrl+left/right arrow navigates words correctly.
 
         Note: just sending escape characters to pexpect and checking the buffer
         doesn't work well, so we run real commands. We want to type
         "help command" while exercising word-navigation, so type it as below,
         where [] indicates cursor position.
 
-        1. Send "el ommand" -> "el ommand[]"
-        2. Ctrl+left once   -> "el []ommand"
-        3. Send "c"         -> "el c[]ommand"
-        4. Ctrl+left twice  -> "[]el command"
-        5. Send "h"         -> "h[]el command"
-        6. Ctrl+right       -> "hel[] command"
-        7. Send "p"         -> "help command"
+        1. Send "el rint"  -> "el rint[]"
+        2. Ctrl+left once  -> "el []rint"
+        3. Send "p"        -> "el p[]rint"
+        4. Ctrl+left twice -> "[]el print"
+        5. Send "h"        -> "h[]el print"
+        6. Ctrl+right      -> "hel[] print"
+        7. Send "p"        -> "help print"
         """
         self.launch()
 
-        # Run help for different commands for escape variants to make sure each
-        # one matches uniquely (the buffer isn't cleared in between matches).
-        cases = [
-            ("print", "\x1b[1;5D", "\x1b[1;5C"),
-            ("step", "\x1b[5D", "\x1b[5C"),
-            ("exit", "\x1b\x1b[D", "\x1b\x1b[C"),
+        escape_pairs = [
+            ("\x1b[1;5D", "\x1b[1;5C"),
+            ("\x1b[5D", "\x1b[5C"),
+            ("\x1b\x1b[D", "\x1b\x1b[C"),
         ]
-        for (cmd, l_escape, r_escape) in cases:
-            self.expect("el {cmd_tail}{L}{cmd_head}{L}{L}h{R}p".format(
-                cmd_head=cmd[0], cmd_tail=cmd[1:], L=l_escape, R=r_escape),
-                substrs=["Syntax: %s" % cmd])
+        for (l_escape, r_escape) in escape_pairs:
+            self.expect("el rint{L}p{L}{L}h{R}p".format(
+                L=l_escape, R=r_escape), substrs=["Syntax: print"])
 
         self.quit()