[Lldb-commits] [PATCH] D56014: [lldb] - Fix crash when listing the history with the key up.

[George Rimar via Phabricator via lldb-commits]

grimar added a comment.

I ended up with the following test case:

**TestHistoryCrash.py:**

  """We crashed when navigated through the history. Check we do not now."""
  
  from __future__ import print_function
  
  import os
  import sys
  import lldb
  from lldbsuite.test.decorators import *
  from lldbsuite.test.lldbtest import *
  from lldbsuite.test import configuration
  from lldbsuite.test import lldbutil
  
  class TestNoCrashForHistoryRecall(TestBase):
  
      mydir = TestBase.compute_mydir(__file__)
  
      # If your test case doesn't stress debug info, the
      # set this to true.  That way it won't be run once for
      # each debug info format.
      NO_DEBUG_INFO_TESTCASE = True
  
      def setUp(self):
          TestBase.setUp(self)
  
      @expectedFailureAll(
          oslist=["windows"],
          bugnumber="llvm.org/pr22274: need a pexpect replacement for windows")
  
      def test_history_recall(self):
          self.child_prompt = '(lldb) '
          self.sample_test()
  
      def sample_test(self):
          import pexpect
  
          self.child = pexpect.spawn(
              '%s %s' %
              (lldbtest_config.lldbExec, self.lldbOption))
          child = self.child
          child.sendline('print')
          child.expect_exact('Enter expressions')
          child.sendline('\x1b[A')
          child.close()
  
          self.assertEqual(child.exitstatus, 0)

It works, but not in all cases. Problem is that history patch is hardcoded and history is stored in the home directory (`~/.lldb/lldb-expr-history`):
https://github.com/llvm-mirror/lldb/blob/master/source/Host/common/Editline.cpp#L180

When the history file exists and is empty (has a header, but no history lines), the test case can trigger the crash successfully.
But if the history file does not yet exist, or if it is not empty, it does not work (always pass), because does not trigger a line changed.
And in the ideal world on a build bot, I believe for a clean build it should never exist (though I guess bots are not cleaning the `~/.lldb` folder perhaps).

It is not a good idea to touch or modify files outside the build folder from the test, so that does not seem what I can change.
And so I am not sure it is worth to add such test, it can help only in some cases, for a local test runs, for example.
Also, It does not work on windows because all pexpect tests seem ban windows.

Given above I suggest going without a test, perhaps.


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

https://reviews.llvm.org/D56014