From 2cfb9ed69b06fe68274333287455fda21cd91c8a Mon Sep 17 00:00:00 2001 From: James Sedgwick Date: Fri, 21 Nov 2014 13:06:05 -0800 Subject: [PATCH] ChannelPipeline tests and fixes Summary: As above. This paid off with a couple bugfixes. Test Plan: run em all Reviewed By: hans@fb.com Subscribers: fugalh, njormrod, folly-diffs@ FB internal diff: D1695106 Signature: t1:1695106:1416522038:0c3345aadf954bf346d35b99877e7f8dfcf3ceff --- .../wangle/channel/AsyncSocketHandler.h | 2 +- .../wangle/channel/ChannelPipeline.h | 21 +- ...hannelTest.cpp => ChannelPipelineTest.cpp} | 31 ++- .../channel/test/ChannelPipelineTest.cpp | 251 ++++++++++++++++++ .../wangle/channel/test/MockChannelHandler.h | 64 +++++ .../test/OutputBufferingHandlerTest.cpp | 22 +- 6 files changed, 369 insertions(+), 22 deletions(-) rename folly/experimental/wangle/channel/{ChannelTest.cpp => ChannelPipelineTest.cpp} (80%) create mode 100644 folly/experimental/wangle/channel/test/ChannelPipelineTest.cpp create mode 100644 folly/experimental/wangle/channel/test/MockChannelHandler.h diff --git a/folly/experimental/wangle/channel/AsyncSocketHandler.h b/folly/experimental/wangle/channel/AsyncSocketHandler.h index 91277b68..8d586d0f 100644 --- a/folly/experimental/wangle/channel/AsyncSocketHandler.h +++ b/folly/experimental/wangle/channel/AsyncSocketHandler.h @@ -123,7 +123,7 @@ class AsyncSocketHandler void readErr(const AsyncSocketException& ex) noexcept override { - ctx_->fireReadException(ex); + ctx_->fireReadException(make_exception_wrapper(ex)); } private: diff --git a/folly/experimental/wangle/channel/ChannelPipeline.h b/folly/experimental/wangle/channel/ChannelPipeline.h index 7af4adb9..ea7959bc 100644 --- a/folly/experimental/wangle/channel/ChannelPipeline.h +++ b/folly/experimental/wangle/channel/ChannelPipeline.h @@ -26,6 +26,10 @@ namespace folly { namespace wangle { +/* + * R is the inbound type, i.e. inbound calls start with pipeline.read(R) + * W is the outbound type, i.e. outbound calls start with pipeline.write(W) + */ template class ChannelPipeline; @@ -84,8 +88,11 @@ class ChannelPipeline : public DelayedDestruction { template ChannelPipeline& addFront(H&& handler) { - ctxs_.insert(0, folly::make_unique>( - this, std::forward(handler))); + ctxs_.insert( + ctxs_.begin(), + folly::make_unique>( + this, + std::forward(handler))); return *this; } @@ -171,6 +178,7 @@ class ChannelPipeline finalize(); } } + public: template explicit ChannelPipeline(HandlersArgs&&... handlersArgs) @@ -195,7 +203,7 @@ class ChannelPipeline void readException(exception_wrapper e) { typename ChannelPipeline::DestructorGuard dg( static_cast(this)); - front_->readEOF(std::move(e)); + front_->readException(std::move(e)); } Future write(W msg) { @@ -242,8 +250,11 @@ class ChannelPipeline template ChannelPipeline& addFront(H&& handler) { - ctxs_.insert(0, folly::make_unique>( - this, std::move(handler))); + ctxs_.insert( + ctxs_.begin(), + folly::make_unique>( + this, + std::move(handler))); return *this; } diff --git a/folly/experimental/wangle/channel/ChannelTest.cpp b/folly/experimental/wangle/channel/ChannelPipelineTest.cpp similarity index 80% rename from folly/experimental/wangle/channel/ChannelTest.cpp rename to folly/experimental/wangle/channel/ChannelPipelineTest.cpp index 2f155226..bab898d9 100644 --- a/folly/experimental/wangle/channel/ChannelTest.cpp +++ b/folly/experimental/wangle/channel/ChannelPipelineTest.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -106,7 +107,7 @@ TEST(ChannelTest, PlzCompile2) { pipeline.read(42); } -TEST(ChannelTest, HandlersCompile) { +TEST(ChannelTest, RealHandlersCompile) { EventBase eb; auto socket = AsyncSocket::newSocket(&eb); ChannelPipeline> pipeline; @@ -115,3 +116,31 @@ TEST(ChannelTest, HandlersCompile) { .addBack(OutputBufferingHandler()) .finalize(); } + +TEST(ChannelTest, MoveOnlyTypesCompile) { + ChannelPipeline, + BytesToBytesHandler, + BytesToBytesHandler> + pipeline(BytesToBytesHandler{}, BytesToBytesHandler{}); + pipeline + .addFront(BytesToBytesHandler{}) + .addBack(BytesToBytesHandler{}) + .finalize(); +} + +typedef StrictMock> IntHandler; + +ACTION(FireRead) { + arg0->fireRead(arg1); +} + +TEST(ChannelTest, Handoffs) { + IntHandler handler1; + IntHandler handler2; + MockChannelHandlerAdapter handler2; + ChannelPipeline, + ChannelHandlerPtr, + ChannelHandlerPtr> + pipeline(IntHandler{}, IntHandler{}); + pipeline.read(1); +} diff --git a/folly/experimental/wangle/channel/test/ChannelPipelineTest.cpp b/folly/experimental/wangle/channel/test/ChannelPipelineTest.cpp new file mode 100644 index 00000000..cae1d063 --- /dev/null +++ b/folly/experimental/wangle/channel/test/ChannelPipelineTest.cpp @@ -0,0 +1,251 @@ +/* + * Copyright 2014 Facebook, Inc. + * + * 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 +#include +#include +#include +#include +#include +#include + +using namespace folly; +using namespace folly::wangle; +using namespace testing; + +typedef StrictMock> IntHandler; +typedef ChannelHandlerPtr IntHandlerPtr; + +ACTION(FireRead) { + arg0->fireRead(arg1); +} + +ACTION(FireReadEOF) { + arg0->fireReadEOF(); +} + +ACTION(FireReadException) { + arg0->fireReadException(arg1); +} + +ACTION(FireWrite) { + arg0->fireWrite(arg1); +} + +ACTION(FireClose) { + arg0->fireClose(); +} + +// Test move only types, among other things +TEST(ChannelTest, RealHandlersCompile) { + EventBase eb; + auto socket = AsyncSocket::newSocket(&eb); + // static + { + ChannelPipeline, + AsyncSocketHandler, + OutputBufferingHandler> + pipeline{AsyncSocketHandler(socket), OutputBufferingHandler()}; + EXPECT_TRUE(pipeline.getHandler(0)); + EXPECT_TRUE(pipeline.getHandler(1)); + } + // dynamic + { + ChannelPipeline> pipeline; + pipeline + .addBack(AsyncSocketHandler(socket)) + .addBack(OutputBufferingHandler()) + .finalize(); + EXPECT_TRUE(pipeline.getHandler(0)); + EXPECT_TRUE(pipeline.getHandler(1)); + } +} + +// Test that handlers correctly fire the next handler when directed +TEST(ChannelTest, FireActions) { + IntHandler handler1; + IntHandler handler2; + + EXPECT_CALL(handler1, attachPipeline(_)); + EXPECT_CALL(handler2, attachPipeline(_)); + + ChannelPipeline + pipeline(&handler1, &handler2); + + EXPECT_CALL(handler1, read_(_, _)).WillOnce(FireRead()); + EXPECT_CALL(handler2, read_(_, _)).Times(1); + pipeline.read(1); + + EXPECT_CALL(handler1, readEOF(_)).WillOnce(FireReadEOF()); + EXPECT_CALL(handler2, readEOF(_)).Times(1); + pipeline.readEOF(); + + EXPECT_CALL(handler1, readException(_, _)).WillOnce(FireReadException()); + EXPECT_CALL(handler2, readException(_, _)).Times(1); + pipeline.readException(make_exception_wrapper("blah")); + + EXPECT_CALL(handler2, write_(_, _)).WillOnce(FireWrite()); + EXPECT_CALL(handler1, write_(_, _)).Times(1); + EXPECT_NO_THROW(pipeline.write(1).value()); + + EXPECT_CALL(handler2, close_(_)).WillOnce(FireClose()); + EXPECT_CALL(handler1, close_(_)).Times(1); + EXPECT_NO_THROW(pipeline.close().value()); + + EXPECT_CALL(handler1, detachPipeline(_)); + EXPECT_CALL(handler2, detachPipeline(_)); +} + +// Test that nothing bad happens when actions reach the end of the pipeline +// (a warning will be logged, however) +TEST(ChannelTest, ReachEndOfPipeline) { + IntHandler handler; + EXPECT_CALL(handler, attachPipeline(_)); + ChannelPipeline + pipeline(&handler); + + EXPECT_CALL(handler, read_(_, _)).WillOnce(FireRead()); + pipeline.read(1); + + EXPECT_CALL(handler, readEOF(_)).WillOnce(FireReadEOF()); + pipeline.readEOF(); + + EXPECT_CALL(handler, readException(_, _)).WillOnce(FireReadException()); + pipeline.readException(make_exception_wrapper("blah")); + + EXPECT_CALL(handler, write_(_, _)).WillOnce(FireWrite()); + EXPECT_NO_THROW(pipeline.write(1).value()); + + EXPECT_CALL(handler, close_(_)).WillOnce(FireClose()); + EXPECT_NO_THROW(pipeline.close().value()); + + EXPECT_CALL(handler, detachPipeline(_)); +} + +// Test having the last read handler turn around and write +TEST(ChannelTest, TurnAround) { + IntHandler handler1; + IntHandler handler2; + + EXPECT_CALL(handler1, attachPipeline(_)); + EXPECT_CALL(handler2, attachPipeline(_)); + + ChannelPipeline + pipeline(&handler1, &handler2); + + EXPECT_CALL(handler1, read_(_, _)).WillOnce(FireRead()); + EXPECT_CALL(handler2, read_(_, _)).WillOnce(FireWrite()); + EXPECT_CALL(handler1, write_(_, _)).Times(1); + pipeline.read(1); + + EXPECT_CALL(handler1, detachPipeline(_)); + EXPECT_CALL(handler2, detachPipeline(_)); +} + +TEST(ChannelTest, DynamicFireActions) { + IntHandler handler1, handler2, handler3; + EXPECT_CALL(handler2, attachPipeline(_)); + ChannelPipeline + pipeline(&handler2); + + EXPECT_CALL(handler1, attachPipeline(_)); + EXPECT_CALL(handler3, attachPipeline(_)); + + pipeline + .addFront(IntHandlerPtr(&handler1)) + .addBack(IntHandlerPtr(&handler3)) + .finalize(); + + EXPECT_TRUE(pipeline.getHandler(0)); + EXPECT_TRUE(pipeline.getHandler(1)); + EXPECT_TRUE(pipeline.getHandler(2)); + + EXPECT_CALL(handler1, read_(_, _)).WillOnce(FireRead()); + EXPECT_CALL(handler2, read_(_, _)).WillOnce(FireRead()); + EXPECT_CALL(handler3, read_(_, _)).Times(1); + pipeline.read(1); + + EXPECT_CALL(handler3, write_(_, _)).WillOnce(FireWrite()); + EXPECT_CALL(handler2, write_(_, _)).WillOnce(FireWrite()); + EXPECT_CALL(handler1, write_(_, _)).Times(1); + EXPECT_NO_THROW(pipeline.write(1).value()); + + EXPECT_CALL(handler1, detachPipeline(_)); + EXPECT_CALL(handler2, detachPipeline(_)); + EXPECT_CALL(handler3, detachPipeline(_)); +} + +template +class ConcreteChannelHandler : public ChannelHandler { + typedef typename ChannelHandler::Context Context; + public: + void read(Context* ctx, Rin msg) {} + Future write(Context* ctx, Win msg) { return makeFuture(); } +}; + +typedef ChannelHandlerAdapter StringHandler; +typedef ConcreteChannelHandler IntToStringHandler; +typedef ConcreteChannelHandler StringToIntHandler; + +TEST(ChannelPipeline, DynamicConstruction) { + { + ChannelPipeline pipeline; + EXPECT_THROW( + pipeline + .addBack(ChannelHandlerAdapter{}) + .finalize(), std::invalid_argument); + } + { + ChannelPipeline pipeline; + EXPECT_THROW( + pipeline + .addFront(ChannelHandlerAdapter{}) + .finalize(), + std::invalid_argument); + } + { + ChannelPipeline + pipeline{StringHandler(), StringHandler()}; + + // Exercise both addFront and addBack. Final pipeline is + // StI <-> ItS <-> StS <-> StS <-> StI <-> ItS + EXPECT_NO_THROW( + pipeline + .addFront(IntToStringHandler{}) + .addFront(StringToIntHandler{}) + .addBack(StringToIntHandler{}) + .addBack(IntToStringHandler{}) + .finalize()); + } +} + +TEST(ChannelPipeline, AttachTransport) { + IntHandler handler; + EXPECT_CALL(handler, attachPipeline(_)); + ChannelPipeline + pipeline(&handler); + + EventBase eb; + auto socket = AsyncSocket::newSocket(&eb); + + EXPECT_CALL(handler, attachTransport(_)); + pipeline.attachTransport(socket); + + EXPECT_CALL(handler, detachTransport(_)); + pipeline.detachTransport(); + + EXPECT_CALL(handler, detachPipeline(_)); +} diff --git a/folly/experimental/wangle/channel/test/MockChannelHandler.h b/folly/experimental/wangle/channel/test/MockChannelHandler.h new file mode 100644 index 00000000..ddf511cb --- /dev/null +++ b/folly/experimental/wangle/channel/test/MockChannelHandler.h @@ -0,0 +1,64 @@ +/* + * Copyright 2014 Facebook, Inc. + * + * 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. + */ + +#pragma once + +#include +#include + +namespace folly { namespace wangle { + +template +class MockChannelHandler : public ChannelHandler { + public: + typedef typename ChannelHandler::Context Context; + + MockChannelHandler() = default; + MockChannelHandler(MockChannelHandler&&) = default; + + MOCK_METHOD2_T(read_, void(Context*, Rin&)); + MOCK_METHOD1_T(readEOF, void(Context*)); + MOCK_METHOD2_T(readException, void(Context*, exception_wrapper)); + + MOCK_METHOD2_T(write_, void(Context*, Win&)); + MOCK_METHOD1_T(close_, void(Context*)); + + MOCK_METHOD1_T(attachPipeline, void(Context*)); + MOCK_METHOD1_T(attachTransport, void(Context*)); + MOCK_METHOD1_T(detachPipeline, void(Context*)); + MOCK_METHOD1_T(detachTransport, void(Context*)); + + void read(Context* ctx, Rin msg) { + read_(ctx, msg); + } + + Future write(Context* ctx, Win msg) override { + return makeFutureTry([&](){ + write_(ctx, msg); + }); + } + + Future close(Context* ctx) override { + return makeFutureTry([&](){ + close_(ctx); + }); + } +}; + +template +using MockChannelHandlerAdapter = MockChannelHandler; + +}} diff --git a/folly/experimental/wangle/channel/test/OutputBufferingHandlerTest.cpp b/folly/experimental/wangle/channel/test/OutputBufferingHandlerTest.cpp index cf7af136..600a6a85 100644 --- a/folly/experimental/wangle/channel/test/OutputBufferingHandlerTest.cpp +++ b/folly/experimental/wangle/channel/test/OutputBufferingHandlerTest.cpp @@ -16,6 +16,7 @@ #include #include +#include #include #include #include @@ -24,27 +25,16 @@ using namespace folly; using namespace folly::wangle; using namespace testing; -// TODO(jsedgwick) Extract this to somewhere common and fill it out. -template -class MockChannelHandler : public ChannelHandlerAdapter { - public: - typedef typename ChannelHandlerAdapter::Context Context; - MOCK_METHOD2_T(read, void(Context*, R)); - MOCK_METHOD2_T(write_, void(Context*, W&)); - - Future write(Context* ctx, W msg) override { - write_(ctx, msg); - return makeFuture(); - } -}; - -typedef StrictMock>> +typedef StrictMock>> MockHandler; MATCHER_P(IOBufContains, str, "") { return arg->moveToFbString() == str; } TEST(OutputBufferingHandlerTest, Basic) { MockHandler mockHandler; + EXPECT_CALL(mockHandler, attachPipeline(_)); ChannelPipeline, ChannelHandlerPtr, OutputBufferingHandler> @@ -52,6 +42,7 @@ TEST(OutputBufferingHandlerTest, Basic) { EventBase eb; auto socket = AsyncSocket::newSocket(&eb); + EXPECT_CALL(mockHandler, attachTransport(_)); pipeline.attachTransport(socket); // Buffering should prevent writes until the EB loops, and the writes should @@ -64,4 +55,5 @@ TEST(OutputBufferingHandlerTest, Basic) { eb.loopOnce(); EXPECT_TRUE(f1.isReady()); EXPECT_TRUE(f2.isReady()); + EXPECT_CALL(mockHandler, detachPipeline(_)); } -- 2.34.1