From adb3f447b33e42bdb6e4a5a1ac79eebd862dafb4 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 10 Aug 2019 20:33:59 -0400 Subject: DRTVWR-493: Introduce test catch_what(), catch_llerrs() functions. Use them in place of awkward try/catch test boilerplate. --- indra/llcommon/tests/lleventcoro_test.cpp | 33 ++++------ indra/llcommon/tests/lleventdispatcher_test.cpp | 83 +++++++------------------ indra/llcommon/tests/lleventfilter_test.cpp | 12 +--- indra/llcommon/tests/llinstancetracker_test.cpp | 33 +++------- indra/llcommon/tests/lllazy_test.cpp | 13 ++-- indra/llcommon/tests/llleap_test.cpp | 20 +++--- indra/llcommon/tests/llprocess_test.cpp | 16 ++--- indra/llcommon/tests/wrapllerrs.h | 8 +++ indra/llmessage/tests/llareslistener_test.cpp | 48 ++++---------- indra/newview/tests/llxmlrpclistener_test.cpp | 24 ++----- indra/test/catch_and_store_what_in.h | 26 +++++++- indra/test/llevents_tut.cpp | 76 ++++++++++------------ indra/test/test.cpp | 1 - 13 files changed, 146 insertions(+), 247 deletions(-) diff --git a/indra/llcommon/tests/lleventcoro_test.cpp b/indra/llcommon/tests/lleventcoro_test.cpp index a459d17fb8..ea198849cd 100644 --- a/indra/llcommon/tests/lleventcoro_test.cpp +++ b/indra/llcommon/tests/lleventcoro_test.cpp @@ -506,16 +506,10 @@ namespace tut replyName = waiter.getName0(); errorName = waiter.getName1(); WrapLLErrs capture; - try - { - result = waiter.suspendWithLog(); - debug("no exception"); - } - catch (const WrapLLErrs::FatalException& e) - { - debug(STRINGIZE("exception " << e.what())); - threw = e.what(); - } + threw = catch_llerrs([&waiter, &debug](){ + result = waiter.suspendWithLog(); + debug("no exception"); + }); } END } @@ -762,18 +756,13 @@ namespace tut { LLCoroEventPumps waiter; WrapLLErrs capture; - try - { - result = waiter.postAndSuspendWithLog( - LLSDMap("value", 31)("fail", LLSD()), - immediateAPI.getPump(), "reply", "error"); - debug("no exception"); - } - catch (const WrapLLErrs::FatalException& e) - { - debug(STRINGIZE("exception " << e.what())); - threw = e.what(); - } + threw = catch_llerrs( + [&waiter, &debug](){ + result = waiter.postAndSuspendWithLog( + LLSDMap("value", 31)("fail", LLSD()), + immediateAPI.getPump(), "reply", "error"); + debug("no exception"); + }); } END } diff --git a/indra/llcommon/tests/lleventdispatcher_test.cpp b/indra/llcommon/tests/lleventdispatcher_test.cpp index 5a4df81bf1..a181d5c941 100644 --- a/indra/llcommon/tests/lleventdispatcher_test.cpp +++ b/indra/llcommon/tests/lleventdispatcher_test.cpp @@ -22,6 +22,7 @@ #include "llsdutil.h" #include "stringize.h" #include "tests/wrapllerrs.h" +#include "../test/catch_and_store_what_in.h" #include #include @@ -630,16 +631,9 @@ namespace tut void call_exc(const std::string& func, const LLSD& args, const std::string& exc_frag) { - std::string threw; - try - { - work(func, args); - } - catch (const std::runtime_error& e) - { - cout << "*** " << e.what() << '\n'; - threw = e.what(); - } + std::string threw = catch_what([this, &func, &args](){ + work(func, args); + }); ensure_has(threw, exc_frag); } @@ -717,15 +711,9 @@ namespace tut LLSD attempts(LLSDArray(17)(LLSDMap("pi", 3.14)("two", 2))); foreach(LLSD ae, inArray(attempts)) { - std::string threw; - try - { - work.add("freena_err", "freena", freena, ae); - } - catch (const std::exception& e) - { - threw = e.what(); - } + std::string threw = catch_what([this, &ae](){ + work.add("freena_err", "freena", freena, ae); + }); ensure_has(threw, "must be an array"); } } @@ -734,15 +722,9 @@ namespace tut void object::test<2>() { set_test_name("map-style registration with badly-formed defaults"); - std::string threw; - try - { - work.add("freena_err", "freena", freena, LLSDArray("a")("b"), 17); - } - catch (const std::exception& e) - { - threw = e.what(); - } + std::string threw = catch_what([this](){ + work.add("freena_err", "freena", freena, LLSDArray("a")("b"), 17); + }); ensure_has(threw, "must be a map or an array"); } @@ -750,17 +732,11 @@ namespace tut void object::test<3>() { set_test_name("map-style registration with too many array defaults"); - std::string threw; - try - { - work.add("freena_err", "freena", freena, - LLSDArray("a")("b"), - LLSDArray(17)(0.9)("gack")); - } - catch (const std::exception& e) - { - threw = e.what(); - } + std::string threw = catch_what([this](){ + work.add("freena_err", "freena", freena, + LLSDArray("a")("b"), + LLSDArray(17)(0.9)("gack")); + }); ensure_has(threw, "shorter than"); } @@ -768,17 +744,11 @@ namespace tut void object::test<4>() { set_test_name("map-style registration with too many map defaults"); - std::string threw; - try - { - work.add("freena_err", "freena", freena, - LLSDArray("a")("b"), - LLSDMap("b", 17)("foo", 3.14)("bar", "sinister")); - } - catch (const std::exception& e) - { - threw = e.what(); - } + std::string threw = catch_what([this](){ + work.add("freena_err", "freena", freena, + LLSDArray("a")("b"), + LLSDMap("b", 17)("foo", 3.14)("bar", "sinister")); + }); ensure_has(threw, "nonexistent params"); ensure_has(threw, "foo"); ensure_has(threw, "bar"); @@ -1039,16 +1009,9 @@ namespace tut // We don't have a comparable helper function for the one-arg // operator() method, and it's not worth building one just for this // case. Write it out. - std::string threw; - try - { - work(LLSDMap("op", "freek")); - } - catch (const std::runtime_error& e) - { - cout << "*** " << e.what() << "\n"; - threw = e.what(); - } + std::string threw = catch_what([this](){ + work(LLSDMap("op", "freek")); + }); ensure_has(threw, "bad"); ensure_has(threw, "op"); ensure_has(threw, "freek"); diff --git a/indra/llcommon/tests/lleventfilter_test.cpp b/indra/llcommon/tests/lleventfilter_test.cpp index eb98b12ef5..a23abf453e 100644 --- a/indra/llcommon/tests/lleventfilter_test.cpp +++ b/indra/llcommon/tests/lleventfilter_test.cpp @@ -350,15 +350,9 @@ namespace tut // Now let the timer expire. filter.forceTimeout(); // Notice the timeout. - std::string threw; - try - { - mainloop.post(17); - } - catch (const WrapLLErrs::FatalException& e) - { - threw = e.what(); - } + std::string threw = catch_llerrs([this](){ + mainloop.post(17); + }); ensure_contains("errorAfter() timeout exception", threw, "timeout"); // Timing out cancels the timer. Verify that. listener0.reset(0); diff --git a/indra/llcommon/tests/llinstancetracker_test.cpp b/indra/llcommon/tests/llinstancetracker_test.cpp index c7d4b8a06b..0fe65f0831 100644 --- a/indra/llcommon/tests/llinstancetracker_test.cpp +++ b/indra/llcommon/tests/llinstancetracker_test.cpp @@ -198,14 +198,9 @@ namespace tut { WrapLLErrs wrapper; Keyed::instance_iter i(Keyed::beginInstances()); - try - { - delete keyed; - } - catch (const WrapLLErrs::FatalException& e) - { - what = e.what(); - } + what = catch_llerrs([&keyed](){ + delete keyed; + }); } ensure(! what.empty()); } @@ -219,14 +214,9 @@ namespace tut { WrapLLErrs wrapper; Keyed::key_iter i(Keyed::beginKeys()); - try - { - delete keyed; - } - catch (const WrapLLErrs::FatalException& e) - { - what = e.what(); - } + what = catch_llerrs([&keyed](){ + delete keyed; + }); } ensure(! what.empty()); } @@ -240,14 +230,9 @@ namespace tut { WrapLLErrs wrapper; Unkeyed::instance_iter i(Unkeyed::beginInstances()); - try - { - delete unkeyed; - } - catch (const WrapLLErrs::FatalException& e) - { - what = e.what(); - } + what = catch_llerrs([&unkeyed](){ + delete unkeyed; + }); } ensure(! what.empty()); } diff --git a/indra/llcommon/tests/lllazy_test.cpp b/indra/llcommon/tests/lllazy_test.cpp index 32a717f4fc..542306ee22 100644 --- a/indra/llcommon/tests/lllazy_test.cpp +++ b/indra/llcommon/tests/lllazy_test.cpp @@ -38,6 +38,7 @@ #include // other Linden headers #include "../test/lltut.h" +#include "../test/catch_and_store_what_in.h" namespace bll = boost::lambda; @@ -200,15 +201,9 @@ namespace tut void lllazy_object::test<2>() { TestNeedsTesting tnt; - std::string threw; - try - { - tnt.toolate(); - } - catch (const LLLazyCommon::InstanceChange& e) - { - threw = e.what(); - } + std::string threw = catch_what([&tnt](){ + tnt.toolate(); + }); ensure_contains("InstanceChange exception", threw, "replace LLLazy instance"); } diff --git a/indra/llcommon/tests/llleap_test.cpp b/indra/llcommon/tests/llleap_test.cpp index 45648536c4..bf0a74d10d 100644 --- a/indra/llcommon/tests/llleap_test.cpp +++ b/indra/llcommon/tests/llleap_test.cpp @@ -23,7 +23,7 @@ #include "../test/lltut.h" #include "../test/namedtempfile.h" #include "../test/catch_and_store_what_in.h" -#include "wrapllerrs.h" +#include "wrapllerrs.h" // CaptureLog #include "llevents.h" #include "llprocess.h" #include "llstring.h" @@ -290,12 +290,9 @@ namespace tut void object::test<6>() { set_test_name("empty plugin vector"); - std::string threw; - try - { - LLLeap::create("empty", StringVec()); - } - CATCH_AND_STORE_WHAT_IN(threw, LLLeap::Error) + std::string threw = catch_what([](){ + LLLeap::create("empty", StringVec()); + }); ensure_contains("LLLeap::Error", threw, "no plugin"); // try the suppress-exception variant ensure("bad launch returned non-NULL", ! LLLeap::create("empty", StringVec(), false)); @@ -308,12 +305,9 @@ namespace tut // Synthesize bogus executable name std::string BADPYTHON(PYTHON.substr(0, PYTHON.length()-1) + "x"); CaptureLog log; - std::string threw; - try - { - LLLeap::create("bad exe", BADPYTHON); - } - CATCH_AND_STORE_WHAT_IN(threw, LLLeap::Error) + std::string threw = catch_what([&BADPYTHON](){ + LLLeap::create("bad exe", BADPYTHON); + }); ensure_contains("LLLeap::create() didn't throw", threw, "failed"); log.messageWith("failed"); log.messageWith(BADPYTHON); diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 5c87cdabd9..222d832084 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -25,8 +25,6 @@ #include #include #include -//#include -//#include // other Linden headers #include "../test/lltut.h" #include "../test/namedtempfile.h" @@ -35,7 +33,7 @@ #include "llsdutil.h" #include "llevents.h" #include "llstring.h" -#include "wrapllerrs.h" +#include "wrapllerrs.h" // CaptureLog #if defined(LL_WINDOWS) #define sleep(secs) _sleep((secs) * 1000) @@ -45,8 +43,7 @@ #include #endif -//namespace lambda = boost::lambda; - std::string apr_strerror_helper(apr_status_t rv) +std::string apr_strerror_helper(apr_status_t rv) { char errbuf[256]; apr_strerror(rv, errbuf, sizeof(errbuf)); @@ -960,12 +957,9 @@ namespace tut #define CATCH_IN(THREW, EXCEPTION, CODE) \ do \ { \ - (THREW).clear(); \ - try \ - { \ - CODE; \ - } \ - CATCH_AND_STORE_WHAT_IN(THREW, EXCEPTION) \ + (THREW) = catch_what([&](){ \ + CODE; \ + }); \ ensure("failed to throw " #EXCEPTION ": " #CODE, ! (THREW).empty()); \ } while (0) diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h index 08fbf19b1c..3fb79f6b5d 100644 --- a/indra/llcommon/tests/wrapllerrs.h +++ b/indra/llcommon/tests/wrapllerrs.h @@ -37,6 +37,7 @@ #include "llerrorcontrol.h" #include "llexception.h" #include "stringize.h" +#include "../test/catch_and_store_what_in.h" #include #include #include @@ -86,6 +87,13 @@ struct WrapLLErrs LLError::FatalFunction mPriorFatal; }; +/// Convenience wrapper for catch_what() +template +std::string catch_llerrs(FUNC func) +{ + return catch_what(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 c04696c86b..8e5bcd326c 100644 --- a/indra/llmessage/tests/llareslistener_test.cpp +++ b/indra/llmessage/tests/llareslistener_test.cpp @@ -138,15 +138,9 @@ namespace tut WrapLLErrs capture; LLSD request; request["op"] = "foo"; - std::string threw; - try - { - LLEventPumps::instance().obtain("LLAres").post(request); - } - catch (const WrapLLErrs::FatalException& e) - { - threw = e.what(); - } + std::string threw = catch_llerrs([&request](){ + LLEventPumps::instance().obtain("LLAres").post(request); + }); ensure_contains("LLAresListener bad op", threw, "bad"); } @@ -157,15 +151,9 @@ namespace tut WrapLLErrs capture; LLSD request; request["op"] = "rewriteURI"; - std::string threw; - try - { - LLEventPumps::instance().obtain("LLAres").post(request); - } - catch (const WrapLLErrs::FatalException& e) - { - threw = e.what(); - } + std::string threw = catch_llerrs([&request](){ + LLEventPumps::instance().obtain("LLAres").post(request); + }); ensure_contains("LLAresListener bad req", threw, "missing"); ensure_contains("LLAresListener bad req", threw, "reply"); ensure_contains("LLAresListener bad req", threw, "uri"); @@ -179,15 +167,9 @@ namespace tut LLSD request; request["op"] = "rewriteURI"; request["reply"] = "nonexistent"; - std::string threw; - try - { - LLEventPumps::instance().obtain("LLAres").post(request); - } - catch (const WrapLLErrs::FatalException& e) - { - threw = e.what(); - } + std::string threw = catch_llerrs([&request](){ + LLEventPumps::instance().obtain("LLAres").post(request); + }); ensure_contains("LLAresListener bad req", threw, "missing"); ensure_contains("LLAresListener bad req", threw, "uri"); ensure_does_not_contain("LLAresListener bad req", threw, "reply"); @@ -201,15 +183,9 @@ namespace tut LLSD request; request["op"] = "rewriteURI"; request["uri"] = "foo.bar.com"; - std::string threw; - try - { - LLEventPumps::instance().obtain("LLAres").post(request); - } - catch (const WrapLLErrs::FatalException& e) - { - threw = e.what(); - } + std::string threw = catch_llerrs([&request](){ + LLEventPumps::instance().obtain("LLAres").post(request); + }); ensure_contains("LLAresListener bad req", threw, "missing"); ensure_contains("LLAresListener bad req", threw, "reply"); ensure_does_not_contain("LLAresListener bad req", threw, "uri"); diff --git a/indra/newview/tests/llxmlrpclistener_test.cpp b/indra/newview/tests/llxmlrpclistener_test.cpp index 6e9756e7d5..f9a4ec373a 100644 --- a/indra/newview/tests/llxmlrpclistener_test.cpp +++ b/indra/newview/tests/llxmlrpclistener_test.cpp @@ -88,15 +88,9 @@ namespace tut WrapLLErrs capture; LLSD request; request["uri"] = uri; - std::string threw; - try - { - pumps.obtain("LLXMLRPCTransaction").post(request); - } - catch (const WrapLLErrs::FatalException& e) - { - threw = e.what(); - } + std::string threw = catch_llerrs([&pumps, &request](){ + pumps.obtain("LLXMLRPCTransaction").post(request); + }); ensure_contains("threw exception", threw, "missing params"); ensure_contains("identified missing", threw, "method"); ensure_contains("identified missing", threw, "reply"); @@ -113,15 +107,9 @@ namespace tut request["reply"] = "reply"; LLSD& params(request["params"]); params["who"]["specifically"] = "world"; // LLXMLRPCListener only handles scalar params - std::string threw; - try - { - pumps.obtain("LLXMLRPCTransaction").post(request); - } - catch (const WrapLLErrs::FatalException& e) - { - threw = e.what(); - } + std::string threw = catch_llerrs([&pumps, &request](){ + pumps.obtain("LLXMLRPCTransaction").post(request); + }); ensure_contains("threw exception", threw, "unknown type"); } diff --git a/indra/test/catch_and_store_what_in.h b/indra/test/catch_and_store_what_in.h index 59f8cc0085..5beba06024 100644 --- a/indra/test/catch_and_store_what_in.h +++ b/indra/test/catch_and_store_what_in.h @@ -2,7 +2,7 @@ * @file catch_and_store_what_in.h * @author Nat Goodspeed * @date 2012-02-15 - * @brief CATCH_AND_STORE_WHAT_IN() macro + * @brief catch_what() template function, CATCH_AND_STORE_WHAT_IN() macro * * $LicenseInfo:firstyear=2012&license=viewerlgpl$ * Copyright (c) 2012, Linden Research, Inc. @@ -12,6 +12,30 @@ #if ! defined(LL_CATCH_AND_STORE_WHAT_IN_H) #define LL_CATCH_AND_STORE_WHAT_IN_H +/** + * In the brave new world of lambdas, we can use a nicer C++ idiom for testing + * exceptions than CATCH_AND_STORE_WHAT_IN() below, e.g.: + * + * @code + * std::string threw = catch_what( + * [](){ throw std::runtime_error("badness"); }); + * ensure_equals(threw, "badness"); + * @endcode + */ +template +std::string catch_what(FUNC func) +{ + try + { + func(); + return {}; + } + catch (const EXCEPTION& err) + { + return err.what(); + } +} + /** * Idiom useful for test programs: catch an expected exception, store its * what() string in a specified std::string variable. From there the caller diff --git a/indra/test/llevents_tut.cpp b/indra/test/llevents_tut.cpp index 16edab6282..3abae3e43e 100644 --- a/indra/test/llevents_tut.cpp +++ b/indra/test/llevents_tut.cpp @@ -134,17 +134,15 @@ void events_object::test<1>() per_frame.post(4); check_listener("re-blocked", listener0, 3); } // unblock - std::string threw; - try - { - // NOTE: boost::bind() saves its arguments by VALUE! If you pass - // an object instance rather than a pointer, you'll end up binding - // to an internal copy of that instance! Use boost::ref() to - // capture a reference instead. - per_frame.listen(listener0.getName(), // note bug, dup name - boost::bind(&Listener::call, boost::ref(listener1), _1)); - } - CATCH_AND_STORE_WHAT_IN(threw, LLEventPump::DupListenerName) + std::string threw = catch_what( + [&per_frame, this](){ + // NOTE: boost::bind() saves its arguments by VALUE! If you pass + // an object instance rather than a pointer, you'll end up binding + // to an internal copy of that instance! Use boost::ref() to + // capture a reference instead. + per_frame.listen(listener0.getName(), // note bug, dup name + boost::bind(&Listener::call, boost::ref(listener1), _1)); + }); ensure_equals(threw, std::string("DupListenerName: " "Attempt to register duplicate listener name '") + @@ -341,15 +339,13 @@ void events_object::test<7>() ensure_equals(collector.result, make(list_of("Mary")("spot")("checked"))); collector.clear(); button.stopListening("spot"); - std::string threw; - try - { - button.listen("spot", - boost::bind(&Collect::add, boost::ref(collector), "spot", _1), - // after "Mary" and "checked" -- whoops! - make(list_of("Mary")("checked"))); - } - CATCH_AND_STORE_WHAT_IN(threw, LLEventPump::Cycle) + std::string threw = catch_what( + [&button, &collector](){ + button.listen("spot", + boost::bind(&Collect::add, boost::ref(collector), "spot", _1), + // after "Mary" and "checked" -- whoops! + make(list_of("Mary")("checked"))); + }); // Obviously the specific wording of the exception text can // change; go ahead and change the test to match. // Establish that it contains: @@ -374,15 +370,13 @@ void events_object::test<7>() button.post(3); ensure_equals(collector.result, make(list_of("Mary")("checked")("yellow")("shoelaces"))); collector.clear(); - threw.clear(); - try - { - button.listen("of", - boost::bind(&Collect::add, boost::ref(collector), "of", _1), - make(list_of("shoelaces")), - make(list_of("yellow"))); - } - CATCH_AND_STORE_WHAT_IN(threw, LLEventPump::OrderChange) + threw = catch_what( + [&button, &collector](){ + button.listen("of", + boost::bind(&Collect::add, boost::ref(collector), "of", _1), + make(list_of("shoelaces")), + make(list_of("yellow"))); + }); // Same remarks about the specific wording of the exception. Just // ensure that it contains enough information to clarify the // problem and what must be done to resolve it. @@ -404,13 +398,11 @@ void events_object::test<8>() { // nested scope // Hand-instantiate an LLEventStream... LLEventStream bob("bob"); - std::string threw; - try - { - // then another with a duplicate name. - LLEventStream bob2("bob"); - } - CATCH_AND_STORE_WHAT_IN(threw, LLEventPump::DupPumpName) + std::string threw = catch_what( + [](){ + // then another with a duplicate name. + LLEventStream bob2("bob"); + }); ensure("Caught DupPumpName", !threw.empty()); } // delete first 'bob' LLEventStream bob("bob"); // should work, previous one unregistered @@ -445,13 +437,11 @@ void events_object::test<9>() listener0.listenTo(random); eventSource("random"); check_listener("got by pump name", listener0, 17); - std::string threw; - try - { - LLListenerOrPumpName empty; - empty(17); - } - CATCH_AND_STORE_WHAT_IN(threw, LLListenerOrPumpName::Empty) + std::string threw = catch_what( + [](){ + LLListenerOrPumpName empty; + empty(17); + }); ensure("threw Empty", !threw.empty()); } diff --git a/indra/test/test.cpp b/indra/test/test.cpp index 861ec1d942..d4cd4b951e 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -37,7 +37,6 @@ #include "linden_common.h" #include "llerrorcontrol.h" #include "lltut.h" -#include "tests/wrapllerrs.h" // RecorderProxy #include "stringize.h" #include "namedtempfile.h" #include "lltrace.h" -- cgit v1.2.3