[Lldb-commits] [lldb] r317795 - llgs-tests: Replace the "log+return false" pattern with llvm::Error
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Thu Nov 9 07:45:10 PST 2017
Author: labath
Date: Thu Nov 9 07:45:09 2017
New Revision: 317795
URL: http://llvm.org/viewvc/llvm-project?rev=317795&view=rev
Log:
llgs-tests: Replace the "log+return false" pattern with llvm::Error
Summary:
These tests used to log the error message and return plain bool mainly
because at the time they we written, we did not have a nice way to
assert on llvm::Error values. That is no longer true, so replace this
pattern with a more idiomatic approach.
As a part of this patch, I also move the formatting of
GDBRemoteCommunication::PacketResult values out of the test code, as
that can be useful elsewhere.
Reviewers: zturner, eugene
Subscribers: mgorny, lldb-commits
Differential Revision: https://reviews.llvm.org/D39790
Modified:
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
lldb/trunk/unittests/tools/lldb-server/tests/CMakeLists.txt
lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp
lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.h
lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp
lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h
lldb/trunk/unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp?rev=317795&r1=317794&r2=317795&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp Thu Nov 9 07:45:09 2017
@@ -1376,3 +1376,39 @@ void GDBRemoteCommunication::AppendBytes
}
}
}
+
+void llvm::format_provider<GDBRemoteCommunication::PacketResult>::format(
+ const GDBRemoteCommunication::PacketResult &result, raw_ostream &Stream,
+ StringRef Style) {
+ using PacketResult = GDBRemoteCommunication::PacketResult;
+
+ switch (result) {
+ case PacketResult::Success:
+ Stream << "Success";
+ break;
+ case PacketResult::ErrorSendFailed:
+ Stream << "ErrorSendFailed";
+ break;
+ case PacketResult::ErrorSendAck:
+ Stream << "ErrorSendAck";
+ break;
+ case PacketResult::ErrorReplyFailed:
+ Stream << "ErrorReplyFailed";
+ break;
+ case PacketResult::ErrorReplyTimeout:
+ Stream << "ErrorReplyTimeout";
+ break;
+ case PacketResult::ErrorReplyInvalid:
+ Stream << "ErrorReplyInvalid";
+ break;
+ case PacketResult::ErrorReplyAck:
+ Stream << "ErrorReplyAck";
+ break;
+ case PacketResult::ErrorDisconnected:
+ Stream << "ErrorDisconnected";
+ break;
+ case PacketResult::ErrorNoSequenceLock:
+ Stream << "ErrorNoSequenceLock";
+ break;
+ }
+}
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h?rev=317795&r1=317794&r2=317795&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h Thu Nov 9 07:45:09 2017
@@ -290,4 +290,14 @@ private:
} // namespace process_gdb_remote
} // namespace lldb_private
+namespace llvm {
+template <>
+struct format_provider<
+ lldb_private::process_gdb_remote::GDBRemoteCommunication::PacketResult> {
+ static void format(const lldb_private::process_gdb_remote::
+ GDBRemoteCommunication::PacketResult &state,
+ raw_ostream &Stream, StringRef Style);
+};
+} // namespace llvm
+
#endif // liblldb_GDBRemoteCommunication_h_
Modified: lldb/trunk/unittests/tools/lldb-server/tests/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/tests/CMakeLists.txt?rev=317795&r1=317794&r2=317795&view=diff
==============================================================================
--- lldb/trunk/unittests/tools/lldb-server/tests/CMakeLists.txt (original)
+++ lldb/trunk/unittests/tools/lldb-server/tests/CMakeLists.txt Thu Nov 9 07:45:09 2017
@@ -10,6 +10,8 @@ add_lldb_unittest(LLDBServerTests
lldbTarget
lldbPluginPlatformLinux
lldbPluginProcessGDBRemote
+
+ LLVMTestingSupport
LINK_COMPONENTS
Support
)
Modified: lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp?rev=317795&r1=317794&r2=317795&view=diff
==============================================================================
--- lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp (original)
+++ lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp Thu Nov 9 07:45:09 2017
@@ -65,19 +65,16 @@ StringRef ThreadInfo::ReadRegister(unsig
return m_registers.lookup(register_id);
}
-bool ThreadInfo::ReadRegisterAsUint64(unsigned int register_id,
- uint64_t &value) const {
+Expected<uint64_t>
+ThreadInfo::ReadRegisterAsUint64(unsigned int register_id) const {
+ uint64_t value;
std::string value_str(m_registers.lookup(register_id));
- if (!llvm::to_integer(value_str, value, 16)) {
- GTEST_LOG_(ERROR)
- << formatv("ThreadInfo: Unable to parse register value at {0}.",
- register_id)
- .str();
- return false;
- }
+ if (!llvm::to_integer(value_str, value, 16))
+ return make_parsing_error("ThreadInfo value for register {0}: {1}",
+ register_id, value_str);
sys::swapByteOrder(value);
- return true;
+ return value;
}
//====== JThreadsInfo ==========================================================
Modified: lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.h?rev=317795&r1=317794&r2=317795&view=diff
==============================================================================
--- lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.h (original)
+++ lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.h Thu Nov 9 07:45:09 2017
@@ -48,7 +48,7 @@ public:
const RegisterMap ®isters, unsigned int signal);
llvm::StringRef ReadRegister(unsigned int register_id) const;
- bool ReadRegisterAsUint64(unsigned int register_id, uint64_t &value) const;
+ llvm::Expected<uint64_t> ReadRegisterAsUint64(unsigned int register_id) const;
private:
std::string m_name;
Modified: lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp?rev=317795&r1=317794&r2=317795&view=diff
==============================================================================
--- lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp (original)
+++ lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp Thu Nov 9 07:45:09 2017
@@ -42,7 +42,7 @@ TestClient::TestClient(const std::string
TestClient::~TestClient() {}
-bool TestClient::StartDebugger() {
+llvm::Error TestClient::StartDebugger() {
const ArchSpec &arch_spec = HostInfo::GetArchitecture();
Args args;
args.AppendArgument(LLDB_SERVER);
@@ -57,13 +57,11 @@ bool TestClient::StartDebugger() {
if (log_file_name.size())
args.AppendArgument("--log-file=" + log_file_name);
- Status error;
+ Status status;
TCPSocket listen_socket(true, false);
- error = listen_socket.Listen("127.0.0.1:0", 5);
- if (error.Fail()) {
- GTEST_LOG_(ERROR) << "Unable to open listen socket.";
- return 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),
@@ -73,12 +71,9 @@ bool TestClient::StartDebugger() {
m_server_process_info.SetArchitecture(arch_spec);
m_server_process_info.SetArguments(args, true);
- Status status = Host::LaunchProcess(m_server_process_info);
- if (status.Fail()) {
- GTEST_LOG_(ERROR)
- << formatv("Failure to launch lldb server: {0}.", status).str();
- return false;
- }
+ status = Host::LaunchProcess(m_server_process_info);
+ if (status.Fail())
+ return status.ToError();
char connect_remote_uri[64];
snprintf(connect_remote_uri, sizeof(connect_remote_uri), "connect://%s",
@@ -88,10 +83,10 @@ bool TestClient::StartDebugger() {
SetConnection(new ConnectionFileDescriptor(accept_socket));
SendAck(); // Send this as a handshake.
- return true;
+ return llvm::Error::success();
}
-bool TestClient::StopDebugger() {
+llvm::Error TestClient::StopDebugger() {
std::string response;
// Debugserver (non-conformingly?) sends a reply to the k packet instead of
// simply closing the connection.
@@ -100,13 +95,14 @@ bool TestClient::StopDebugger() {
return SendMessage("k", response, result);
}
-bool TestClient::SetInferior(llvm::ArrayRef<std::string> inferior_args) {
+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) {
- GTEST_LOG_(ERROR) << "failed to set environment variable `" << env[i] << "`";
- return false;
+ return make_error<StringError>(
+ formatv("Failed to set environment variable: {0}", env[i]).str(),
+ inconvertibleErrorCode());
}
}
std::stringstream command;
@@ -118,36 +114,32 @@ bool TestClient::SetInferior(llvm::Array
command << hex_encoded.size() << ',' << i << ',' << hex_encoded;
}
- if (!SendMessage(command.str()))
- return false;
- if (!SendMessage("qLaunchSuccess"))
- return false;
+ if (Error E = SendMessage(command.str()))
+ return E;
+ if (Error E = SendMessage("qLaunchSuccess"))
+ return E;
std::string response;
- if (!SendMessage("qProcessInfo", response))
- return false;
+ if (Error E = SendMessage("qProcessInfo", response))
+ return E;
auto create_or_error = ProcessInfo::Create(response);
- if (auto create_error = create_or_error.takeError()) {
- GTEST_LOG_(ERROR) << toString(std::move(create_error));
- return false;
- }
+ if (auto create_error = create_or_error.takeError())
+ return create_error;
m_process_info = *create_or_error;
- return true;
+ return Error::success();
}
-bool TestClient::ListThreadsInStopReply() {
+Error TestClient::ListThreadsInStopReply() {
return SendMessage("QListThreadsInStopReply");
}
-bool TestClient::SetBreakpoint(unsigned long address) {
- std::stringstream command;
- command << "Z0," << std::hex << address << ",1";
- return SendMessage(command.str());
+Error TestClient::SetBreakpoint(unsigned long address) {
+ return SendMessage(formatv("Z0,{0:x-},1", address).str());
}
-bool TestClient::ContinueAll() { return Continue("vCont;c"); }
+Error TestClient::ContinueAll() { return Continue("vCont;c"); }
-bool TestClient::ContinueThread(unsigned long thread_id) {
+Error TestClient::ContinueThread(unsigned long thread_id) {
return Continue(formatv("vCont;c:{0:x-}", thread_id).str());
}
@@ -155,7 +147,7 @@ const ProcessInfo &TestClient::GetProces
Optional<JThreadsInfo> TestClient::GetJThreadsInfo() {
std::string response;
- if (!SendMessage("jThreadsInfo", response))
+ if (SendMessage("jThreadsInfo", response))
return llvm::None;
auto creation = JThreadsInfo::Create(response, m_process_info->GetEndian());
if (auto create_error = creation.takeError()) {
@@ -170,36 +162,37 @@ const StopReply &TestClient::GetLatestSt
return m_stop_reply.getValue();
}
-bool TestClient::SendMessage(StringRef message) {
+Error TestClient::SendMessage(StringRef message) {
std::string dummy_string;
return SendMessage(message, dummy_string);
}
-bool TestClient::SendMessage(StringRef message, std::string &response_string) {
- if (!SendMessage(message, response_string, PacketResult::Success))
- return false;
- else if (response_string[0] == 'E') {
- GTEST_LOG_(ERROR) << "Error " << response_string
- << " while sending message: " << message.str();
- return false;
+Error TestClient::SendMessage(StringRef message, std::string &response_string) {
+ if (Error E = SendMessage(message, response_string, PacketResult::Success))
+ return E;
+ if (response_string[0] == 'E') {
+ return make_error<StringError>(
+ formatv("Error `{0}` while sending message: {1}", response_string,
+ message)
+ .str(),
+ inconvertibleErrorCode());
}
-
- return true;
+ return Error::success();
}
-bool TestClient::SendMessage(StringRef message, std::string &response_string,
- PacketResult expected_result) {
+Error TestClient::SendMessage(StringRef message, std::string &response_string,
+ PacketResult expected_result) {
StringExtractorGDBRemote response;
GTEST_LOG_(INFO) << "Send Packet: " << message.str();
PacketResult result = SendPacketAndWaitForResponse(message, response, false);
response.GetEscapedBinaryData(response_string);
GTEST_LOG_(INFO) << "Read Packet: " << response_string;
- if (result != expected_result) {
- GTEST_LOG_(ERROR) << FormatFailedResult(message, result);
- return false;
- }
+ if (result != expected_result)
+ return make_error<StringError>(
+ formatv("Error sending message `{0}`: {1}", message, result).str(),
+ inconvertibleErrorCode());
- return true;
+ return Error::success();
}
unsigned int TestClient::GetPcRegisterId() {
@@ -209,7 +202,7 @@ unsigned int TestClient::GetPcRegisterId
for (unsigned int register_id = 0;; register_id++) {
std::string message = formatv("qRegisterInfo{0:x-}", register_id).str();
std::string response;
- if (!SendMessage(message, response)) {
+ if (SendMessage(message, response)) {
GTEST_LOG_(ERROR) << "Unable to query register ID for PC register.";
return UINT_MAX;
}
@@ -231,23 +224,18 @@ unsigned int TestClient::GetPcRegisterId
return m_pc_register;
}
-bool TestClient::Continue(StringRef message) {
- if (!m_process_info.hasValue()) {
- GTEST_LOG_(ERROR) << "Continue() called before m_process_info initialized.";
- return false;
- }
+Error TestClient::Continue(StringRef message) {
+ assert(m_process_info.hasValue());
std::string response;
- if (!SendMessage(message, response))
- return false;
+ if (Error E = SendMessage(message, response))
+ return E;
auto creation = StopReply::Create(response, m_process_info->GetEndian());
- if (auto create_error = creation.takeError()) {
- GTEST_LOG_(ERROR) << toString(std::move(create_error));
- return false;
- }
+ if (Error E = creation.takeError())
+ return E;
m_stop_reply = std::move(*creation);
- return true;
+ return Error::success();
}
std::string TestClient::GenerateLogFileName(const ArchSpec &arch) const {
@@ -267,42 +255,4 @@ std::string TestClient::GenerateLogFileN
return log_file.str();
}
-std::string TestClient::FormatFailedResult(const std::string &message,
- PacketResult result) {
- std::string formatted_error;
- raw_string_ostream error_stream(formatted_error);
- error_stream << "Failure sending message: " << message << " Result: ";
-
- switch (result) {
- case PacketResult::ErrorSendFailed:
- error_stream << "ErrorSendFailed";
- break;
- case PacketResult::ErrorSendAck:
- error_stream << "ErrorSendAck";
- break;
- case PacketResult::ErrorReplyFailed:
- error_stream << "ErrorReplyFailed";
- break;
- case PacketResult::ErrorReplyTimeout:
- error_stream << "ErrorReplyTimeout";
- break;
- case PacketResult::ErrorReplyInvalid:
- error_stream << "ErrorReplyInvalid";
- break;
- case PacketResult::ErrorReplyAck:
- error_stream << "ErrorReplyAck";
- break;
- case PacketResult::ErrorDisconnected:
- error_stream << "ErrorDisconnected";
- break;
- case PacketResult::ErrorNoSequenceLock:
- error_stream << "ErrorNoSequenceLock";
- break;
- default:
- error_stream << "Unknown Error";
- }
-
- error_stream.str();
- return formatted_error;
-}
} // namespace llgs_tests
Modified: lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h?rev=317795&r1=317794&r2=317795&view=diff
==============================================================================
--- lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h (original)
+++ lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h Thu Nov 9 07:45:09 2017
@@ -27,27 +27,25 @@ public:
TestClient(const std::string &test_name, const std::string &test_case_name);
virtual ~TestClient();
- LLVM_NODISCARD bool StartDebugger();
- LLVM_NODISCARD bool StopDebugger();
- LLVM_NODISCARD bool SetInferior(llvm::ArrayRef<std::string> inferior_args);
- LLVM_NODISCARD bool ListThreadsInStopReply();
- LLVM_NODISCARD bool SetBreakpoint(unsigned long address);
- LLVM_NODISCARD bool ContinueAll();
- LLVM_NODISCARD bool ContinueThread(unsigned long thread_id);
+ llvm::Error StartDebugger();
+ llvm::Error StopDebugger();
+ 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_NODISCARD bool SendMessage(llvm::StringRef message);
- LLVM_NODISCARD bool SendMessage(llvm::StringRef message,
- std::string &response_string);
- LLVM_NODISCARD bool SendMessage(llvm::StringRef message,
- std::string &response_string,
- PacketResult expected_result);
+ 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:
- LLVM_NODISCARD bool Continue(llvm::StringRef message);
- LLVM_NODISCARD bool GenerateConnectionAddress(std::string &address);
+ llvm::Error Continue(llvm::StringRef message);
std::string GenerateLogFileName(const lldb_private::ArchSpec &arch) const;
std::string FormatFailedResult(
const std::string &message,
Modified: lldb/trunk/unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp?rev=317795&r1=317794&r2=317795&view=diff
==============================================================================
--- lldb/trunk/unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp (original)
+++ lldb/trunk/unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp Thu Nov 9 07:45:09 2017
@@ -8,6 +8,7 @@
//===----------------------------------------------------------------------===//
#include "TestClient.h"
+#include "llvm/Testing/Support/Error.h"
#include "gtest/gtest.h"
#include <string>
@@ -27,10 +28,10 @@ TEST_F(ThreadsInJstopinfoTest, TestStopR
auto test_info = ::testing::UnitTest::GetInstance()->current_test_info();
TestClient client(test_info->name(), test_info->test_case_name());
- ASSERT_TRUE(client.StartDebugger());
- ASSERT_TRUE(client.SetInferior(inferior_args));
- ASSERT_TRUE(client.ListThreadsInStopReply());
- ASSERT_TRUE(client.ContinueAll());
+ 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();
ASSERT_NE(pc_reg, UINT_MAX);
@@ -47,12 +48,11 @@ TEST_F(ThreadsInJstopinfoTest, TestStopR
unsigned long tid = stop_reply_pc.first;
ASSERT_TRUE(thread_infos.find(tid) != thread_infos.end())
<< "Thread ID: " << tid << " not in JThreadsInfo.";
- uint64_t pc_value;
- ASSERT_TRUE(thread_infos[tid].ReadRegisterAsUint64(pc_reg, pc_value))
- << "Failure reading ThreadInfo register " << pc_reg;
- ASSERT_EQ(stop_reply_pcs[tid], pc_value)
+ auto pc_value = thread_infos[tid].ReadRegisterAsUint64(pc_reg);
+ ASSERT_THAT_EXPECTED(pc_value, llvm::Succeeded());
+ ASSERT_EQ(stop_reply_pcs[tid], *pc_value)
<< "Mismatched PC for thread: " << tid;
}
- ASSERT_TRUE(client.StopDebugger());
+ ASSERT_THAT_ERROR(client.StopDebugger(), llvm::Succeeded());
}
More information about the lldb-commits
mailing list