Prefer to nest helper functions in coro tests

Summary: [Folly] Prefer to nest helper functions in coro tests within the specific tests which use them.

Reviewed By: ispeters, Orvid

Differential Revision: D20324066

fbshipit-source-id: 4134145c406f2d1fdeaddf9a7d244c1bd25ec60b

folly/experimental/coro/test/CoroTest.cpp

@@ -39,26 +39,11 @@
 
 using namespace folly;
 
-struct S {
-  int x = 42;
-};
-
-coro::Task<S> taskS() {
-  co_return {};
-}
-
-coro::Task<int> task42() {
-  co_return 42;
-}
-
-SemiFuture<int> semifuture_task42() {
-  return task42().semi();
-}
-
 class CoroTest : public testing::Test {};
 
 TEST_F(CoroTest, Basic) {
   ManualExecutor executor;
+  auto task42 = []() -> coro::Task<int> { co_return 42; };
   auto future = task42().scheduleOn(&executor).start();
 
   EXPECT_FALSE(future.isReady());
@@ -71,7 +56,8 @@ TEST_F(CoroTest, Basic) {
 
 TEST_F(CoroTest, BasicSemiFuture) {
   ManualExecutor executor;
-  auto future = semifuture_task42().via(&executor);
+  auto task42 = []() -> coro::Task<int> { co_return 42; };
+  auto future = task42().semi().via(&executor);
 
   EXPECT_FALSE(future.isReady());
 
@@ -84,6 +70,7 @@ TEST_F(CoroTest, BasicSemiFuture) {
 TEST_F(CoroTest, BasicFuture) {
   ManualExecutor executor;
 
+  auto task42 = []() -> coro::Task<int> { co_return 42; };
   auto future = task42().scheduleOn(&executor).start();
 
   EXPECT_FALSE(future.isReady());
@@ -92,6 +79,7 @@ TEST_F(CoroTest, BasicFuture) {
 }
 
 coro::Task<void> taskVoid() {
+  auto task42 = []() -> coro::Task<int> { co_return 42; };
   (void)co_await task42();
   co_return;
 }
@@ -147,18 +135,19 @@ TEST_F(CoroTest, ExecutorKeepAlive) {
   EXPECT_TRUE(future.isReady());
 }
 
-struct CountingExecutor : public ManualExecutor {
+TEST_F(CoroTest, ExecutorKeepAliveDummy) {
+  struct CountingExecutor : public ManualExecutor {
     bool keepAliveAcquire() noexcept override {
       ++keepAliveCounter;
       return true;
     }
-
     void keepAliveRelease() noexcept override { --keepAliveCounter; }
 
     size_t keepAliveCounter{0};
-};
+  };
 
-coro::Task<void> executorRec(int depth) {
+  struct ExecutorRec {
+    static coro::Task<void> go(int depth) {
       if (depth == 0) {
         co_return;
       }
@@ -170,21 +159,21 @@ coro::Task<void> executorRec(int depth) {
       // Note, extra keep-alives are being kept by the Futures.
       EXPECT_EQ(3, executor->keepAliveCounter);
 
-  co_await executorRec(depth - 1);
-}
+      co_await go(depth - 1);
+    }
+  };
 
-TEST_F(CoroTest, ExecutorKeepAliveDummy) {
   CountingExecutor executor;
-  executorRec(42).scheduleOn(&executor).start().via(&executor).getVia(
+  ExecutorRec::go(42).scheduleOn(&executor).start().via(&executor).getVia(
       &executor);
 }
 
-coro::Task<int> taskException() {
+TEST_F(CoroTest, FutureThrow) {
+  auto taskException = []() -> coro::Task<int> {
     throw std::runtime_error("Test exception");
     co_return 42;
-}
+  };
 
-TEST_F(CoroTest, FutureThrow) {
   ManualExecutor executor;
   auto future = taskException().scheduleOn(&executor).start();
 
@@ -213,14 +202,15 @@ TEST_F(CoroTest, LargeStack) {
   EXPECT_EQ(50000, coro::blockingWait(std::move(task)));
 }
 
-coro::Task<void> taskThreadNested(std::thread::id threadId) {
+TEST_F(CoroTest, NestedThreads) {
+  auto taskThreadNested = [](std::thread::id threadId) -> coro::Task<void> {
     EXPECT_EQ(threadId, std::this_thread::get_id());
     (void)co_await coro::sleep(std::chrono::seconds{1});
     EXPECT_EQ(threadId, std::this_thread::get_id());
     co_return;
-}
+  };
 
-coro::Task<int> taskThread() {
+  auto taskThread = [&]() -> coro::Task<int> {
     auto threadId = std::this_thread::get_id();
 
     // BUG: Under @mode/clang-opt builds this object is placed on the coroutine
@@ -236,33 +226,33 @@ coro::Task<int> taskThread() {
     EXPECT_EQ(threadId, std::this_thread::get_id());
 
     co_return 42;
-}
+  };
 
-TEST_F(CoroTest, NestedThreads) {
   ScopedEventBaseThread evbThread;
   auto task = taskThread().scheduleOn(evbThread.getEventBase());
   EXPECT_EQ(42, coro::blockingWait(std::move(task)));
 }
 
-coro::Task<int> taskGetCurrentExecutor(Executor* executor) {
+TEST_F(CoroTest, CurrentExecutor) {
+  auto taskGetCurrentExecutor = [](Executor* executor) -> coro::Task<int> {
     auto current = co_await coro::co_current_executor;
     EXPECT_EQ(executor, current);
+    auto task42 = []() -> coro::Task<int> { co_return 42; };
     co_return co_await task42().scheduleOn(current);
-}
+  };
 
-TEST_F(CoroTest, CurrentExecutor) {
   ScopedEventBaseThread evbThread;
   auto task = taskGetCurrentExecutor(evbThread.getEventBase())
                   .scheduleOn(evbThread.getEventBase());
   EXPECT_EQ(42, coro::blockingWait(std::move(task)));
 }
 
-coro::Task<void> taskTimedWaitFuture() {
+TEST_F(CoroTest, TimedWaitFuture) {
+  auto taskTimedWaitFuture = []() -> coro::Task<void> {
     auto ex = co_await coro::co_current_executor;
-  auto fastFuture =
-      futures::sleep(std::chrono::milliseconds{50}).via(ex).thenValue([](Unit) {
-        return 42;
-      });
+    auto fastFuture = futures::sleep(std::chrono::milliseconds{50})
+                          .via(ex)
+                          .thenValue([](Unit) { return 42; });
     auto fastResult = co_await coro::timed_wait(
         std::move(fastFuture), std::chrono::milliseconds{100});
     EXPECT_TRUE(fastResult);
@@ -273,9 +263,9 @@ coro::Task<void> taskTimedWaitFuture() {
     };
 
     auto throwingFuture =
-      futures::sleep(std::chrono::milliseconds{50}).via(ex).thenValue([](Unit) {
-        throw ExpectedException();
-      });
+        futures::sleep(std::chrono::milliseconds{50})
+            .via(ex)
+            .thenValue([](Unit) { throw ExpectedException(); });
     EXPECT_THROW(
         (void)co_await coro::timed_wait(
             std::move(throwingFuture), std::chrono::milliseconds{100}),
@@ -299,13 +289,13 @@ coro::Task<void> taskTimedWaitFuture() {
     (void)co_await std::move(lifetimeFuture);
 
     co_return;
-}
+  };
 
-TEST_F(CoroTest, TimedWaitFuture) {
   coro::blockingWait(taskTimedWaitFuture());
 }
 
-coro::Task<void> taskTimedWaitTask() {
+TEST_F(CoroTest, TimedWaitTask) {
+  auto taskTimedWaitTask = []() -> coro::Task<void> {
     auto fastTask = []() -> coro::Task<int> {
       co_await coro::sleep(std::chrono::milliseconds{50});
       co_return 42;
@@ -337,9 +327,8 @@ coro::Task<void> taskTimedWaitTask() {
     EXPECT_FALSE(slowResult);
 
     co_return;
-}
+  };
 
-TEST_F(CoroTest, TimedWaitTask) {
   coro::blockingWait(taskTimedWaitTask());
 }
 
@@ -367,6 +356,8 @@ TEST_F(CoroTest, TimedWaitNonCopyable) {
           }()));
 }
 
+namespace {
+
 template <int value>
 struct AwaitableInt {
   bool await_ready() const { return true; }
@@ -384,36 +375,39 @@ AwaitableInt<42> operator co_await(const AwaitableWithOperator&) {
   return {};
 }
 
-coro::Task<int> taskAwaitableWithOperator() {
-  co_return co_await AwaitableWithOperator();
-}
-
-TEST_F(CoroTest, AwaitableWithOperator) {
-  EXPECT_EQ(42, coro::blockingWait(taskAwaitableWithOperator()));
-}
-
 struct AwaitableWithMemberOperator {
   AwaitableInt<42> operator co_await() { return {}; }
 };
 
-AwaitableInt<24> operator co_await(const AwaitableWithMemberOperator&) {
+FOLLY_MAYBE_UNUSED AwaitableInt<24> operator co_await(
+    const AwaitableWithMemberOperator&) {
   return {};
 }
 
-coro::Task<int> taskAwaitableWithMemberOperator() {
-  co_return co_await AwaitableWithMemberOperator();
+} // namespace
+
+TEST_F(CoroTest, AwaitableWithOperator) {
+  auto taskAwaitableWithOperator = []() -> coro::Task<int> {
+    co_return co_await AwaitableWithOperator();
+  };
+
+  EXPECT_EQ(42, coro::blockingWait(taskAwaitableWithOperator()));
 }
 
 TEST_F(CoroTest, AwaitableWithMemberOperator) {
+  auto taskAwaitableWithMemberOperator = []() -> coro::Task<int> {
+    co_return co_await AwaitableWithMemberOperator();
+  };
+
   EXPECT_EQ(42, coro::blockingWait(taskAwaitableWithMemberOperator()));
 }
 
-coro::Task<int> taskBaton(fibers::Baton& baton) {
+TEST_F(CoroTest, Baton) {
+  auto taskBaton = [](fibers::Baton& baton) -> coro::Task<int> {
     co_await baton;
     co_return 42;
-}
+  };
 
-TEST_F(CoroTest, Baton) {
   ManualExecutor executor;
   fibers::Baton baton;
   auto future = taskBaton(baton).scheduleOn(&executor).start();
@@ -431,16 +425,19 @@ TEST_F(CoroTest, Baton) {
   EXPECT_EQ(42, std::move(future).get());
 }
 
-template <class Type>
-coro::Task<Type> taskFuture(Type value) {
-  co_return co_await folly::makeFuture<Type>(std::move(value));
-}
-
 TEST_F(CoroTest, FulfilledFuture) {
+  auto taskFuture = [](auto value) -> coro::Task<decltype(value)> {
+    co_return co_await folly::makeFuture(std::move(value));
+  };
+
   EXPECT_EQ(42, coro::blockingWait(taskFuture(42)));
 }
 
 TEST_F(CoroTest, MoveOnlyReturn) {
+  auto taskFuture = [](auto value) -> coro::Task<decltype(value)> {
+    co_return co_await folly::makeFuture(std::move(value));
+  };
+
   EXPECT_EQ(42, *coro::blockingWait(taskFuture(std::make_unique<int>(42))));
 }
 
@@ -588,6 +585,12 @@ TEST_F(CoroTest, CancelOneSemaphoreWaitDoesNotAffectOthers) {
 
 TEST_F(CoroTest, FutureTry) {
   folly::coro::blockingWait([]() -> folly::coro::Task<void> {
+    auto task42 = []() -> coro::Task<int> { co_return 42; };
+    auto taskException = []() -> coro::Task<int> {
+      throw std::runtime_error("Test exception");
+      co_return 42;
+    };
+
     {
       auto result = co_await folly::coro::co_awaitTry(task42().semi());
       EXPECT_TRUE(result.hasValue());
@@ -643,6 +646,12 @@ TEST_F(CoroTest, CancellableSleep) {
 
 TEST_F(CoroTest, DefaultConstructible) {
   coro::blockingWait([]() -> coro::Task<void> {
+    struct S {
+      int x = 42;
+    };
+
+    auto taskS = []() -> coro::Task<S> { co_return {}; };
+
     auto s = co_await taskS();
     EXPECT_EQ(42, s.x);
   }());

folly/experimental/coro/test/InlineTaskTest.cpp

@@ -107,28 +107,13 @@ TEST_F(InlineTaskTest, SimpleRefTask) {
   EXPECT_EQ(&hasRun, &result);
 }
 
-struct MoveOnlyType {
-  int value_;
-
-  explicit MoveOnlyType(int value) noexcept : value_(value) {}
-
-  MoveOnlyType(MoveOnlyType&& other) noexcept
-      : value_(std::exchange(other.value_, -1)) {}
-
-  MoveOnlyType& operator=(MoveOnlyType&& other) noexcept {
-    value_ = std::exchange(other.value_, -1);
-    return *this;
-  }
-
-  ~MoveOnlyType() { value_ = -2; }
-};
-
-struct TypeWithImplicitSingleValueConstructor {
+TEST_F(InlineTaskTest, ReturnValueWithInitializerListSyntax) {
+  struct TypeWithImplicitSingleValueConstructor {
     float value_;
-  /* implicit */ TypeWithImplicitSingleValueConstructor(float x) : value_(x) {}
-};
+    /* implicit */ TypeWithImplicitSingleValueConstructor(float x)
+        : value_(x) {}
+  };
 
-TEST_F(InlineTaskTest, ReturnValueWithInitializerListSyntax) {
   auto f = []() -> InlineTask<TypeWithImplicitSingleValueConstructor> {
     co_return {1.23f};
   };
@@ -137,15 +122,15 @@ TEST_F(InlineTaskTest, ReturnValueWithInitializerListSyntax) {
   EXPECT_EQ(1.23f, result.value_);
 }
 
-struct TypeWithImplicitMultiValueConstructor {
+TEST_F(InlineTaskTest, ReturnValueWithInitializerListSyntax2) {
+  struct TypeWithImplicitMultiValueConstructor {
     std::string s_;
     float x_;
     /* implicit */ TypeWithImplicitMultiValueConstructor(
         std::string s, float x) noexcept
         : s_(s), x_(x) {}
-};
+  };
 
-TEST_F(InlineTaskTest, ReturnValueWithInitializerListSyntax2) {
   auto f = []() -> InlineTask<TypeWithImplicitMultiValueConstructor> {
     co_return {"hello", 3.1415f};
   };
@@ -155,6 +140,26 @@ TEST_F(InlineTaskTest, ReturnValueWithInitializerListSyntax2) {
   EXPECT_EQ(3.1415f, result.x_);
 }
 
+namespace {
+
+struct MoveOnlyType {
+  int value_;
+
+  explicit MoveOnlyType(int value) noexcept : value_(value) {}
+
+  MoveOnlyType(MoveOnlyType&& other) noexcept
+      : value_(std::exchange(other.value_, -1)) {}
+
+  FOLLY_MAYBE_UNUSED MoveOnlyType& operator=(MoveOnlyType&& other) noexcept {
+    value_ = std::exchange(other.value_, -1);
+    return *this;
+  }
+
+  ~MoveOnlyType() { value_ = -2; }
+};
+
+} // namespace
+
 TEST_F(InlineTaskTest, TaskOfMoveOnlyType) {
   auto f = []() -> InlineTask<MoveOnlyType> { co_return MoveOnlyType{42}; };
 
@@ -191,9 +196,9 @@ TEST_F(InlineTaskTest, ReturnLvalueReference) {
   EXPECT_EQ(&value, &x);
 }
 
-struct MyException : std::exception {};
-
 TEST_F(InlineTaskTest, ExceptionsPropagateFromVoidTask) {
+  struct MyException : std::exception {};
+
   auto f = []() -> InlineTask<void> {
     co_await std::experimental::suspend_never{};
     throw MyException{};
@@ -202,6 +207,8 @@ TEST_F(InlineTaskTest, ExceptionsPropagateFromVoidTask) {
 }
 
 TEST_F(InlineTaskTest, ExceptionsPropagateFromValueTask) {
+  struct MyException : std::exception {};
+
   auto f = []() -> InlineTask<int> {
     co_await std::experimental::suspend_never{};
     throw MyException{};
@@ -210,6 +217,8 @@ TEST_F(InlineTaskTest, ExceptionsPropagateFromValueTask) {
 }
 
 TEST_F(InlineTaskTest, ExceptionsPropagateFromRefTask) {
+  struct MyException : std::exception {};
+
   auto f = []() -> InlineTask<int&> {
     co_await std::experimental::suspend_never{};
     throw MyException{};
@@ -217,54 +226,64 @@ TEST_F(InlineTaskTest, ExceptionsPropagateFromRefTask) {
   EXPECT_THROW(folly::coro::blockingWait(f()), MyException);
 }
 
-struct ThrowingCopyConstructor {
-  ThrowingCopyConstructor() noexcept = default;
+TEST_F(InlineTaskTest, ExceptionsPropagateFromReturnValueConstructor) {
+  struct MyException : std::exception {};
+
+  struct ThrowingCopyConstructor {
+    FOLLY_MAYBE_UNUSED ThrowingCopyConstructor() noexcept = default;
 
-  [[noreturn]] ThrowingCopyConstructor(const ThrowingCopyConstructor&) noexcept(
-      false) {
+    [[noreturn]] ThrowingCopyConstructor(
+        const ThrowingCopyConstructor&) noexcept(false) {
       throw MyException{};
     }
 
     ThrowingCopyConstructor& operator=(const ThrowingCopyConstructor&) = delete;
-};
+  };
 
-TEST_F(InlineTaskTest, ExceptionsPropagateFromReturnValueConstructor) {
   auto f = []() -> InlineTask<ThrowingCopyConstructor> { co_return {}; };
   EXPECT_THROW(folly::coro::blockingWait(f()), MyException);
 }
 
-InlineTask<void> recursiveTask(int depth) {
+TEST_F(InlineTaskTest, DeepRecursionDoesntStackOverflow) {
+  struct RecursiveTask {
+    InlineTask<void> operator()(int depth) {
       if (depth > 0) {
-    co_await recursiveTask(depth - 1);
+        co_await operator()(depth - 1);
       }
-}
+    }
+  };
 
-TEST_F(InlineTaskTest, DeepRecursionDoesntStackOverflow) {
-  folly::coro::blockingWait(recursiveTask(500000));
+  folly::coro::blockingWait(RecursiveTask{}(500000));
 }
 
-InlineTask<int> recursiveValueTask(int depth) {
+TEST_F(InlineTaskTest, DeepRecursionOfValueTaskDoesntStackOverflow) {
+  struct RecursiveValueTask {
+    InlineTask<int> operator()(int depth) {
       if (depth > 0) {
-    co_return co_await recursiveValueTask(depth - 1) + 1;
+        co_return co_await operator()(depth - 1) + 1;
       }
       co_return 0;
-}
+    }
+  };
 
-TEST_F(InlineTaskTest, DeepRecursionOfValueTaskDoesntStackOverflow) {
-  EXPECT_EQ(500000, folly::coro::blockingWait(recursiveValueTask(500000)));
+  EXPECT_EQ(500000, folly::coro::blockingWait(RecursiveValueTask{}(500000)));
 }
 
-InlineTask<void> recursiveThrowingTask(int depth) {
+TEST_F(InlineTaskTest, DeepRecursionOfExceptions) {
+  struct MyException : std::exception {};
+
+  struct RecursiveThrowingTask {
+    static InlineTask<void> go(int depth) {
       if (depth > 0) {
-    co_await recursiveThrowingTask(depth - 1);
+        co_await go(depth - 1);
       }
 
       throw MyException{};
-}
+    }
+  };
 
-TEST_F(InlineTaskTest, DeepRecursionOfExceptions) {
   EXPECT_THROW(
-      folly::coro::blockingWait(recursiveThrowingTask(50000)), MyException);
+      folly::coro::blockingWait(RecursiveThrowingTask::go(50000)), MyException);
 }
 
 #endif

folly/experimental/coro/test/TaskTest.cpp

@@ -362,9 +362,10 @@ TEST_F(TaskTest, FutureRoundtrip) {
       std::runtime_error);
 }
 
-// NOTE: This function is unused.
+namespace {
+
 // We just want to make sure this compiles without errors or warnings.
-folly::coro::Task<void>
+FOLLY_MAYBE_UNUSED folly::coro::Task<void>
 checkAwaitingFutureOfUnitDoesntWarnAboutDiscardedResult() {
   co_await folly::makeSemiFuture();
 
@@ -372,11 +373,13 @@ checkAwaitingFutureOfUnitDoesntWarnAboutDiscardedResult() {
   co_await folly::futures::sleep(1ms);
 }
 
-folly::coro::Task<int&> returnIntRef(int& value) {
-  co_return value;
-}
+} // namespace
 
 TEST_F(TaskTest, TaskOfLvalueReference) {
+  auto returnIntRef = [](int& value) -> folly::coro::Task<int&> {
+    co_return value;
+  };
+
   int value = 123;
   auto&& result = folly::coro::blockingWait(returnIntRef(value));
   static_assert(std::is_same_v<decltype(result), int&>);
@@ -385,6 +388,10 @@ TEST_F(TaskTest, TaskOfLvalueReference) {
 
 TEST_F(TaskTest, TaskOfLvalueReferenceAsTry) {
   folly::coro::blockingWait([]() -> folly::coro::Task<void> {
+    auto returnIntRef = [](int& value) -> folly::coro::Task<int&> {
+      co_return value;
+    };
+
     int value = 123;
     auto&& result = co_await co_awaitTry(returnIntRef(value));
     CHECK(result.hasValue());