Skip to content

Commit

Permalink
Remove general timeouts from CDPAgentTest
Browse files Browse the repository at this point in the history
Summary:
Many of our tests are flaky and one of the reasons is that we hit the
arbitrarily defined timeout. This diff removes all timeouts except the
case where we're using timeout to check there is no response.

Reviewed By: neildhar

Differential Revision: D68528488

fbshipit-source-id: 1a2052b942417c2d41c4a0216f8f2a1b5c83204e
  • Loading branch information
dannysu authored and facebook-github-bot committed Jan 24, 2025
1 parent d63b9d7 commit 8997563
Showing 1 changed file with 12 additions and 9 deletions.
21 changes: 12 additions & 9 deletions unittests/API/CDPAgentTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,7 @@ T waitFor(

callback(promise);

auto status = future.wait_for(std::chrono::milliseconds(2500));
if (status != std::future_status::ready) {
throw std::runtime_error("triggerInterrupt didn't get executed: " + reason);
}
future.wait();
return future.get();
}

Expand Down Expand Up @@ -130,7 +127,7 @@ class CDPAgentTest : public ::testing::Test {
/// from the debugger. returns the message. throws on timeout.
std::string waitForMessage(
std::string context = "reply",
std::chrono::milliseconds timeout = std::chrono::milliseconds(2500));
std::optional<std::chrono::milliseconds> timeout = std::nullopt);

/// check to see if a response or notification is immediately available.
/// returns the message, or nullopt if no message is available.
Expand Down Expand Up @@ -361,11 +358,17 @@ void CDPAgentTest::waitForScheduledScripts() {

std::string CDPAgentTest::waitForMessage(
std::string context,
std::chrono::milliseconds timeout) {
std::optional<std::chrono::milliseconds> timeout) {
std::unique_lock<std::mutex> lock(messageMutex_);

bool success = hasMessage_.wait_for(
lock, timeout, [this]() -> bool { return !messages_.empty(); });
bool success;
if (timeout.has_value()) {
success = hasMessage_.wait_for(
lock, timeout.value(), [this]() -> bool { return !messages_.empty(); });
} else {
hasMessage_.wait(lock, [this]() -> bool { return !messages_.empty(); });
success = true;
}

if (!success) {
throw std::runtime_error("timed out waiting for " + context);
Expand Down Expand Up @@ -394,7 +397,7 @@ std::optional<std::string> CDPAgentTest::tryGetMessage() {
void CDPAgentTest::expectNothing() {
std::string message;
try {
message = waitForMessage();
message = waitForMessage("nothing", std::chrono::milliseconds(2500));
} catch (...) {
// if no values are received it times out with an exception
// so we can say that we've succeeded at seeing nothing
Expand Down

0 comments on commit 8997563

Please sign in to comment.