Test framework: fix the old test bugs in llcommon's tests, and LLApp::instance() after the app is gone

indra/llcommon/llapp.cpp

@@ -127,6 +127,12 @@ LLApp::~LLApp()
     setStopped();
 
     SUBSYSTEM_CLEANUP_DBG(LLCommon);
+
+    // Gone, so instance() no longer finds it.
+    if (sApplication == this)
+    {
+        sApplication = NULL;
+    }
 }
 
 // static

indra/llcommon/tests/alrecoverystore_test.cpp

@@ -422,6 +422,7 @@ namespace
         ALRecoveryStore other(folder, "theirs");
         CHECK(other.write(entry("item:x", "left before")));
         const std::optional<ALRecoveryEntry> took = store.leftFor("item:x");
+        CHECK(took.has_value());
         CHECK(store.write(entry("item:x", "typed since")));
         // A file where the folder of the discarded would be: nothing can be
         // written there.
@@ -628,8 +629,10 @@ namespace
         CHECK_FALSE(LLFile::isfile(bare));
 
         CHECK(store.setAside(entry("item:b", "old")));
+        const std::vector<std::string> again = files("/discarded");
+        REQUIRE(again.size() == size_t(1));
         const std::string garbled = folder + "/discarded/0001.mine.soon.llsd";
-        CHECK(LLFile::rename(folder + "/discarded/" + files("/discarded").front(), garbled) == 0);
+        CHECK(LLFile::rename(folder + "/discarded/" + again.front(), garbled) == 0);
         store.prune(60.0, LLDate(LLDate::now().secondsSinceEpoch() + 30.0));
         UNSCOPED_INFO("a stamp that is no number read past");
         CHECK(LLFile::isfile(garbled));
@@ -732,8 +735,10 @@ namespace
     {
         ALRecoveryStore store(folder, "now");
         ALRecoveryEntry went = entry("task:a:b", "typed, then its window went");
-        store.write(went);
-        const LLDate written = store.list().front().when;
+        CHECK(store.write(went));
+        const std::vector<ALRecoveryEntry> own = store.list();
+        REQUIRE(own.size() == size_t(1));
+        const LLDate written = own.front().when;
         UNSCOPED_INFO("not left by another session");
         REQUIRE_FALSE(store.leftFor("task:a:b"));
 
@@ -763,7 +768,7 @@ namespace
         REQUIRE_FALSE(store.reclaim("task:a:b"));
 
         ALRecoveryStore other(folder, "earlier");
-        other.write(entry("task:c:d", "another session's"));
+        CHECK(other.write(entry("task:c:d", "another session's")));
         {
             INFO("another session's is not this one's to reclaim");
             REQUIRE_FALSE(store.reclaim("task:c:d"));
@@ -862,8 +867,10 @@ namespace
         }
         CHECK(require_whole(store, store.leftFor("item:a"))->text == "left");
         store.markOffered(store.left(), LLDate(offered.secondsSinceEpoch() + 100.0));
+        const std::vector<std::string> remarked = files();
+        REQUIRE(remarked.size() == 1);
         UNSCOPED_INFO("marked once");
-        CHECK(files().front() == names.front());
+        CHECK(remarked.front() == names.front());
         store.prune(60.0, LLDate(offered.secondsSinceEpoch() + 30.0));
         UNSCOPED_INFO("young, it stays");
         REQUIRE(files().size() == 1);
@@ -891,14 +898,15 @@ namespace
             ms_sleep(5);
         }
         std::vector<ALRecoveryEntry> kept = require_whole(store, store.list());
-        REQUIRE(files("/discarded").size() == size_t(3));
+        const std::vector<std::string> discarded = files("/discarded");
+        REQUIRE(discarded.size() == size_t(3));
         {
             INFO("the newest");
             CHECK(std::any_of(kept.begin(), kept.end(), [](const ALRecoveryEntry& one) { return one.text == "discarded 4"; }));
             CHECK(std::none_of(kept.begin(), kept.end(), [](const ALRecoveryEntry& one) { return one.text == "discarded 1"; }));
         }
         std::error_code ec;
-        const uintmax_t one = std::filesystem::file_size(fsyspath(folder + "/discarded/" + files("/discarded").front()), ec);
+        const uintmax_t one = std::filesystem::file_size(fsyspath(folder + "/discarded/" + discarded.front()), ec);
         store.limitDiscarded(100, static_cast<size_t>(one * 2 + one / 2));
         store.prune(60.0);
         UNSCOPED_INFO("two, by what they weigh");

indra/llcommon/tests/alregex_test.cpp

@@ -264,9 +264,19 @@ namespace
         CHECK_FALSE(ALRegex(".*x").firstByteRange(lo, hi));
         UNSCOPED_INFO("nor is a pattern that did not compile");
         CHECK_FALSE(ALRegex("(?=x)").firstByteRange(lo, hi));
-        // After a word, \b is before what is not one.
+        // After a word, \b is before what is not one: here any byte but a
+        // word's, 0 and 0xFF among them. With the start's, a word's byte that
+        // is no small letter, that is every byte, so no range.
         UNSCOPED_INFO("a boundary before a non-letter");
-        CHECK((ALRegex("\\b[^a-z]x").firstByteRange(lo, hi) == false || (lo <= '!' && hi >= '_')));
+        CHECK_FALSE(ALRegex("\\b[^a-z]x").firstByteRange(lo, hi));
+        // One that can begin only after a word: \b at the start wants a
+        // word's byte, and no punctuation is one. The range is what may
+        // follow a word, rounded up one past '/' as RE2 rounds.
+        UNSCOPED_INFO("a boundary before punctuation");
+        CHECK(ALRegex("\\b[!-/]x").firstByteRange(lo, hi));
+        CHECK(lo == '!');
+        CHECK(hi >= '/');
+        CHECK(hi <= '0');
         CHECK(ALRegex("@[A-Za-z_][A-Za-z0-9_]*").firstByteRange(lo, hi));
         CHECK(lo == '@');
         CHECK(hi <= 'A');

indra/llcommon/tests/commonmisc_test.cpp

@@ -1,8 +1,8 @@
 /**
- * @file common.cpp
+ * @file commonmisc_test.cpp
  * @author Phoenix
  * @date 2005-10-12
- * @brief Common templates for test framework
+ * @brief Tests of llhash, LLMemoryStream, LLSD notation and the U64 conversions
  *
  * $LicenseInfo:firstyear=2005&license=viewerlgpl$
  * Second Life Viewer Source Code
@@ -26,12 +26,6 @@
  * $/LicenseInfo$
  */
 
-/**
- *
- * THOROUGH_DESCRIPTION of common.cpp
- *
- */
-
 #include <algorithm>
 #include <iomanip>
 #include <iterator>
@@ -46,32 +40,6 @@
 
 #include <catch2/catch_test_macros.hpp>
 
-namespace
-{
-/*
-    template<> template<>
-    void sd_object::test<16>()
-    {
-    }
-*/
-}
-
-#if 0
-'{\'task_id\':u1fd77b79-a8e7-25a5-9454-02a4d948ba1c}\n{\n\tname\tObject|\n\tpermissions 0\n\t{\n\t\tbase_mask\t7fffffff\n\t\towner_mask\t7fffffff\n\t\tgroup_mask\t00000000\n\t\teveryone_mask\t00000000\n\t\tnext_owner_mask\t00082000\n\t\tcreator_id\t3c115e51-04f4-523c-9fa6-98aff1034730\n\t\towner_id\t3c115e51-04f4-523c-9fa6-98aff1034730\n\t\tlast_owner_id\t00000000-0000-0000-0000-000000000000\n\t\tgroup_id\t00000000-0000-0000-0000-000000000000\n\t}\n\tlocal_id\t10284\n\ttotal_crc\t35\n\ttype\t1\n\ttask_valid\t2\n\ttravel_access\t21\n\tdisplayopts\t2\n\tdisplaytype\tv\n\tpos\t0\t0\t0\n\toldpos\t0\t0\t0\n\trotation\t4.371139183945160766597837e-08\t1\t4.371139183945160766597837e-08\t0\n\tvelocity\t0\t0\t0\n\tangvel\t0\t0\t0\n\tscale\t0.2816932\t0.2816932\t0.2816932\n\tsit_offset\t0\t0\t0\n\tcamera_eye_offset\t0\t0\t0\n\tcamera_at_offset\t0\t0\t0\n\tsit_quat\t0\t0\t0\t1\n\tsit_hint\t0\n\tstate\t80\n\tmaterial\t3\n\tsoundid\t00000000-0000-0000-0000-000000000000\n\tsoundgain\t0\n\tsoundradius\t0\n\tsoundflags\t0\n\ttextcolor\t0 0 0 1\n\tselected\t0\n\tselector\t00000000-0000-0000-0000-000000000000\n\tusephysics\t0\n\trotate_x\t1\n\trotate_y\t1\n\trotate_z\t1\n\tphantom\t0\n\tremote_script_access_pin\t0\n\tvolume_detect\t0\n\tblock_grabs\t0\n\tdie_at_edge\t0\n\treturn_at_edge\t0\n\ttemporary\t0\n\tsandbox\t0\n\tsandboxhome\t0\t0\t0\n\tshape 0\n\t{\n\t\tpath 0\n\t\t{\n\t\t\tcurve\t16\n\t\t\tbegin\t0\n\t\t\tend\t1\n\t\t\tscale_x\t1\n\t\t\tscale_y\t1\n\t\t\tshear_x\t0\n\t\t\tshear_y\t0\n\t\t\ttwist\t0\n\t\t\ttwist_begin\t0\n\t\t\tradius_offset\t0\n\t\t\ttaper_x\t0\n\t\t\ttaper_y\t0\n\t\t\trevolutions\t1\n\t\t\tskew\t0\n\t\t}\n\t\tprofile 0\n\t\t{\n\t\t\tcurve\t1\n\t\t\tbegin\t0\n\t\t\tend\t1\n\t\t\thollow\t0\n\t\t}\n\t}\n\tfaces\t6\n\t{\n\t\timageid\t89556747-24cb-43ed-920b-47caed15465f\n\t\tcolors\t1 1 1 1\n\t\tscales\t0.56\n\t\tscalet\t0.56\n\t\toffsets\t0\n\t\toffsett\t0\n\t\timagerot\t0\n\t\tbump\t0\n\t\tfullbright\t0\n\t\tmedia_flags\t0\n\t}\n\t{\n\t\timageid\t89556747-24cb-43ed-920b-47caed15465f\n\t\tcolors\t1 1 1 1\n\t\tscales\t0.56\n\t\tscalet\t0.56\n\t\toffsets\t0\n\t\toffsett\t0\n\t\timagerot\t0\n\t\tbump\t0\n\t\tfullbright\t0\n\t\tmedia_flags\t0\n\t}\n\t{\n\t\timageid\t89556747-24cb-43ed-920b-47caed15465f\n\t\tcolors\t1 1 1 1\n\t\tscales\t0.56\n\t\tscalet\t0.56\n\t\toffsets\t0\n\t\toffsett\t0\n\t\timagerot\t0\n\t\tbump\t0\n\t\tfullbright\t0\n\t\tmedia_flags\t0\n\t}\n\t{\n\t\timageid\t89556747-24cb-43ed-920b-47caed15465f\n\t\tcolors\t1 1 1 1\n\t\tscales\t0.56\n\t\tscalet\t0.56\n\t\toffsets\t0\n\t\toffsett\t0\n\t\timagerot\t0\n\t\tbump\t0\n\t\tfullbright\t0\n\t\tmedia_flags\t0\n\t}\n\t{\n\t\timageid\t89556747-24cb-43ed-920b-47caed15465f\n\t\tcolors\t1 1 1 1\n\t\tscales\t0.56\n\t\tscalet\t0.56\n\t\toffsets\t0\n\t\toffsett\t0\n\t\timagerot\t0\n\t\tbump\t0\n\t\tfullbright\t0\n\t\tmedia_flags\t0\n\t}\n\t{\n\t\timageid\t89556747-24cb-43ed-920b-47caed15465f\n\t\tcolors\t1 1 1 1\n\t\tscales\t0.56\n\t\tscalet\t0.56\n\t\toffsets\t0\n\t\toffsett\t0\n\t\timagerot\t0\n\t\tbump\t0\n\t\tfullbright\t0\n\t\tmedia_flags\t0\n\t}\n\tps_next_crc\t1\n\tgpw_bias\t1\n\tip\t0\n\tcomplete\tTRUE\n\tdelay\t50000\n\tnextstart\t1132625972249870\n\tbirthtime\t1132625953120694\n\treztime\t1132625953120694\n\tparceltime\t1132625953120694\n\ttax_rate\t1.01615\n\tnamevalue\tAttachmentOrientation VEC3 RW DS -3.141593, 0.000000, -3.141593\n\tnamevalue\tAttachmentOffset VEC3 RW DS 0.000000, 0.000000, 0.000000\n\tnamevalue\tAttachPt U32 RW S 5\n\tnamevalue\tAttachItemID STRING RW SV 1f9975c0-2951-1b93-dd83-46e2b932fcc8\n\tscratchpad\t0\n\t{\n\t\n\t}\n\tsale_info\t0\n\t{\n\t\tsale_type\tnot\n\t\tsale_price\t10\n\t}\n\torig_asset_id\t52019cdd-b464-ba19-e66d-3da751fef9da\n\torig_item_id\t1f9975c0-2951-1b93-dd83-46e2b932fcc8\n\tcorrect_family_id\t00000000-0000-0000-0000-000000000000\n\thas_rezzed\t0\n\tpre_link_base_mask\t7fffffff\n\tdefault_pay_price\t-2\t1\t5\t10\t20\n}\n'
-#endif
-
-namespace
-{
-}
-
-namespace
-{
-    // llstrtou64
-    // seems to be deprecated - could not find it being used
-    // anywhere in the tarball - skipping unit tests for now
-}
-
-
 namespace
 {
     TEST_CASE("LLHash: llhash depends on the string and not on its pointer", "[LLHash]")
@@ -128,7 +96,6 @@ namespace
     TEST_CASE("LLSD: binary writes to notation as b(6) and parses back", "[LLSD]")
     {
         const std::string decoded("random");
-        //const std::string encoded("cmFuZG9t\n");
         const std::string streamed("b(6)\"random\"");
         typedef std::vector<U8> buf_t;
         buf_t buf;
@@ -293,11 +260,8 @@ namespace
         REQUIRE(sd.size() == 4);
         CHECK(sd["label"].asString() ==
             "short binary test");
-        std::vector<U8> bin =  sd["singlebinary"].asBinary();
-        std::vector<U8> expected;
-        expected.resize(1);
-        expected[0] = 'A';
-        CHECK(memcmp(&bin[0], &expected[0], 1) == 0);
+        const std::vector<U8> expected{'A'};
+        CHECK(sd["singlebinary"].asBinary() == expected);
         CHECK(sd["singlerawstring"].asString() ==
             std::string("A"));
         CHECK(sd["endoftest"].asString() == "end");
@@ -359,47 +323,15 @@ namespace
             std::stringstream str;
             str << "'" << LLSDNotationFormatter::escapeString(expected) << "'";
             LLSD sd;
-            S32 count = LLSDSerialize::fromNotation(sd, str, expected.size());
+            S32 count = LLSDSerialize::fromNotation(sd, str, str.str().size());
             REQUIRE(count == 1);
             std::string actual = sd.asString();
-/*
-            if(actual != expected)
-            {
-                LL_WARNS() << "iteration " << i << LL_ENDL;
-                std::ostringstream e_str;
-                std::string::iterator iter = expected.begin();
-                std::string::iterator end = expected.end();
-                for(; iter != end; ++iter)
-                {
-                    e_str << (S32)((U8)(*iter)) << " ";
-                }
-                e_str << std::endl;
-                llsd_serialize_string(e_str, expected);
-                LL_WARNS() << "expected size: " << expected.size() << LL_ENDL;
-                LL_WARNS() << "expected:      " << e_str.str() << LL_ENDL;
-
-                std::ostringstream a_str;
-                iter = actual.begin();
-                end = actual.end();
-                for(; iter != end; ++iter)
-                {
-                    a_str << (S32)((U8)(*iter)) << " ";
-                }
-                a_str << std::endl;
-                llsd_serialize_string(a_str, actual);
-                LL_WARNS() << "actual size:   " << actual.size() << LL_ENDL;
-                LL_WARNS() << "actual:      " << a_str.str() << LL_ENDL;
-            }
-*/
             REQUIRE(actual == expected);
         }
     }
 
     TEST_CASE("LLSD: an instant message parameter array parses", "[LLSD]")
     {
-//#if LL_WINDOWS && _MSC_VER >= 1400
-//        skip_fail("Fails on VS2005 due to broken LLSDSerialize::fromNotation() parser.");
-//#endif
         std::string param = "[{'version':i1},{'data':{'binary_bucket':b(0)\"\"},'from_id':u3c115e51-04f4-523c-9fa6-98aff1034730,'from_name':'Phoenix Linden','id':u004e45e5-5576-277a-fba7-859d6a4cb5c8,'message':'hey','offline':i0,'timestamp':i0,'to_id':u3c5f1bb4-5182-7546-6401-1d329b4ff2f8,'type':i0},{'agent_id':u3c115e51-04f4-523c-9fa6-98aff1034730,'god_level':i0,'limited_to_estate':i1}]";
         std::istringstream istr;
         istr.str(param);

indra/llcommon/tests/lazyeventapi_test.cpp

@@ -15,6 +15,7 @@
 #include "lazyeventapi.h"
 // STL headers
 // std headers
+#include <memory>
 // external library headers
 // other Linden headers
 #include <catch2/catch_test_macros.hpp>
@@ -95,11 +96,12 @@ namespace
         CHECK(data.asString() == "hey");
     }
 
-    TEST_CASE_METHOD(lazyeventapi_data, "lazyeventapi: without a LazyEventAPI no listener answers its pump", "[lazyeventapi]")
+    TEST_CASE_METHOD(lazyeventapi_data, "lazyeventapi: a destroyed LazyEventAPI no longer creates its listener", "[lazyeventapi]")
     {
-        // Because the MyRegistrar declaration in test<1>() is local, because
-        // it has been destroyed, we fully expect NOT to reach a MyListener
-        // instance with this post.
+        // A registrar destroyed before anyone obtains its pump takes its pump
+        // factory with it, so this post must NOT reach a MyListener instance.
+        auto regster{ std::make_unique<MyRegistrar>() };
+        regster.reset();
         LLEventPumps::instance().obtain("Test").post(llsd::map("op", "set", "data", "moot"));
         CHECK_FALSE(data.isDefined());
     }

indra/llcommon/tests/llapp_test.cpp

@@ -84,4 +84,12 @@ namespace
         mApp->setOptionData(LLApp::PRIORITY_RUNTIME_OVERRIDE, options);
         CHECK_FALSE(mApp->getOption("boolean-test").asBoolean());
     }
+
+    TEST_CASE_METHOD(application, "application: instance() finds no app once it is gone", "[application]")
+    {
+        CHECK(LLApp::instance() == mApp);
+        delete mApp;
+        mApp = nullptr;
+        CHECK(LLApp::instance() == nullptr);
+    }
 }

indra/llcommon/tests/lldate_test.cpp

@@ -51,17 +51,21 @@
 #define INVALID_DATE_EMPTY                          ""
 
 // invalid values
-// apr 1.1.1 seems to not care about constraining the date to valid
-// dates. Put these back when the parser checks.
+// LLDate::fromStream checks only the format: it hands the fields to timegm,
+// which normalizes a value out of range rather than rejecting it (24:00:00 is
+// the next day's midnight, 2003-02-29 is 2003-03-01). Turn the case below on
+// when the parser checks the ranges.
 #define LL_DATE_PARSER_CHECKS_BOUNDARY 0
-//#define INVALID_DATE_24HOUR_BOUNDARY              "2003-04-30T24:00:00Z"
-//#define INVALID_DATE_LEAP                         "2003-04-29T04:00:00Z"
-//#define INVALID_DATE_HOUR                         "2003-04-30T24:59:59Z"
-//#define INVALID_DATE_MIN                          "2003-04-30T22:69:59Z"
-//#define INVALID_DATE_SEC                          "2003-04-30T22:59:69Z"
-//#define INVALID_DATE_YEAR                         "0-04-30T22:59:59Z"
-//#define INVALID_DATE_MONTH                            "2003-13-30T22:59:59Z"
-//#define INVALID_DATE_DAY                          "2003-04-35T22:59:59Z"
+#if LL_DATE_PARSER_CHECKS_BOUNDARY
+#define INVALID_DATE_24HOUR_BOUNDARY                "2003-04-30T24:00:00Z"
+#define INVALID_DATE_LEAP                           "2003-02-29T04:00:00Z"
+#define INVALID_DATE_HOUR                           "2003-04-30T24:59:59Z"
+#define INVALID_DATE_MIN                            "2003-04-30T22:69:59Z"
+#define INVALID_DATE_SEC                            "2003-04-30T22:59:69Z"
+#define INVALID_DATE_YEAR                           "0-04-30T22:59:59Z"
+#define INVALID_DATE_MONTH                          "2003-13-30T22:59:59Z"
+#define INVALID_DATE_DAY                            "2003-04-35T22:59:59Z"
+#endif
 
 namespace tut
 {
@@ -133,12 +137,11 @@ namespace tut
         CHECK_FALSE(result);
     }
 
+#if LL_DATE_PARSER_CHECKS_BOUNDARY
     /* Invalid Value Handling */
     TEST_CASE("LLDate: fromString rejects out-of-range field values", "[LLDate]")
     {
-#if LL_DATE_PARSER_CHECKS_BOUNDARY
         LLDate date;
-        std::string  expected_string;
         bool result;
 
         result = date.fromString(INVALID_DATE_24HOUR_BOUNDARY);
@@ -174,8 +177,8 @@ namespace tut
         result = date.fromString(INVALID_DATE_DAY);
         UNSCOPED_INFO("INVALID_DATE_DAY should have failed");
         CHECK_FALSE(result);
-#endif
     }
+#endif // LL_DATE_PARSER_CHECKS_BOUNDARY
 
     /* API checks */
     TEST_CASE("LLDate: fromStream reads a date", "[LLDate]")
@@ -266,9 +269,12 @@ namespace tut
         LLDate d;
         CHECK(d.toLocalDateString(std::string("%Y-%m-%d")).size() > 0);
         CHECK(d.toHTTPDateString(std::string("%Y-%m-%d")).size() > 0);
-        // RFC1123 wraps toHTTPDateString; verifies the day-of-week token
-        // is present (a strftime success indicator that doesn't pin locale).
+        // RFC1123 wraps toHTTPDateString as "%A, %d %b %Y %H:%M:%S GMT".
+        // The day-of-week token, whatever the locale names it, comes before
+        // the first ", ": a strftime success indicator that doesn't pin locale.
         std::string rfc = d.asRFC1123();
-        CHECK(rfc.size() > 0);
+        const size_t day_end = rfc.find(", ");
+        CHECK(day_end != std::string::npos);
+        CHECK(day_end > 0);
     }
 }

indra/llcommon/tests/lldeadmantimer_test.cpp

@@ -334,7 +334,7 @@ TEST_CASE_METHOD(deadmantimer_test, "LLDeadmanTimer: isExpired reports an expiry
         count = U64L(8);
         user_cpu = 29000;
         sys_cpu = 57000;
-        CHECK_FALSE(timer.isExpired(now, started, stopped, count));
+        CHECK_FALSE(timer.isExpired(now, started, stopped, count, user_cpu, sys_cpu));
         CHECK_THAT(started, ll_test::WithinBits(F64(42.0), 2));
         CHECK_THAT(stopped, ll_test::WithinBits(F64(97.0), 2));
         CHECK(count == U64L(8));