llgs-tests: Make addition of new tests easier

[labath]

Summary

Adding a new test would require one to duplicate a significant part of
the existing test that we have. This attempts to reduce that by moving
some part of that code to the test fixture. The StandardStartupTest
fixture automatically starts up the server and connects it to the
client. I also add a more low-level TestBase fixture, which allows one
to start up the client and server in a custom way (I am going to need
this for the test I am writing).

lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt

+set(ALL_LLDB_TEST_EXECUTABLES)
+
 function(add_lldb_test_executable test_name)
   set(EXCLUDE_FROM_ALL ON)
   add_llvm_executable(${test_name} NO_INSTALL_RPATH ${ARGN})
   set(outdir ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR})
   set_output_directory(${test_name} BINARY_DIR ${outdir} LIBRARY_DIR ${outdir})
+  list(APPEND ALL_LLDB_TEST_EXECUTABLES ${test_name})
+  set(ALL_LLDB_TEST_EXECUTABLES ${ALL_LLDB_TEST_EXECUTABLES} PARENT_SCOPE)
 endfunction()
 
 add_lldb_test_executable(thread_inferior inferior/thread_inferior.cpp)
 
 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   add_definitions(-DLLDB_SERVER="$<TARGET_FILE:debugserver>")
 else()
   add_definitions(-DLLDB_SERVER="$<TARGET_FILE:lldb-server>")
 endif()
 
-add_definitions(-DTHREAD_INFERIOR="${CMAKE_CURRENT_BINARY_DIR}/thread_inferior")
+add_definitions(
+  -DLLDB_TEST_INFERIOR_PATH="${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}"
+  -DLLDB_TEST_INFERIOR_SUFFIX="${CMAKE_EXECUTABLE_SUFFIX}"
+  )
 add_subdirectory(tests)
-add_dependencies(LLDBServerTests thread_inferior)

lldb/trunk/unittests/tools/lldb-server/tests/CMakeLists.txt

 add_lldb_unittest(LLDBServerTests
+  TestBase.cpp
   TestClient.cpp
   MessageObjects.cpp
   ThreadIdsInJstopinfoTest.cpp
 
   LINK_LIBS
     lldbHost
     lldbCore
     lldbInterpreter
     lldbTarget
     lldbPluginPlatformLinux
     lldbPluginProcessGDBRemote
 
     LLVMTestingSupport
   LINK_COMPONENTS
     Support
   )
+
+add_dependencies(LLDBServerTests lldb-server ${ALL_LLDB_TEST_EXECUTABLES})

lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.h

 //===-- MessageObjects.h ----------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
+#ifndef LLDB_SERVER_TESTS_MESSAGEOBJECTS_H
+#define LLDB_SERVER_TESTS_MESSAGEOBJECTS_H
+
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
 #include <string>
 

 template <typename... Args>
 llvm::Error make_parsing_error(llvm::StringRef format, Args &&... args) {
   std::string error =
       "Unable to parse " +
       llvm::formatv(format.data(), std::forward<Args>(args)...).str();
   return llvm::make_error<llvm::StringError>(error,
                                              llvm::inconvertibleErrorCode());
 }
+
 } // namespace llgs_tests
+
+#endif // LLDB_SERVER_TESTS_MESSAGEOBJECTS_H

lldb/trunk/unittests/tools/lldb-server/tests/TestBase.h

+//===-- TestBase.h ----------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_SERVER_TESTS_TESTBASE_H
+#define LLDB_SERVER_TESTS_TESTBASE_H
+
+#include "TestClient.h"
+#include "lldb/Host/HostInfo.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+
+namespace llgs_tests {
+
+class TestBase: public ::testing::Test {
+public:
+  static void SetUpTestCase() { lldb_private::HostInfo::Initialize(); }
+
+  static std::string getInferiorPath(llvm::StringRef Name) {
+    llvm::SmallString<64> Path(LLDB_TEST_INFERIOR_PATH);
+    llvm::sys::path::append(Path, Name + LLDB_TEST_INFERIOR_SUFFIX);
+    return Path.str();
+  }
+
+  static std::string getLogFileName();
+};
+
+class StandardStartupTest: public TestBase {
+public:
+  void SetUp() override {
+    auto ClientOr = TestClient::launch(getLogFileName());
+    ASSERT_THAT_EXPECTED(ClientOr, llvm::Succeeded());
+    Client = std::move(*ClientOr);
+  }
+
+protected:
+  std::unique_ptr<TestClient> Client;
+};
+
+} // namespace llgs_tests
+
+#endif // LLDB_SERVER_TESTS_TESTBASE_H

lldb/trunk/unittests/tools/lldb-server/tests/TestBase.cpp

+//===-- TestBase.cpp --------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "TestBase.h"
+#include <cstdlib>
+
+using namespace llgs_tests;
+using namespace llvm;
+
+std::string TestBase::getLogFileName() {
+  const auto *test_info =
+      ::testing::UnitTest::GetInstance()->current_test_info();
+  assert(test_info);
+
+  const char *Dir = getenv("LOG_FILE_DIRECTORY");
+  if (!Dir)
+    return "";
+
+  if (!llvm::sys::fs::is_directory(Dir)) {
+    GTEST_LOG_(WARNING) << "Cannot access log directory: " << Dir;
+    return "";
+  }
+
+  SmallString<64> DirStr(Dir);
+  sys::path::append(DirStr, std::string("server-") +
+                                test_info->test_case_name() + "-" +
+                                test_info->name() + ".log");
+  return DirStr.str();
+}
+

lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h

 //===-- TestClient.h --------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
+#ifndef LLDB_SERVER_TESTS_TESTCLIENT_H
+#define LLDB_SERVER_TESTS_TESTCLIENT_H
+
 #include "MessageObjects.h"
 #include "Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h"
 #include "lldb/Target/ProcessLaunchInfo.h"
 #include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/Connection.h"
 #include "llvm/ADT/Optional.h"
 #include <memory>
 #include <string>
 
 namespace llgs_tests {
-// TODO: Make the test client an abstract base class, with different children
-// for different types of connections: llgs v. debugserver
 class TestClient
     : public lldb_private::process_gdb_remote::GDBRemoteCommunicationClient {
 public:
-  static void Initialize();
   static bool IsDebugServer();
   static bool IsLldbServer();
 
-  TestClient(const std::string &test_name, const std::string &test_case_name);
-  virtual ~TestClient();
-  llvm::Error StartDebugger();
-  llvm::Error StopDebugger();
+  /// Launches the server, connects it to the client and returns the client. If
+  /// Log is non-empty, the server will write it's log to this file.
+  static llvm::Expected<std::unique_ptr<TestClient>> launch(llvm::StringRef Log);
+
+  /// Launches the server, while specifying the inferior on its command line.
+  /// When the client connects, it already has a process ready.
+  static llvm::Expected<std::unique_ptr<TestClient>>
+  launch(llvm::StringRef Log, llvm::ArrayRef<llvm::StringRef> InferiorArgs);
+
+  ~TestClient() override;
   llvm::Error SetInferior(llvm::ArrayRef<std::string> inferior_args);
   llvm::Error ListThreadsInStopReply();
   llvm::Error SetBreakpoint(unsigned long address);
   llvm::Error ContinueAll();
   llvm::Error ContinueThread(unsigned long thread_id);
   const ProcessInfo &GetProcessInfo();
   llvm::Optional<JThreadsInfo> GetJThreadsInfo();
   const StopReply &GetLatestStopReply();
   llvm::Error SendMessage(llvm::StringRef message);
   llvm::Error SendMessage(llvm::StringRef message,
                           std::string &response_string);
   llvm::Error SendMessage(llvm::StringRef message, std::string &response_string,
                           PacketResult expected_result);
   unsigned int GetPcRegisterId();
 
 private:
+  TestClient(std::unique_ptr<lldb_private::Connection> Conn);
+
+  llvm::Error QueryProcessInfo();
   llvm::Error Continue(llvm::StringRef message);
-  std::string GenerateLogFileName(const lldb_private::ArchSpec &arch) const;
   std::string FormatFailedResult(
       const std::string &message,
       lldb_private::process_gdb_remote::GDBRemoteCommunication::PacketResult
           result);
 
   llvm::Optional<ProcessInfo> m_process_info;
   llvm::Optional<StopReply> m_stop_reply;
-  lldb_private::ProcessLaunchInfo m_server_process_info;
-  std::string m_test_name;
-  std::string m_test_case_name;
-  unsigned int m_pc_register;
+  unsigned int m_pc_register = UINT_MAX;
 };
+
 } // namespace llgs_tests
+
+#endif // LLDB_SERVER_TESTS_TESTCLIENT_H

lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp

 #include "TestClient.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Host/common/TCPSocket.h"
 #include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
 #include "lldb/Interpreter/Args.h"
 #include "lldb/Target/ProcessLaunchInfo.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 #include <cstdlib>
 #include <future>
 #include <sstream>
 #include <string>
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace llvm;
 
 namespace llgs_tests {
-void TestClient::Initialize() { HostInfo::Initialize(); }
-
 bool TestClient::IsDebugServer() {
   return sys::path::filename(LLDB_SERVER).contains("debugserver");
 }
 
 bool TestClient::IsLldbServer() { return !IsDebugServer(); }
 
-TestClient::TestClient(const std::string &test_name,
-                       const std::string &test_case_name)
-    : m_test_name(test_name), m_test_case_name(test_case_name),
-      m_pc_register(UINT_MAX) {}
+TestClient::TestClient(std::unique_ptr<Connection> Conn) {
+  SetConnection(Conn.release());
+
+  SendAck(); // Send this as a handshake.
+}
+
+TestClient::~TestClient() {
+  std::string response;
+  // Debugserver (non-conformingly?) sends a reply to the k packet instead of
+  // simply closing the connection.
+  PacketResult result =
+      IsDebugServer() ? PacketResult::Success : PacketResult::ErrorDisconnected;
+  EXPECT_THAT_ERROR(SendMessage("k", response, result), Succeeded());
+}
 
-TestClient::~TestClient() {}
+Expected<std::unique_ptr<TestClient>> TestClient::launch(StringRef Log) {
+  return launch(Log, {});
+}
 
-llvm::Error TestClient::StartDebugger() {
+Expected<std::unique_ptr<TestClient>> TestClient::launch(StringRef Log, ArrayRef<StringRef> InferiorArgs) {
   const ArchSpec &arch_spec = HostInfo::GetArchitecture();
   Args args;
   args.AppendArgument(LLDB_SERVER);
-  if (IsLldbServer()) {
+  if (IsLldbServer())
     args.AppendArgument("gdbserver");
+  args.AppendArgument("--reverse-connect");
+
+  if (!Log.empty()) {
+    args.AppendArgument(("--log-file=" + Log).str());
+    if (IsLldbServer())
       args.AppendArgument("--log-channels=gdb-remote packets");
-  } else {
+    else
       args.AppendArgument("--log-flags=0x800000");
   }
-  args.AppendArgument("--reverse-connect");
-  std::string log_file_name = GenerateLogFileName(arch_spec);
-  if (log_file_name.size())
-    args.AppendArgument("--log-file=" + log_file_name);
 
   Status status;
   TCPSocket listen_socket(true, false);
   status = listen_socket.Listen("127.0.0.1:0", 5);
   if (status.Fail())
     return status.ToError();
 
-  char connect_remote_address[64];
-  snprintf(connect_remote_address, sizeof(connect_remote_address),
-           "localhost:%u", listen_socket.GetLocalPortNumber());
-
-  args.AppendArgument(connect_remote_address);
-
-  m_server_process_info.SetArchitecture(arch_spec);
-  m_server_process_info.SetArguments(args, true);
-  status = Host::LaunchProcess(m_server_process_info);
+  args.AppendArgument(
+      ("localhost:" + Twine(listen_socket.GetLocalPortNumber())).str());
+
+  if (!InferiorArgs.empty()) {
+    args.AppendArgument("--");
+    for (StringRef arg : InferiorArgs)
+      args.AppendArgument(arg);
+  }
+
+  ProcessLaunchInfo Info;
+  Info.SetArchitecture(arch_spec);
+  Info.SetArguments(args, true);
+  status = Host::LaunchProcess(Info);
   if (status.Fail())
     return status.ToError();
 
-  char connect_remote_uri[64];
-  snprintf(connect_remote_uri, sizeof(connect_remote_uri), "connect://%s",
-           connect_remote_address);
   Socket *accept_socket;
   listen_socket.Accept(accept_socket);
-  SetConnection(new ConnectionFileDescriptor(accept_socket));
-
-  SendAck(); // Send this as a handshake.
-  return llvm::Error::success();
-}
-
-llvm::Error TestClient::StopDebugger() {
-  std::string response;
-  // Debugserver (non-conformingly?) sends a reply to the k packet instead of
-  // simply closing the connection.
-  PacketResult result =
-      IsDebugServer() ? PacketResult::Success : PacketResult::ErrorDisconnected;
-  return SendMessage("k", response, result);
+  auto Conn = llvm::make_unique<ConnectionFileDescriptor>(accept_socket);
+  return std::unique_ptr<TestClient>(new TestClient(std::move(Conn)));
 }
 
 Error TestClient::SetInferior(llvm::ArrayRef<std::string> inferior_args) {
   StringList env;
   Host::GetEnvironment(env);
   for (size_t i = 0; i < env.GetSize(); ++i) {
     if (SendEnvironmentPacket(env[i].c_str()) != 0) {
       return make_error<StringError>(
for (size_t i = 0; i < inferior_args.size(); i++) {
     std::string hex_encoded = toHex(inferior_args[i]);
     command << hex_encoded.size() << ',' << i << ',' << hex_encoded;
   }
 
   if (Error E = SendMessage(command.str()))
     return E;
   if (Error E = SendMessage("qLaunchSuccess"))
     return E;
-  std::string response;
-  if (Error E = SendMessage("qProcessInfo", response))
+  if (Error E = QueryProcessInfo())
     return E;
-  auto create_or_error = ProcessInfo::Create(response);
-  if (auto create_error = create_or_error.takeError())
-    return create_error;
-
-  m_process_info = *create_or_error;
   return Error::success();
 }
 
 Error TestClient::ListThreadsInStopReply() {
   return SendMessage("QListThreadsInStopReply");
 }
 
 Error TestClient::SetBreakpoint(unsigned long address) {
if (elements["alt-name"] == "pc" || elements["generic"] == "pc") {
       m_pc_register = register_id;
       break;
     }
   }
 
   return m_pc_register;
 }
 
+llvm::Error TestClient::QueryProcessInfo() {
+  std::string response;
+  if (Error E = SendMessage("qProcessInfo", response))
+    return E;
+  auto create_or_error = ProcessInfo::Create(response);
+  if (!create_or_error)
+    return create_or_error.takeError();
+  m_process_info = *create_or_error;
+  return Error::success();
+}
+
 Error TestClient::Continue(StringRef message) {
   assert(m_process_info.hasValue());
 
   std::string response;
   if (Error E = SendMessage(message, response))
     return E;
   auto creation = StopReply::Create(response, m_process_info->GetEndian());
   if (Error E = creation.takeError())
     return E;
 
   m_stop_reply = std::move(*creation);
   return Error::success();
 }
 
-std::string TestClient::GenerateLogFileName(const ArchSpec &arch) const {
-  char *log_directory = getenv("LOG_FILE_DIRECTORY");
-  if (!log_directory)
-    return "";
-
-  if (!llvm::sys::fs::is_directory(log_directory)) {
-    GTEST_LOG_(WARNING) << "Cannot access log directory: " << log_directory;
-    return "";
-  }
-
-  std::string log_file_name;
-  raw_string_ostream log_file(log_file_name);
-  log_file << log_directory << "/lldb-" << m_test_case_name << '-'
-           << m_test_name << '-' << arch.GetArchitectureName() << ".log";
-  return log_file.str();
-}
-
 } // namespace llgs_tests

lldb/trunk/unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp

 //===-- ThreadsInJstopinfoTest.cpp ------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
+#include "TestBase.h"
 #include "TestClient.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 #include <string>
 
 using namespace llgs_tests;
+using namespace llvm;
 
-class ThreadsInJstopinfoTest : public ::testing::Test {
-protected:
-  virtual void SetUp() { TestClient::Initialize(); }
-};
-
-TEST_F(ThreadsInJstopinfoTest, TestStopReplyContainsThreadPcsLlgs) {
-  std::vector<std::string> inferior_args;
-  // This inferior spawns N threads, then forces a break.
-  inferior_args.push_back(THREAD_INFERIOR);
-  inferior_args.push_back("4");
-
-  auto test_info = ::testing::UnitTest::GetInstance()->current_test_info();
-
-  TestClient client(test_info->name(), test_info->test_case_name());
-  ASSERT_THAT_ERROR(client.StartDebugger(), llvm::Succeeded());
-  ASSERT_THAT_ERROR(client.SetInferior(inferior_args), llvm::Succeeded());
-  ASSERT_THAT_ERROR(client.ListThreadsInStopReply(), llvm::Succeeded());
-  ASSERT_THAT_ERROR(client.ContinueAll(), llvm::Succeeded());
-  unsigned int pc_reg = client.GetPcRegisterId();
+TEST_F(StandardStartupTest, TestStopReplyContainsThreadPcs) {
+  // This inferior spawns 4 threads, then forces a break.
+  ASSERT_THAT_ERROR(
+      Client->SetInferior({getInferiorPath("thread_inferior"), "4"}),
+      Succeeded());
+
+  ASSERT_THAT_ERROR(Client->ListThreadsInStopReply(), Succeeded());
+  ASSERT_THAT_ERROR(Client->ContinueAll(), Succeeded());
+  unsigned int pc_reg = Client->GetPcRegisterId();
   ASSERT_NE(pc_reg, UINT_MAX);
 
-  auto jthreads_info = client.GetJThreadsInfo();
+  auto jthreads_info = Client->GetJThreadsInfo();
   ASSERT_TRUE(jthreads_info);
 
-  auto stop_reply = client.GetLatestStopReply();
+  auto stop_reply = Client->GetLatestStopReply();
   auto stop_reply_pcs = stop_reply.GetThreadPcs();
   auto thread_infos = jthreads_info->GetThreadInfos();
   ASSERT_EQ(stop_reply_pcs.size(), thread_infos.size())
       << "Thread count mismatch.";
 
   for (auto stop_reply_pc : stop_reply_pcs) {
     unsigned long tid = stop_reply_pc.first;
     ASSERT_TRUE(thread_infos.find(tid) != thread_infos.end())
         << "Thread ID: " << tid << " not in JThreadsInfo.";
     auto pc_value = thread_infos[tid].ReadRegisterAsUint64(pc_reg);
-    ASSERT_THAT_EXPECTED(pc_value, llvm::Succeeded());
+    ASSERT_THAT_EXPECTED(pc_value, Succeeded());
     ASSERT_EQ(stop_reply_pcs[tid], *pc_value)
         << "Mismatched PC for thread: " << tid;
   }
-
-  ASSERT_THAT_ERROR(client.StopDebugger(), llvm::Succeeded());
 }