[debugger] Improve resume count checking in tests.
Previously tests would check the total count of resumes sent to the
agent as a way to test whether the program has resumed or not. This
stateful method made it diffult to follow what the value is supposed to
be.
Changes the getter for the resume count to reset the value. The test
code can now check for 1 (resumed) or 0 (stayed stopped) which makes the
test easier to follow and modify.
TEST=this is
Change-Id: I71654b2eaa549b6780eef6cd76f0f65a2cc320ca
diff --git a/garnet/bin/zxdb/client/finish_physical_frame_thread_controller_unittest.cc b/garnet/bin/zxdb/client/finish_physical_frame_thread_controller_unittest.cc
index 5003c40..cba4706 100644
--- a/garnet/bin/zxdb/client/finish_physical_frame_thread_controller_unittest.cc
+++ b/garnet/bin/zxdb/client/finish_physical_frame_thread_controller_unittest.cc
@@ -133,7 +133,7 @@
// Since the return address is null, we should not have attempted to create
// a breakpoint, and the thread should have been resumed.
ASSERT_EQ(0, mock_remote_api()->breakpoint_add_count());
- ASSERT_EQ(1, mock_remote_api()->resume_count());
+ ASSERT_EQ(1, mock_remote_api()->GetAndResetResumeCount());
}
} // namespace zxdb
diff --git a/garnet/bin/zxdb/client/finish_thread_controller_unittest.cc b/garnet/bin/zxdb/client/finish_thread_controller_unittest.cc
index 4854901..85a1915 100644
--- a/garnet/bin/zxdb/client/finish_thread_controller_unittest.cc
+++ b/garnet/bin/zxdb/client/finish_thread_controller_unittest.cc
@@ -42,7 +42,7 @@
// It should have been able to step without doing any further async work.
EXPECT_TRUE(continued);
- EXPECT_EQ(1, mock_remote_api()->resume_count());
+ EXPECT_EQ(1, mock_remote_api()->GetAndResetResumeCount());
// Do one step inside the inline function (add 4 to the address).
mock_frames = GetStack();
@@ -53,7 +53,7 @@
true);
// That's still inside the frame's range, so it should continue.
- EXPECT_EQ(2, mock_remote_api()->resume_count());
+ EXPECT_EQ(1, mock_remote_api()->GetAndResetResumeCount());
// Set exception at the first instruction after the inline frame.
mock_frames = GetStack();
@@ -67,7 +67,7 @@
true);
// Should not have resumed.
- EXPECT_EQ(2, mock_remote_api()->resume_count());
+ EXPECT_EQ(0, mock_remote_api()->GetAndResetResumeCount());
EXPECT_EQ(debug_ipc::ThreadRecord::State::kBlocked, thread()->GetState());
}
@@ -97,7 +97,7 @@
// That should have sent a resume + a breakpoint set at the frame 2 IP (this
// breakpoint is implementing the "finish" to step out of the frame 1
// physical frame).
- EXPECT_EQ(1, mock_remote_api()->resume_count());
+ EXPECT_EQ(1, mock_remote_api()->GetAndResetResumeCount());
EXPECT_EQ(0, mock_remote_api()->breakpoint_remove_count());
EXPECT_EQ(frame_2_ip, mock_remote_api()->last_breakpoint_address());
@@ -120,7 +120,7 @@
// The breakpoint should have been cleared and the thread should have been
// resumed.
- EXPECT_EQ(2, mock_remote_api()->resume_count());
+ EXPECT_EQ(1, mock_remote_api()->GetAndResetResumeCount());
EXPECT_EQ(1, mock_remote_api()->breakpoint_remove_count());
// Do another stop 4 bytes later in the inline frame 2 which should get
@@ -132,7 +132,7 @@
debug_ipc::NotifyException::Type::kSingleStep,
MockFrameVectorToFrameVector(std::move(mock_frames)),
true);
- EXPECT_EQ(3, mock_remote_api()->resume_count());
+ EXPECT_EQ(1, mock_remote_api()->GetAndResetResumeCount());
// Stop in inline frame 1. This leaves inline frame 2 (right after its
// address range) but should still continue since we haven't reached the
@@ -144,7 +144,7 @@
debug_ipc::NotifyException::Type::kSingleStep,
MockFrameVectorToFrameVector(std::move(mock_frames)),
true);
- EXPECT_EQ(4, mock_remote_api()->resume_count());
+ EXPECT_EQ(1, mock_remote_api()->GetAndResetResumeCount());
// Stop in middle frame which is the target (right after the inline 1 range).
mock_frames = GetStack();
@@ -154,7 +154,7 @@
debug_ipc::NotifyException::Type::kSingleStep,
MockFrameVectorToFrameVector(std::move(mock_frames)),
true);
- EXPECT_EQ(4, mock_remote_api()->resume_count()); // Unchanged
+ EXPECT_EQ(0, mock_remote_api()->GetAndResetResumeCount()); // Stopped.
}
} // namespace zxdb
diff --git a/garnet/bin/zxdb/client/mock_remote_api.cc b/garnet/bin/zxdb/client/mock_remote_api.cc
index ab647ac..c6be04f 100644
--- a/garnet/bin/zxdb/client/mock_remote_api.cc
+++ b/garnet/bin/zxdb/client/mock_remote_api.cc
@@ -12,6 +12,12 @@
MockRemoteAPI::MockRemoteAPI() = default;
MockRemoteAPI::~MockRemoteAPI() = default;
+int MockRemoteAPI::GetAndResetResumeCount() {
+ int result = resume_count_;
+ resume_count_ = 0;
+ return result;
+}
+
void MockRemoteAPI::AddOrChangeBreakpoint(
const debug_ipc::AddOrChangeBreakpointRequest& request,
std::function<void(const Err&, debug_ipc::AddOrChangeBreakpointReply)> cb) {
diff --git a/garnet/bin/zxdb/client/mock_remote_api.h b/garnet/bin/zxdb/client/mock_remote_api.h
index 6c29409..c32a5e5 100644
--- a/garnet/bin/zxdb/client/mock_remote_api.h
+++ b/garnet/bin/zxdb/client/mock_remote_api.h
@@ -21,7 +21,7 @@
~MockRemoteAPI();
// Resume.
- int resume_count() const { return resume_count_; }
+ int GetAndResetResumeCount(); // Zeroes out internal value.
// Thread status.
void set_thread_status_reply(const debug_ipc::ThreadStatusReply& reply) {
diff --git a/garnet/bin/zxdb/client/step_over_thread_controller_unittest.cc b/garnet/bin/zxdb/client/step_over_thread_controller_unittest.cc
index 240c9bb..07c1a01 100644
--- a/garnet/bin/zxdb/client/step_over_thread_controller_unittest.cc
+++ b/garnet/bin/zxdb/client/step_over_thread_controller_unittest.cc
@@ -57,14 +57,14 @@
// It should have been able to step without doing any further async work.
EXPECT_TRUE(continued);
- EXPECT_EQ(1, mock_remote_api()->resume_count());
+ EXPECT_EQ(1, mock_remote_api()->GetAndResetResumeCount());
// Issue a stop in the range. This should get transparently resumed. In
// general the backend won't issue this since it will continue stepping in
// the given range, but it could, and we should resume anyway.
exception.thread.frames[0].ip += 4;
InjectException(exception);
- EXPECT_EQ(2, mock_remote_api()->resume_count());
+ EXPECT_EQ(1, mock_remote_api()->GetAndResetResumeCount());
// Issue a stop in a new stack frame. The base pointer will be the same as
// the outer function since the prologue hasn't executed yet. The previous
@@ -79,7 +79,7 @@
// That should have sent a resume + a breakpoint set at the frame 1 IP (this
// breakpoint is implementing the "finish" to step out of the function call).
- EXPECT_EQ(3, mock_remote_api()->resume_count());
+ EXPECT_EQ(1, mock_remote_api()->GetAndResetResumeCount());
EXPECT_EQ(0, mock_remote_api()->breakpoint_remove_count());
EXPECT_EQ(exception.thread.frames[1].ip,
mock_remote_api()->last_breakpoint_address());
@@ -97,7 +97,7 @@
// That should have removed the breakpoint and resumed the thread.
EXPECT_EQ(1, mock_remote_api()->breakpoint_remove_count());
- EXPECT_EQ(4, mock_remote_api()->resume_count());
+ EXPECT_EQ(1, mock_remote_api()->GetAndResetResumeCount());
// Last exception is outside the range (the end is non-inclusive).
exception.hit_breakpoints.clear();
@@ -105,7 +105,7 @@
InjectException(exception);
// Should have stopped.
- EXPECT_EQ(4, mock_remote_api()->resume_count()); // Same value as above.
+ EXPECT_EQ(0, mock_remote_api()->GetAndResetResumeCount()); // Stopped.
EXPECT_EQ(debug_ipc::ThreadRecord::State::kBlocked, thread()->GetState());
}
@@ -133,7 +133,7 @@
thread()->ContinueWith(
std::make_unique<StepOverThreadController>(AddressRanges(step_range1)),
[](const Err& err) {});
- EXPECT_EQ(1, mock_remote_api()->resume_count()); // Continued.
+ EXPECT_EQ(1, mock_remote_api()->GetAndResetResumeCount()); // Continued.
// Issue a stop at the beginning of the inline function. We provide the
// full stack (including the inline function) because that's where the
@@ -141,7 +141,7 @@
InjectExceptionWithStack(process()->GetKoid(), thread()->GetKoid(),
debug_ipc::NotifyException::Type::kSingleStep,
MockFrameVectorToFrameVector(GetStack()), true);
- EXPECT_EQ(1, mock_remote_api()->resume_count()); // Unchanged (stopped).
+ EXPECT_EQ(0, mock_remote_api()->GetAndResetResumeCount()); // Stopped.
// The "step over" controller should have reported "stop" and fixed up the
// stack to be at the call point of the inline function.
@@ -151,7 +151,7 @@
thread()->ContinueWith(std::make_unique<StepOverThreadController>(
AddressRanges(kTopFunctionRange)),
[](const Err& err) {});
- EXPECT_EQ(2, mock_remote_api()->resume_count()); // Continued.
+ EXPECT_EQ(1, mock_remote_api()->GetAndResetResumeCount()); // Continued.
// Pretend that immediately following that inline call is another inline
// call. This is as if two inline functions are back-to-back in the source
diff --git a/garnet/bin/zxdb/client/step_thread_controller_unittest.cc b/garnet/bin/zxdb/client/step_thread_controller_unittest.cc
index cfc5d37..42c2909 100644
--- a/garnet/bin/zxdb/client/step_thread_controller_unittest.cc
+++ b/garnet/bin/zxdb/client/step_thread_controller_unittest.cc
@@ -58,7 +58,7 @@
// It should have been able to step without doing any further async work.
EXPECT_TRUE(continued);
- EXPECT_EQ(1, mock_remote_api()->resume_count());
+ EXPECT_EQ(1, mock_remote_api()->GetAndResetResumeCount());
// Issue a software exception in the range.
exception.type = debug_ipc::NotifyException::Type::kSoftware;
@@ -66,7 +66,7 @@
InjectException(exception);
// It should have stayed stopped despite being in range.
- EXPECT_EQ(1, mock_remote_api()->resume_count()); // Same count as above.
+ EXPECT_EQ(0, mock_remote_api()->GetAndResetResumeCount()); // Stopped
EXPECT_EQ(debug_ipc::ThreadRecord::State::kBlocked, thread()->GetState());
}
@@ -126,23 +126,23 @@
// It should have been able to step without doing any further async work.
EXPECT_TRUE(continued);
- EXPECT_EQ(1, mock_remote_api()->resume_count());
+ EXPECT_EQ(1, mock_remote_api()->GetAndResetResumeCount());
// Stop on 2nd instruction (line 0). This should be automatically resumed.
exception.thread.frames[0].ip = kAddr2;
InjectException(exception);
- EXPECT_EQ(2, mock_remote_api()->resume_count());
+ EXPECT_EQ(1, mock_remote_api()->GetAndResetResumeCount());
// Stop on 3rd instruction (line 10). Since this matches the original line,
// it should be automatically resumed.
exception.thread.frames[0].ip = kAddr3;
InjectException(exception);
- EXPECT_EQ(3, mock_remote_api()->resume_count());
+ EXPECT_EQ(1, mock_remote_api()->GetAndResetResumeCount());
// Stop on 4th instruction. Since this is line 11, we should stay stopped.
exception.thread.frames[0].ip = kAddr4;
InjectException(exception);
- EXPECT_EQ(3, mock_remote_api()->resume_count()); // Same count as above.
+ EXPECT_EQ(0, mock_remote_api()->GetAndResetResumeCount()); // Stopped
EXPECT_EQ(debug_ipc::ThreadRecord::State::kBlocked, thread()->GetState());
}
@@ -199,7 +199,7 @@
// It should have been able to step without doing any further async work.
EXPECT_TRUE(continued);
- EXPECT_EQ(1, mock_remote_api()->resume_count());
+ EXPECT_EQ(1, mock_remote_api()->GetAndResetResumeCount());
// Stop on the thunk instruction with no line info. This is a separate
// function so we push an entry on the stack.
@@ -211,19 +211,19 @@
if (stop_on_no_symbols) {
// For this variant of the test, the unsymbolized thunk should have stopped
// stepping.
- EXPECT_EQ(1, mock_remote_api()->resume_count());
+ EXPECT_EQ(0, mock_remote_api()->GetAndResetResumeCount()); // Stopped.
EXPECT_EQ(debug_ipc::ThreadRecord::State::kBlocked, thread()->GetState());
return;
}
// The rest of this test is the "step over unsymbolized thunks" case. It
// should have automatically resumed from the previous exception.
- EXPECT_EQ(2, mock_remote_api()->resume_count());
+ EXPECT_EQ(1, mock_remote_api()->GetAndResetResumeCount());
// Stop on dest instruction. Since it's a different line, we should now stop.
exception.thread.frames[0].ip = kAddrDest;
InjectException(exception);
- EXPECT_EQ(2, mock_remote_api()->resume_count()); // Unchanged from previous.
+ EXPECT_EQ(0, mock_remote_api()->GetAndResetResumeCount()); // Stopped
EXPECT_EQ(debug_ipc::ThreadRecord::State::kBlocked, thread()->GetState());
}
@@ -276,7 +276,7 @@
// It should have been able to step without doing any further async work.
EXPECT_TRUE(continued);
- EXPECT_EQ(1, mock_remote_api()->resume_count());
+ EXPECT_EQ(1, mock_remote_api()->GetAndResetResumeCount());
// Stop on the destination unsymbolized address.
debug_ipc::NotifyException dest_exception(src_exception);
@@ -291,14 +291,14 @@
if (stop_on_no_symbols) {
// For this variant of the test, the unsymbolized thunk should have stopped
// stepping.
- EXPECT_EQ(1, mock_remote_api()->resume_count());
+ EXPECT_EQ(0, mock_remote_api()->GetAndResetResumeCount()); // Stopped
EXPECT_EQ(debug_ipc::ThreadRecord::State::kBlocked, thread()->GetState());
return;
}
// The rest of this test is the "step over unsymbolized thunks" case. It
// should have automatically resumed from the previous exception.
- EXPECT_EQ(2, mock_remote_api()->resume_count());
+ EXPECT_EQ(1, mock_remote_api()->GetAndResetResumeCount());
// Send a breakpoint completion notification at the previous stack frame.
// Breakpoint exceptions are "software".
@@ -312,12 +312,12 @@
// This should have continued since the return address is still in the
// original address range.
- EXPECT_EQ(3, mock_remote_api()->resume_count());
+ EXPECT_EQ(1, mock_remote_api()->GetAndResetResumeCount());
// Stop on dest instruction, this is still in range so we should continue.
src_exception.thread.frames[0].ip = kAddrOutOfRange;
InjectException(src_exception);
- EXPECT_EQ(3, mock_remote_api()->resume_count()); // Unchanged from previous.
+ EXPECT_EQ(0, mock_remote_api()->GetAndResetResumeCount()); // Stopped
EXPECT_EQ(debug_ipc::ThreadRecord::State::kBlocked, thread()->GetState());
}
