From 6466c35737eff21e9b48c3ce2353d42628f4bb77 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Tue, 10 Jul 2018 17:45:16 -0400 Subject: C++ migration: add a C++ implementation of `FSTExponentialBackoff` (#1465) This is a pretty close port of `FSTExponentialBackoff`. The changes are pretty minor: * delay is calculated using duration types, not plain numbers, which should be a little more type-safe; * split a piece of code into a ClampDelay function, because it's reasonably close to std::clamp; * rephrased the class-level comment to make it clearer that the first attempt always has delay = 0; * added simple tests (other platforms don't have tests for this). Also make sure that canceling a DelayedOperation is always valid. --- .../test/firebase/firestore/remote/CMakeLists.txt | 3 + .../firestore/remote/exponential_backoff_test.cc | 91 ++++++++++++++++++++++ .../test/firebase/firestore/util/executor_test.cc | 24 ++++++ 3 files changed, 118 insertions(+) create mode 100644 Firestore/core/test/firebase/firestore/remote/exponential_backoff_test.cc (limited to 'Firestore/core/test') diff --git a/Firestore/core/test/firebase/firestore/remote/CMakeLists.txt b/Firestore/core/test/firebase/firestore/remote/CMakeLists.txt index 1b4142a..d91dc0f 100644 --- a/Firestore/core/test/firebase/firestore/remote/CMakeLists.txt +++ b/Firestore/core/test/firebase/firestore/remote/CMakeLists.txt @@ -16,8 +16,10 @@ cc_test( firebase_firestore_remote_test SOURCES datastore_test.cc + exponential_backoff_test.cc serializer_test.cc DEPENDS + absl_base # NB: Order is important. We need to include the ffp_libprotobuf library # before ff_remote, or else we'll end up with nanopb's headers earlier in # the include path than libprotobuf's, which makes using libprotobuf in the @@ -26,4 +28,5 @@ cc_test( # exists in both the libprotobuf path and the nanopb path. firebase_firestore_protos_libprotobuf firebase_firestore_remote + firebase_firestore_util_executor_std ) diff --git a/Firestore/core/test/firebase/firestore/remote/exponential_backoff_test.cc b/Firestore/core/test/firebase/firestore/remote/exponential_backoff_test.cc new file mode 100644 index 0000000..dbc34ef --- /dev/null +++ b/Firestore/core/test/firebase/firestore/remote/exponential_backoff_test.cc @@ -0,0 +1,91 @@ +/* + * Copyright 2018 Google + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include // NOLINT(build/c++11) + +#include "Firestore/core/src/firebase/firestore/remote/exponential_backoff.h" +#include "Firestore/core/src/firebase/firestore/util/async_queue.h" +#include "Firestore/core/src/firebase/firestore/util/executor_std.h" +#include "Firestore/core/test/firebase/firestore/util/async_tests_util.h" +#include "absl/memory/memory.h" +#include "gtest/gtest.h" + +using firebase::firestore::util::AsyncQueue; +using firebase::firestore::util::TestWithTimeoutMixin; +using firebase::firestore::util::TimerId; +using firebase::firestore::util::internal::ExecutorStd; + +namespace chr = std::chrono; + +namespace firebase { +namespace firestore { +namespace remote { + +class ExponentialBackoffTest : public TestWithTimeoutMixin, + public testing::Test { + public: + ExponentialBackoffTest() + : queue{absl::make_unique()}, + backoff{&queue, timer_id, 1.5, chr::seconds{5}, chr::seconds{30}} { + } + + TimerId timer_id = TimerId::ListenStreamConnectionBackoff; + AsyncQueue queue; + ExponentialBackoff backoff; +}; + +TEST_F(ExponentialBackoffTest, CanScheduleOperations) { + EXPECT_FALSE(queue.IsScheduled(timer_id)); + + queue.EnqueueBlocking([&] { + backoff.BackoffAndRun([&] { signal_finished(); }); + EXPECT_TRUE(queue.IsScheduled(timer_id)); + }); + + EXPECT_TRUE(WaitForTestToFinish()); + EXPECT_FALSE(queue.IsScheduled(timer_id)); +} + +TEST_F(ExponentialBackoffTest, CanCancelOperations) { + std::string str{"untouched"}; + EXPECT_FALSE(queue.IsScheduled(timer_id)); + + queue.EnqueueBlocking([&] { + backoff.BackoffAndRun([&] { str = "Shouldn't be modified"; }); + EXPECT_TRUE(queue.IsScheduled(timer_id)); + backoff.Cancel(); + }); + + EXPECT_FALSE(queue.IsScheduled(timer_id)); + EXPECT_EQ(str, "untouched"); +} + +TEST_F(ExponentialBackoffTest, SequentialCallsToBackoffAndRun) { + queue.EnqueueBlocking([&] { + backoff.BackoffAndRun([] {}); + backoff.BackoffAndRun([] {}); + backoff.BackoffAndRun([&] { signal_finished(); }); + }); + + // The chosen value of initial_delay is large enough that it shouldn't be + // realistically possible for backoff to finish already. + queue.RunScheduledOperationsUntil(timer_id); + EXPECT_TRUE(WaitForTestToFinish()); +} + +} // namespace remote +} // namespace firestore +} // namespace firebase diff --git a/Firestore/core/test/firebase/firestore/util/executor_test.cc b/Firestore/core/test/firebase/firestore/util/executor_test.cc index 99bddce..e983bfe 100644 --- a/Firestore/core/test/firebase/firestore/util/executor_test.cc +++ b/Firestore/core/test/firebase/firestore/util/executor_test.cc @@ -115,6 +115,30 @@ TEST_P(ExecutorTest, DelayedOperationIsValidAfterTheOperationHasRun) { EXPECT_NO_THROW(delayed_operation.Cancel()); } +TEST_P(ExecutorTest, CancelingEmptyDelayedOperationIsValid) { + DelayedOperation delayed_operation; + EXPECT_NO_THROW(delayed_operation.Cancel()); +} + +TEST_P(ExecutorTest, DoubleCancelingDelayedOperationIsValid) { + std::string steps; + + executor->Execute([&] { + DelayedOperation delayed_operation = Schedule( + executor.get(), Executor::Milliseconds(1), [&steps] { steps += '1'; }); + Schedule(executor.get(), Executor::Milliseconds(5), [&] { + steps += '2'; + signal_finished(); + }); + + delayed_operation.Cancel(); + delayed_operation.Cancel(); + }); + + EXPECT_TRUE(WaitForTestToFinish()); + EXPECT_EQ(steps, "2"); +} + TEST_P(ExecutorTest, IsCurrentExecutor) { EXPECT_FALSE(executor->IsCurrentExecutor()); EXPECT_NE(executor->Name(), executor->CurrentExecutorName()); -- cgit v1.2.3