diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 71fec404b35d..886873ce6caa 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -46,8 +46,8 @@ repos: rev: 2.0.0 hooks: - id: cpplint - args: ["--filter=-whitespace/line_length,-build/c++11,-build/c++14,-build/c++17,-readability/braces,-whitespace/indent_namespace,-runtime/int,-runtime/references,-build/include_order"] - files: ^src/ray/(util|raylet_client|scheduling)/.*\.(h|cc)$ + args: ["--filter=-whitespace/braces,-whitespace/line_length,-build/c++11,-build/c++14,-build/c++17,-readability/braces,-whitespace/indent_namespace,-runtime/int,-runtime/references,-build/include_order"] + files: ^src/ray/(util|raylet_client|scheduling|pubsub(?:/.*)?)/.*\.(h|cc)$ - repo: https://github.com/psf/black rev: 22.10.0 diff --git a/src/ray/pubsub/publisher.cc b/src/ray/pubsub/publisher.cc index 444de5da9fbd..ec9de99534a8 100644 --- a/src/ray/pubsub/publisher.cc +++ b/src/ray/pubsub/publisher.cc @@ -14,6 +14,11 @@ #include "ray/pubsub/publisher.h" +#include +#include +#include +#include + #include "ray/common/ray_config.h" namespace ray { diff --git a/src/ray/pubsub/publisher.h b/src/ray/pubsub/publisher.h index b64717c535f9..f70f53f52605 100644 --- a/src/ray/pubsub/publisher.h +++ b/src/ray/pubsub/publisher.h @@ -18,9 +18,12 @@ #include #include +#include #include #include #include +#include +#include #include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" @@ -91,7 +94,7 @@ class EntityState { /// Also supports subscribers to all keys in the channel. class SubscriptionIndex { public: - SubscriptionIndex(rpc::ChannelType channel_type); + explicit SubscriptionIndex(rpc::ChannelType channel_type); ~SubscriptionIndex() = default; SubscriptionIndex(SubscriptionIndex &&) noexcept = default; diff --git a/src/ray/pubsub/subscriber.cc b/src/ray/pubsub/subscriber.cc index 93364b4d9774..9b5d5f808821 100644 --- a/src/ray/pubsub/subscriber.cc +++ b/src/ray/pubsub/subscriber.cc @@ -14,6 +14,11 @@ #include "ray/pubsub/subscriber.h" +#include +#include +#include +#include + namespace ray { namespace pubsub { diff --git a/src/ray/pubsub/subscriber.h b/src/ray/pubsub/subscriber.h index ae45fc331377..b5753a949122 100644 --- a/src/ray/pubsub/subscriber.h +++ b/src/ray/pubsub/subscriber.h @@ -17,7 +17,11 @@ #include #include +#include #include +#include +#include +#include #include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" diff --git a/src/ray/pubsub/test/integration_test.cc b/src/ray/pubsub/test/integration_test.cc index b6fe87326736..62ec12c50d15 100644 --- a/src/ray/pubsub/test/integration_test.cc +++ b/src/ray/pubsub/test/integration_test.cc @@ -14,6 +14,8 @@ #include #include +#include +#include #include "absl/synchronization/blocking_counter.h" #include "absl/synchronization/mutex.h" diff --git a/src/ray/pubsub/test/publisher_test.cc b/src/ray/pubsub/test/publisher_test.cc index 69b9cd245259..600845d9116a 100644 --- a/src/ray/pubsub/test/publisher_test.cc +++ b/src/ray/pubsub/test/publisher_test.cc @@ -14,6 +14,11 @@ #include "ray/pubsub/publisher.h" +#include +#include +#include +#include + #include "gmock/gmock.h" #include "gtest/gtest.h" #include "ray/common/asio/instrumented_io_context.h" @@ -27,7 +32,8 @@ namespace { const NodeID kDefaultPublisherId = NodeID::FromRandom(); } -using namespace pub_internal; +using pub_internal::SubscriberState; +using pub_internal::SubscriptionIndex; class PublisherTest : public ::testing::Test { public: diff --git a/src/ray/pubsub/test/subscriber_test.cc b/src/ray/pubsub/test/subscriber_test.cc index 2adb6ed287ac..78032136c7d1 100644 --- a/src/ray/pubsub/test/subscriber_test.cc +++ b/src/ray/pubsub/test/subscriber_test.cc @@ -14,6 +14,15 @@ #include "ray/pubsub/subscriber.h" +#include +#include +#include +#include +#include +#include +#include +#include + #include "gmock/gmock.h" #include "gtest/gtest.h" #include "ray/common/asio/instrumented_io_context.h" @@ -236,13 +245,13 @@ TEST_F(SubscriberTest, TestBasicSubscription) { ASSERT_TRUE(ReplyLongPolling(channel, objects_batched)); // Make sure the long polling batch works as expected. for (const auto &object_id : objects_batched) { - ASSERT_TRUE(object_subscribed_[object_id] == 1); + ASSERT_EQ(object_subscribed_[object_id], 1); } // Publish the objects again, and subscriber should receive it. ASSERT_TRUE(ReplyLongPolling(channel, objects_batched)); for (const auto &object_id : objects_batched) { - ASSERT_TRUE(object_subscribed_[object_id] == 2); + ASSERT_EQ(object_subscribed_[object_id], 2); } ASSERT_TRUE(subscriber_->Unsubscribe(channel, owner_addr, object_id.Binary())); @@ -279,7 +288,7 @@ TEST_F(SubscriberTest, TestIgnoreOutofOrderMessage) { ASSERT_EQ(2, owner_client->GetReportedMaxProcessedSequenceId()); for (const auto &object_id : objects_batched) { - ASSERT_TRUE(object_subscribed_[object_id] == 1); + ASSERT_EQ(object_subscribed_[object_id], 1); } // By resetting the sequence_id, the message now come out of order, @@ -289,14 +298,14 @@ TEST_F(SubscriberTest, TestIgnoreOutofOrderMessage) { // Make sure the long polling batch works as expected. for (const auto &object_id : objects_batched) { - ASSERT_TRUE(object_subscribed_[object_id] == 1); + ASSERT_EQ(object_subscribed_[object_id], 1); } // message arrives out of order (sequence_id 4 comes before 3), // we will ignore message with sequence id 3. ASSERT_TRUE(ReplyLongPolling(channel, objects_batched, {4, 3})); - ASSERT_TRUE(object_subscribed_[object_id] == 2); - ASSERT_TRUE(object_subscribed_[object_id1] == 1); + ASSERT_EQ(object_subscribed_[object_id], 2); + ASSERT_EQ(object_subscribed_[object_id1], 1); ASSERT_EQ(4, owner_client->GetReportedMaxProcessedSequenceId()); } @@ -325,7 +334,7 @@ TEST_F(SubscriberTest, TestPublisherFailsOver) { ASSERT_EQ(2, owner_client->GetReportedMaxProcessedSequenceId()); for (const auto &object_id : objects_batched) { - ASSERT_TRUE(object_subscribed_[object_id] == 1); + ASSERT_EQ(object_subscribed_[object_id], 1); } // By resetting the sequence_id, the message now come out of order, @@ -376,7 +385,7 @@ TEST_F(SubscriberTest, TestSingleLongPollingWithMultipleSubscriptions) { // Make sure the long polling batch works as expected. for (const auto &object_id : objects_batched) { // RAY_LOG(ERROR) << "haha " << object_subscribed_[object_id]; - ASSERT_TRUE(object_subscribed_[object_id] > 0); + ASSERT_GT(object_subscribed_[object_id], 0); } } @@ -407,7 +416,7 @@ TEST_F(SubscriberTest, TestMultiLongPollingWithTheSameSubscription) { std::vector objects_batched; objects_batched.push_back(object_id); ASSERT_TRUE(ReplyLongPolling(channel, objects_batched)); - ASSERT_TRUE(object_subscribed_[object_id] > 0); + ASSERT_GT(object_subscribed_[object_id], 0); objects_batched.clear(); object_subscribed_.clear(); @@ -415,7 +424,7 @@ TEST_F(SubscriberTest, TestMultiLongPollingWithTheSameSubscription) { ASSERT_EQ(owner_client->GetNumberOfInFlightLongPollingRequests(), 1); objects_batched.push_back(object_id); ASSERT_TRUE(ReplyLongPolling(channel, objects_batched)); - ASSERT_TRUE(object_subscribed_[object_id] > 0); + ASSERT_GT(object_subscribed_[object_id], 0); } TEST_F(SubscriberTest, TestCallbackNotInvokedForNonSubscribedObject) {