From f0fa4f94a5400fe5d21cf5bf7570129916bf9787 Mon Sep 17 00:00:00 2001
From: Nat Goodspeed <nat@lindenlab.com>
Date: Mon, 12 Aug 2019 08:26:51 -0400
Subject: [PATCH] DRTVWR-493: Make catch_llerrs() a member of WrapLLErrs.

---
 indra/llcommon/tests/lleventcoro_test.cpp     |  4 +--
 indra/llcommon/tests/lleventfilter_test.cpp   |  2 +-
 .../llcommon/tests/llinstancetracker_test.cpp |  6 ++--
 indra/llcommon/tests/wrapllerrs.h             | 32 +++++++++++++++----
 indra/llmessage/tests/llareslistener_test.cpp |  8 ++---
 indra/newview/tests/llxmlrpclistener_test.cpp |  4 +--
 6 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/indra/llcommon/tests/lleventcoro_test.cpp b/indra/llcommon/tests/lleventcoro_test.cpp
index ea198849cd6..fa02d2bb1a0 100644
--- a/indra/llcommon/tests/lleventcoro_test.cpp
+++ b/indra/llcommon/tests/lleventcoro_test.cpp
@@ -506,7 +506,7 @@ namespace tut
             replyName = waiter.getName0();
             errorName = waiter.getName1();
             WrapLLErrs capture;
-            threw = catch_llerrs([&waiter, &debug](){
+            threw = capture.catch_llerrs([&waiter, &debug](){
                     result = waiter.suspendWithLog();
                     debug("no exception");
                 });
@@ -756,7 +756,7 @@ namespace tut
         {
             LLCoroEventPumps waiter;
             WrapLLErrs capture;
-            threw = catch_llerrs(
+            threw = capture.catch_llerrs(
                 [&waiter, &debug](){
                     result = waiter.postAndSuspendWithLog(
                         LLSDMap("value", 31)("fail", LLSD()),
diff --git a/indra/llcommon/tests/lleventfilter_test.cpp b/indra/llcommon/tests/lleventfilter_test.cpp
index a23abf453ee..1875013794f 100644
--- a/indra/llcommon/tests/lleventfilter_test.cpp
+++ b/indra/llcommon/tests/lleventfilter_test.cpp
@@ -350,7 +350,7 @@ namespace tut
         // Now let the timer expire.
         filter.forceTimeout();
         // Notice the timeout.
-        std::string threw = catch_llerrs([this](){
+        std::string threw = capture.catch_llerrs([this](){
                 mainloop.post(17);
             });
         ensure_contains("errorAfter() timeout exception", threw, "timeout");
diff --git a/indra/llcommon/tests/llinstancetracker_test.cpp b/indra/llcommon/tests/llinstancetracker_test.cpp
index 0fe65f08310..d94fc0c56d0 100644
--- a/indra/llcommon/tests/llinstancetracker_test.cpp
+++ b/indra/llcommon/tests/llinstancetracker_test.cpp
@@ -198,7 +198,7 @@ namespace tut
         {
             WrapLLErrs wrapper;
             Keyed::instance_iter i(Keyed::beginInstances());
-            what = catch_llerrs([&keyed](){
+            what = wrapper.catch_llerrs([&keyed](){
                     delete keyed;
                 });
         }
@@ -214,7 +214,7 @@ namespace tut
         {
             WrapLLErrs wrapper;
             Keyed::key_iter i(Keyed::beginKeys());
-            what = catch_llerrs([&keyed](){
+            what = wrapper.catch_llerrs([&keyed](){
                     delete keyed;
                 });
         }
@@ -230,7 +230,7 @@ namespace tut
         {
             WrapLLErrs wrapper;
             Unkeyed::instance_iter i(Unkeyed::beginInstances());
-            what = catch_llerrs([&unkeyed](){
+            what = wrapper.catch_llerrs([&unkeyed](){
                     delete unkeyed;
                 });
         }
diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h
index 3fb79f6b5d8..b07d5afbd8f 100644
--- a/indra/llcommon/tests/wrapllerrs.h
+++ b/indra/llcommon/tests/wrapllerrs.h
@@ -82,18 +82,36 @@ struct WrapLLErrs
         LLTHROW(FatalException(message));
     }
 
+    /// Convenience wrapper for catch_what<FatalException>()
+    //
+    // The implementation makes it clear that this function need not be a
+    // member; it could easily be a free function. It is a member because it
+    // makes no sense to attempt to catch FatalException unless there is a
+    // WrapLLErrs instance in scope. Without a live WrapLLErrs instance, any
+    // LL_ERRS() reached by code within 'func' would terminate the test
+    // program instead of throwing FatalException.
+    //
+    // We were tempted to introduce a free function, likewise accepting
+    // arbitrary 'func', that would instantiate WrapLLErrs and then call
+    // catch_llerrs() on that instance. We decided against it, for this
+    // reason: on extending a test function containing a single call to that
+    // free function, a maintainer would most likely make additional calls to
+    // that free function, instead of switching to an explicit WrapLLErrs
+    // declaration with several calls to its catch_llerrs() member function.
+    // Even a construct such as WrapLLErrs().catch_llerrs(...) would make the
+    // object declaration more visible; it's not unreasonable to expect a
+    // maintainer to extend that by naming and reusing the WrapLLErrs instance.
+    template <typename FUNC>
+    std::string catch_llerrs(FUNC func)
+    {
+        return catch_what<FatalException>(func);
+    }
+
     std::string error;
     LLError::SettingsStoragePtr mPriorErrorSettings;
     LLError::FatalFunction mPriorFatal;
 };
 
-/// Convenience wrapper for catch_what<WrapLLErrs::FatalException>()
-template <typename FUNC>
-std::string catch_llerrs(FUNC func)
-{
-    return catch_what<WrapLLErrs::FatalException>(func);
-}
-
 /**
  * Capture log messages. This is adapted (simplified) from the one in
  * llerror_test.cpp.
diff --git a/indra/llmessage/tests/llareslistener_test.cpp b/indra/llmessage/tests/llareslistener_test.cpp
index 8e5bcd326ca..254185cbd09 100644
--- a/indra/llmessage/tests/llareslistener_test.cpp
+++ b/indra/llmessage/tests/llareslistener_test.cpp
@@ -138,7 +138,7 @@ namespace tut
         WrapLLErrs capture;
         LLSD request;
         request["op"] = "foo";
-        std::string threw = catch_llerrs([&request](){
+        std::string threw = capture.catch_llerrs([&request](){
                 LLEventPumps::instance().obtain("LLAres").post(request);
             });
         ensure_contains("LLAresListener bad op", threw, "bad");
@@ -151,7 +151,7 @@ namespace tut
         WrapLLErrs capture;
         LLSD request;
         request["op"] = "rewriteURI";
-        std::string threw = catch_llerrs([&request](){
+        std::string threw = capture.catch_llerrs([&request](){
                 LLEventPumps::instance().obtain("LLAres").post(request);
             });
         ensure_contains("LLAresListener bad req", threw, "missing");
@@ -167,7 +167,7 @@ namespace tut
         LLSD request;
         request["op"] = "rewriteURI";
         request["reply"] = "nonexistent";
-        std::string threw = catch_llerrs([&request](){
+        std::string threw = capture.catch_llerrs([&request](){
                 LLEventPumps::instance().obtain("LLAres").post(request);
             });
         ensure_contains("LLAresListener bad req", threw, "missing");
@@ -183,7 +183,7 @@ namespace tut
         LLSD request;
         request["op"] = "rewriteURI";
         request["uri"] = "foo.bar.com";
-        std::string threw = catch_llerrs([&request](){
+        std::string threw = capture.catch_llerrs([&request](){
                 LLEventPumps::instance().obtain("LLAres").post(request);
             });
         ensure_contains("LLAresListener bad req", threw, "missing");
diff --git a/indra/newview/tests/llxmlrpclistener_test.cpp b/indra/newview/tests/llxmlrpclistener_test.cpp
index f9a4ec373a4..dbaae7280c6 100644
--- a/indra/newview/tests/llxmlrpclistener_test.cpp
+++ b/indra/newview/tests/llxmlrpclistener_test.cpp
@@ -88,7 +88,7 @@ namespace tut
         WrapLLErrs capture;
         LLSD request;
         request["uri"] = uri;
-        std::string threw = catch_llerrs([&pumps, &request](){
+        std::string threw = capture.catch_llerrs([&pumps, &request](){
                 pumps.obtain("LLXMLRPCTransaction").post(request);
             });
         ensure_contains("threw exception", threw, "missing params");
@@ -107,7 +107,7 @@ namespace tut
         request["reply"] = "reply";
         LLSD& params(request["params"]);
         params["who"]["specifically"] = "world"; // LLXMLRPCListener only handles scalar params
-        std::string threw = catch_llerrs([&pumps, &request](){
+        std::string threw = capture.catch_llerrs([&pumps, &request](){
                 pumps.obtain("LLXMLRPCTransaction").post(request);
             });
         ensure_contains("threw exception", threw, "unknown type");
-- 
GitLab