Replace RequestContext::create with RequestContextScopeGuard in tests

Summary: RequestContextScopeGuard should be preferred to RequestContext::create because it makes sure that RequestContext is cleared afterwards - need to create more examples of this in the codebase, migrating unit tests should be safe

Reviewed By: interwq

Differential Revision: D3489969

fbshipit-source-id: 202fec93116db3d435c108fafecad26a4ed9b603

diff --git a/folly/fibers/test/FibersTest.cpp b/folly/fibers/test/FibersTest.cpp
index e5762af141b9abc1ed87c4868b6e79bc51902ad7..a149c44b8a6e1da2a2589fa28c6e8ac39d0ae961 100644 (file)
--- a/folly/fibers/test/FibersTest.cpp
+++ b/folly/fibers/test/FibersTest.cpp
@@ -1267,79 +1267,86 @@ TEST(FiberManager, RequestContext) {
   folly::fibers::Baton baton3;
   folly::fibers::Baton baton4;
 
-  folly::RequestContext::create();
-  auto rcontext1 = folly::RequestContext::get();
-  fm.addTask([&]() {
-    EXPECT_EQ(rcontext1, folly::RequestContext::get());
-    baton1.wait([&]() { EXPECT_EQ(rcontext1, folly::RequestContext::get()); });
-    EXPECT_EQ(rcontext1, folly::RequestContext::get());
-    runInMainContext(
-        [&]() { EXPECT_EQ(rcontext1, folly::RequestContext::get()); });
-    checkRun1 = true;
-  });
-
-  folly::RequestContext::create();
-  auto rcontext2 = folly::RequestContext::get();
-  fm.addTaskRemote([&]() {
-    EXPECT_EQ(rcontext2, folly::RequestContext::get());
-    baton2.wait();
-    EXPECT_EQ(rcontext2, folly::RequestContext::get());
-    checkRun2 = true;
-  });
-
-  folly::RequestContext::create();
-  auto rcontext3 = folly::RequestContext::get();
-  fm.addTaskFinally(
-      [&]() {
-        EXPECT_EQ(rcontext3, folly::RequestContext::get());
-        baton3.wait();
-        EXPECT_EQ(rcontext3, folly::RequestContext::get());
-
-        return folly::Unit();
-      },
-      [&](Try<folly::Unit>&& /* t */) {
-        EXPECT_EQ(rcontext3, folly::RequestContext::get());
-        checkRun3 = true;
-      });
-
-  folly::RequestContext::setContext(nullptr);
-  fm.addTask([&]() {
-    folly::RequestContext::create();
-    auto rcontext4 = folly::RequestContext::get();
-    baton4.wait();
-    EXPECT_EQ(rcontext4, folly::RequestContext::get());
-    checkRun4 = true;
-  });
-
-  folly::RequestContext::create();
-  auto rcontext = folly::RequestContext::get();
+  {
+    folly::RequestContextScopeGuard rctx;
+    auto rcontext1 = folly::RequestContext::get();
+    fm.addTask([&]() {
+      EXPECT_EQ(rcontext1, folly::RequestContext::get());
+      baton1.wait(
+          [&]() { EXPECT_EQ(rcontext1, folly::RequestContext::get()); });
+      EXPECT_EQ(rcontext1, folly::RequestContext::get());
+      runInMainContext(
+          [&]() { EXPECT_EQ(rcontext1, folly::RequestContext::get()); });
+      checkRun1 = true;
+    });
+  }
+  {
+    folly::RequestContextScopeGuard rctx;
+    auto rcontext2 = folly::RequestContext::get();
+    fm.addTaskRemote([&]() {
+      EXPECT_EQ(rcontext2, folly::RequestContext::get());
+      baton2.wait();
+      EXPECT_EQ(rcontext2, folly::RequestContext::get());
+      checkRun2 = true;
+    });
+  }
+  {
+    folly::RequestContextScopeGuard rctx;
+    auto rcontext3 = folly::RequestContext::get();
+    fm.addTaskFinally(
+        [&]() {
+          EXPECT_EQ(rcontext3, folly::RequestContext::get());
+          baton3.wait();
+          EXPECT_EQ(rcontext3, folly::RequestContext::get());
+
+          return folly::Unit();
+        },
+        [&](Try<folly::Unit>&& /* t */) {
+          EXPECT_EQ(rcontext3, folly::RequestContext::get());
+          checkRun3 = true;
+        });
+  }
+  {
+    folly::RequestContext::setContext(nullptr);
+    fm.addTask([&]() {
+      folly::RequestContextScopeGuard rctx;
+      auto rcontext4 = folly::RequestContext::get();
+      baton4.wait();
+      EXPECT_EQ(rcontext4, folly::RequestContext::get());
+      checkRun4 = true;
+    });
+  }
+  {
+    folly::RequestContextScopeGuard rctx;
+    auto rcontext = folly::RequestContext::get();
 
-  fm.loopUntilNoReady();
-  EXPECT_EQ(rcontext, folly::RequestContext::get());
+    fm.loopUntilNoReady();
+    EXPECT_EQ(rcontext, folly::RequestContext::get());
 
-  baton1.post();
-  EXPECT_EQ(rcontext, folly::RequestContext::get());
-  fm.loopUntilNoReady();
-  EXPECT_TRUE(checkRun1);
-  EXPECT_EQ(rcontext, folly::RequestContext::get());
+    baton1.post();
+    EXPECT_EQ(rcontext, folly::RequestContext::get());
+    fm.loopUntilNoReady();
+    EXPECT_TRUE(checkRun1);
+    EXPECT_EQ(rcontext, folly::RequestContext::get());
 
-  baton2.post();
-  EXPECT_EQ(rcontext, folly::RequestContext::get());
-  fm.loopUntilNoReady();
-  EXPECT_TRUE(checkRun2);
-  EXPECT_EQ(rcontext, folly::RequestContext::get());
+    baton2.post();
+    EXPECT_EQ(rcontext, folly::RequestContext::get());
+    fm.loopUntilNoReady();
+    EXPECT_TRUE(checkRun2);
+    EXPECT_EQ(rcontext, folly::RequestContext::get());
 
-  baton3.post();
-  EXPECT_EQ(rcontext, folly::RequestContext::get());
-  fm.loopUntilNoReady();
-  EXPECT_TRUE(checkRun3);
-  EXPECT_EQ(rcontext, folly::RequestContext::get());
+    baton3.post();
+    EXPECT_EQ(rcontext, folly::RequestContext::get());
+    fm.loopUntilNoReady();
+    EXPECT_TRUE(checkRun3);
+    EXPECT_EQ(rcontext, folly::RequestContext::get());
 
-  baton4.post();
-  EXPECT_EQ(rcontext, folly::RequestContext::get());
-  fm.loopUntilNoReady();
-  EXPECT_TRUE(checkRun4);
-  EXPECT_EQ(rcontext, folly::RequestContext::get());
+    baton4.post();
+    EXPECT_EQ(rcontext, folly::RequestContext::get());
+    fm.loopUntilNoReady();
+    EXPECT_TRUE(checkRun4);
+    EXPECT_EQ(rcontext, folly::RequestContext::get());
+  }
 }
 
 TEST(FiberManager, resizePeriodically) {

diff --git a/folly/futures/test/ContextTest.cpp b/folly/futures/test/ContextTest.cpp
index 357739f9878b2f47da1576cac7cf42d737658744..e5946b2657218ace5e8dffd880ec73510c2b6498 100644 (file)
--- a/folly/futures/test/ContextTest.cpp
+++ b/folly/futures/test/ContextTest.cpp
@@ -30,7 +30,7 @@ class TestData : public RequestData {
 TEST(Context, basic) {
 
   // Start a new context
-  RequestContext::create();
+  folly::RequestContextScopeGuard rctx;
 
   EXPECT_EQ(nullptr, RequestContext::get()->getContextData("test"));
 

diff --git a/folly/futures/test/FutureTest.cpp b/folly/futures/test/FutureTest.cpp
index 18fca468e2ce7a56252316f8c1cee4c36b868b21..a025dba9771b70c2609bc37143b94f4f60fa90c3 100644 (file)
--- a/folly/futures/test/FutureTest.cpp
+++ b/folly/futures/test/FutureTest.cpp
@@ -822,30 +822,32 @@ TEST(Future, RequestContext) {
     bool value;
   };
 
-  NewThreadExecutor e;
-  RequestContext::create();
-  RequestContext::get()->setContextData("key",
-      folly::make_unique<MyRequestData>(true));
-  auto checker = [](int lineno) {
-    return [lineno](Try<int>&& /* t */) {
-      auto d = static_cast<MyRequestData*>(
-        RequestContext::get()->getContextData("key"));
-      EXPECT_TRUE(d && d->value) << "on line " << lineno;
+  Promise<int> p1, p2;
+  {
+    NewThreadExecutor e;
+    folly::RequestContextScopeGuard rctx;
+    RequestContext::get()->setContextData(
+        "key", folly::make_unique<MyRequestData>(true));
+    auto checker = [](int lineno) {
+      return [lineno](Try<int>&& /* t */) {
+        auto d = static_cast<MyRequestData*>(
+            RequestContext::get()->getContextData("key"));
+        EXPECT_TRUE(d && d->value) << "on line " << lineno;
+      };
     };
-  };
 
-  makeFuture(1).via(&e).then(checker(__LINE__));
+    makeFuture(1).via(&e).then(checker(__LINE__));
 
-  e.setHandlesPriorities();
-  makeFuture(2).via(&e).then(checker(__LINE__));
+    e.setHandlesPriorities();
+    makeFuture(2).via(&e).then(checker(__LINE__));
 
-  Promise<int> p1, p2;
-  p1.getFuture().then(checker(__LINE__));
-
-  e.setThrowsOnAdd();
-  p2.getFuture().via(&e).then(checker(__LINE__));
+    p1.getFuture().then(checker(__LINE__));
 
-  RequestContext::create();
+    e.setThrowsOnAdd();
+    p2.getFuture().via(&e).then(checker(__LINE__));
+  }
+  // Assert that no RequestContext is set
+  EXPECT_FALSE(RequestContext::saveContext());
   p1.setValue(3);
   p2.setValue(4);
 }