From 2596816f316e13b717bcdacddad0da48c90c3b6d Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 1 Mar 2012 13:43:52 -0500 Subject: Break out TestRecorder class as CaptureLog into wrapllerrs.h. Giving more unit tests the ability to capture and examine log output is generally useful. Renaming the class just makes it less ambiguous: what's a TestRecorder? Something that records tests? --- indra/llcommon/tests/wrapllerrs.h | 65 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) (limited to 'indra/llcommon/tests/wrapllerrs.h') diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h index ffda84729b..a61f8451b3 100644 --- a/indra/llcommon/tests/wrapllerrs.h +++ b/indra/llcommon/tests/wrapllerrs.h @@ -30,6 +30,14 @@ #define LL_WRAPLLERRS_H #include "llerrorcontrol.h" +#include +#include +#include +#include + +// statically reference the function in test.cpp... it's short, we could +// replicate, but better to reuse +extern void wouldHaveCrashed(const std::string& message); struct WrapLL_ERRS { @@ -70,4 +78,61 @@ struct WrapLL_ERRS LLError::FatalFunction mPriorFatal; }; +/** + * Capture log messages. This is adapted (simplified) from the one in + * llerror_test.cpp. + */ +class CaptureLog : public LLError::Recorder +{ +public: + CaptureLog(): + // Mostly what we're trying to accomplish by saving and resetting + // LLError::Settings is to bypass the default RecordToStderr and + // RecordToWinDebug Recorders. As these are visible only inside + // llerror.cpp, we can't just call LLError::removeRecorder() with + // each. For certain tests we need to produce, capture and examine + // DEBUG log messages -- but we don't want to spam the user's console + // with that output. If it turns out that saveAndResetSettings() has + // some bad effect, give up and just let the DEBUG level log messages + // display. + mOldSettings(LLError::saveAndResetSettings()) + { + LLError::setFatalFunction(wouldHaveCrashed); + LLError::setDefaultLevel(LLError::LEVEL_DEBUG); + LLError::addRecorder(this); + } + + ~CaptureLog() + { + LLError::removeRecorder(this); + LLError::restoreSettings(mOldSettings); + } + + void recordMessage(LLError::ELevel level, + const std::string& message) + { + mMessages.push_back(message); + } + + /// Don't assume the message we want is necessarily the LAST log message + /// emitted by the underlying code; search backwards through all messages + /// for the sought string. + std::string messageWith(const std::string& search) + { + for (std::list::const_reverse_iterator rmi(mMessages.rbegin()), + rmend(mMessages.rend()); + rmi != rmend; ++rmi) + { + if (rmi->find(search) != std::string::npos) + return *rmi; + } + // failed to find any such message + return std::string(); + } + + typedef std::list MessageList; + MessageList mMessages; + LLError::Settings* mOldSettings; +}; + #endif /* ! defined(LL_WRAPLLERRS_H) */ -- cgit v1.2.3 From f904867720291bc63ea5e140194069d29f70fb40 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 1 Mar 2012 14:33:23 -0500 Subject: Make CaptureLog::withMessage() raise tut::failure if not found. All known callers were using ensure(! withMessage(...).empty()). Centralize that logic. Make failure message report the string being sought and the log messages in which it wasn't found. In case someone does want to permit the search to fail, add an optional 'required' parameter, default true. Leverage new functionality in llprocess_test.cpp. --- indra/llcommon/tests/wrapllerrs.h | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) (limited to 'indra/llcommon/tests/wrapllerrs.h') diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h index a61f8451b3..28ffbf517f 100644 --- a/indra/llcommon/tests/wrapllerrs.h +++ b/indra/llcommon/tests/wrapllerrs.h @@ -29,11 +29,13 @@ #if ! defined(LL_WRAPLLERRS_H) #define LL_WRAPLLERRS_H +#include #include "llerrorcontrol.h" #include #include #include #include +#include // statically reference the function in test.cpp... it's short, we could // replicate, but better to reuse @@ -117,17 +119,26 @@ public: /// Don't assume the message we want is necessarily the LAST log message /// emitted by the underlying code; search backwards through all messages /// for the sought string. - std::string messageWith(const std::string& search) + std::string messageWith(const std::string& search, bool required=true) { - for (std::list::const_reverse_iterator rmi(mMessages.rbegin()), - rmend(mMessages.rend()); + for (MessageList::const_reverse_iterator rmi(mMessages.rbegin()), rmend(mMessages.rend()); rmi != rmend; ++rmi) { if (rmi->find(search) != std::string::npos) return *rmi; } // failed to find any such message - return std::string(); + if (! required) + return std::string(); + + std::ostringstream out; + out << "failed to find '" << search << "' in captured log messages:"; + for (MessageList::const_iterator mi(mMessages.begin()), mend(mMessages.end()); + mi != mend; ++mi) + { + out << '\n' << *mi; + } + throw tut::failure(out.str()); } typedef std::list MessageList; -- cgit v1.2.3 From f0612f6fc424ab4726ef187f41a257c0abdd1d34 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 1 Mar 2012 17:30:50 -0500 Subject: Allow CaptureLog's consumer to specify desired log level. Of course, given the way the log machinery works, it's really "everything at that level or stronger." --- indra/llcommon/tests/wrapllerrs.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/llcommon/tests/wrapllerrs.h') diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h index 28ffbf517f..10cf25b39e 100644 --- a/indra/llcommon/tests/wrapllerrs.h +++ b/indra/llcommon/tests/wrapllerrs.h @@ -87,7 +87,7 @@ struct WrapLL_ERRS class CaptureLog : public LLError::Recorder { public: - CaptureLog(): + CaptureLog(LLError::ELevel level=LLError::LEVEL_DEBUG): // Mostly what we're trying to accomplish by saving and resetting // LLError::Settings is to bypass the default RecordToStderr and // RecordToWinDebug Recorders. As these are visible only inside @@ -100,7 +100,7 @@ public: mOldSettings(LLError::saveAndResetSettings()) { LLError::setFatalFunction(wouldHaveCrashed); - LLError::setDefaultLevel(LLError::LEVEL_DEBUG); + LLError::setDefaultLevel(level); LLError::addRecorder(this); } -- cgit v1.2.3 From cfe37cbfb5bdec0aa01c86a608ecc6cdc3df8a2a Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 1 Mar 2012 22:45:16 -0500 Subject: Break out std::ostream << CaptureLog routine for general use. --- indra/llcommon/tests/wrapllerrs.h | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) (limited to 'indra/llcommon/tests/wrapllerrs.h') diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h index 10cf25b39e..a9c0c19c28 100644 --- a/indra/llcommon/tests/wrapllerrs.h +++ b/indra/llcommon/tests/wrapllerrs.h @@ -31,11 +31,11 @@ #include #include "llerrorcontrol.h" +#include "stringize.h" #include #include #include #include -#include // statically reference the function in test.cpp... it's short, we could // replicate, but better to reuse @@ -131,14 +131,9 @@ public: if (! required) return std::string(); - std::ostringstream out; - out << "failed to find '" << search << "' in captured log messages:"; - for (MessageList::const_iterator mi(mMessages.begin()), mend(mMessages.end()); - mi != mend; ++mi) - { - out << '\n' << *mi; - } - throw tut::failure(out.str()); + throw tut::failure(STRINGIZE("failed to find '" << search + << "' in captured log messages:\n" + << *this)); } typedef std::list MessageList; @@ -146,4 +141,20 @@ public: LLError::Settings* mOldSettings; }; +std::ostream& operator<<(std::ostream& out, const CaptureLog& log) +{ + CaptureLog::MessageList::const_iterator mi(log.mMessages.begin()), mend(log.mMessages.end()); + if (mi != mend) + { + // handle first message separately: it doesn't get a newline + out << *mi++; + for ( ; mi != mend; ++mi) + { + // every subsequent message gets a newline + out << '\n' << *mi; + } + } + return out; +} + #endif /* ! defined(LL_WRAPLLERRS_H) */ -- cgit v1.2.3 From b3b51f012f49d82c3b7f6f5cadfc0d76ef82f8bf Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 5 Mar 2012 13:03:23 -0500 Subject: Move std::ostream << CaptureLog logic into CaptureLog::streamto(). That lets us reliably declare the operator<<() free function inline, which permits multiple translation units in the same executable to #include "wrapllerrs.h". --- indra/llcommon/tests/wrapllerrs.h | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) (limited to 'indra/llcommon/tests/wrapllerrs.h') diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h index a9c0c19c28..f79acacb22 100644 --- a/indra/llcommon/tests/wrapllerrs.h +++ b/indra/llcommon/tests/wrapllerrs.h @@ -136,25 +136,31 @@ public: << *this)); } + std::ostream& streamto(std::ostream& out) const + { + MessageList::const_iterator mi(mMessages.begin()), mend(mMessages.end()); + if (mi != mend) + { + // handle first message separately: it doesn't get a newline + out << *mi++; + for ( ; mi != mend; ++mi) + { + // every subsequent message gets a newline + out << '\n' << *mi; + } + } + return out; + } + typedef std::list MessageList; MessageList mMessages; LLError::Settings* mOldSettings; }; +inline std::ostream& operator<<(std::ostream& out, const CaptureLog& log) { - CaptureLog::MessageList::const_iterator mi(log.mMessages.begin()), mend(log.mMessages.end()); - if (mi != mend) - { - // handle first message separately: it doesn't get a newline - out << *mi++; - for ( ; mi != mend; ++mi) - { - // every subsequent message gets a newline - out << '\n' << *mi; - } - } - return out; + return log.streamto(out); } #endif /* ! defined(LL_WRAPLLERRS_H) */ -- cgit v1.2.3 From eb1bea222322385e6e5b05206f09f21bb891f3f7 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 23 Apr 2012 11:26:18 -0400 Subject: IQA-463: LLError::addRecorder() claims ownership of passed Recorder*. That is, when the underlying LLError::Settings object is destroyed -- possibly at termination, possibly on LLError::restoreSettings() -- the passed Recorder* is deleted. There was much existing code that seemed as unaware of this alarming fact as I was myself. Passing to addRecorder() a pointer to a stack object, or to a member of some other object, is just Bad. It might be preferable to make addRecorder() accept std::auto_ptr to make the ownership transfer more explicit -- or even boost::shared_ptr instead, which would allow the caller to either forget or retain the passed Recorder. This preliminary pass retains the Recorder* dumb pointer API, but documents the ownership issue, and eliminates known instances of passing pointers to anything but a standalone heap Recorder subclass object. --- indra/llcommon/tests/wrapllerrs.h | 46 ++++++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 5 deletions(-) (limited to 'indra/llcommon/tests/wrapllerrs.h') diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h index f79acacb22..a4d3a4e026 100644 --- a/indra/llcommon/tests/wrapllerrs.h +++ b/indra/llcommon/tests/wrapllerrs.h @@ -29,10 +29,15 @@ #if ! defined(LL_WRAPLLERRS_H) #define LL_WRAPLLERRS_H +#if LL_WINDOWS +#pragma warning (disable : 4355) // 'this' used in initializer list: yes, intentionally +#endif + #include #include "llerrorcontrol.h" #include "stringize.h" #include +#include #include #include #include @@ -80,11 +85,39 @@ struct WrapLL_ERRS LLError::FatalFunction mPriorFatal; }; +/** + * LLError::addRecorder() accepts ownership of the passed Recorder* -- it + * expects to be able to delete it later. CaptureLog isa Recorder whose + * pointer we want to be able to pass without any ownership implications. + * For such cases, instantiate a new RecorderProxy(yourRecorder) and pass + * that. Your heap RecorderProxy might later be deleted, but not yourRecorder. + */ +class RecorderProxy: public LLError::Recorder +{ +public: + RecorderProxy(LLError::Recorder* recorder): + mRecorder(recorder) + {} + + virtual void recordMessage(LLError::ELevel level, const std::string& message) + { + mRecorder->recordMessage(level, message); + } + + virtual bool wantsTime() + { + return mRecorder->wantsTime(); + } + +private: + LLError::Recorder* mRecorder; +}; + /** * Capture log messages. This is adapted (simplified) from the one in * llerror_test.cpp. */ -class CaptureLog : public LLError::Recorder +class CaptureLog : public LLError::Recorder, public boost::noncopyable { public: CaptureLog(LLError::ELevel level=LLError::LEVEL_DEBUG): @@ -97,16 +130,18 @@ public: // with that output. If it turns out that saveAndResetSettings() has // some bad effect, give up and just let the DEBUG level log messages // display. - mOldSettings(LLError::saveAndResetSettings()) + mOldSettings(LLError::saveAndResetSettings()), + mProxy(new RecorderProxy(this)) { LLError::setFatalFunction(wouldHaveCrashed); LLError::setDefaultLevel(level); - LLError::addRecorder(this); + LLError::addRecorder(mProxy); } ~CaptureLog() { - LLError::removeRecorder(this); + LLError::removeRecorder(mProxy); + delete mProxy; LLError::restoreSettings(mOldSettings); } @@ -133,7 +168,7 @@ public: throw tut::failure(STRINGIZE("failed to find '" << search << "' in captured log messages:\n" - << *this)); + << boost::ref(*this))); } std::ostream& streamto(std::ostream& out) const @@ -155,6 +190,7 @@ public: typedef std::list MessageList; MessageList mMessages; LLError::Settings* mOldSettings; + LLError::Recorder* mProxy; }; inline -- cgit v1.2.3