The small libraries' tests check what they say they check

indra/llcharacter/tests/altestmotion.h

@@ -91,6 +91,7 @@ class ALTestMotion final : public LLMotion
     F64 sendStopTimestamp() const { return mSendStopTimestamp; }
 
     using LLMotion::addJointState;
+    using LLMotion::mJointSignature;
 
     // Creates a joint state on `joint` with `usage`, adds it to the motion, and
     // returns it so the test can set its transform.

indra/llcharacter/tests/llbvhloader_test.cpp

@@ -636,7 +636,7 @@ namespace
                LINE_FRAME_TIME, "Frame Time: 0.0", E_ST_NO_FRAME_TIME);
         mutate("a negative frame time is refused",
                LINE_FRAME_TIME, "Frame Time: -0.033333", E_ST_NO_FRAME_TIME);
-        mutate("a frame time that is not a number is refused",
+        mutate("a frame time of NaN is refused",
                LINE_FRAME_TIME, "Frame Time: nan", E_ST_NO_FRAME_TIME);
         mutate("an infinite frame time is refused",
                LINE_FRAME_TIME, "Frame Time: inf", E_ST_NO_FRAME_TIME);
@@ -791,8 +791,12 @@ namespace
 
             UNSCOPED_INFO("the priority is the one the table gave");
             CHECK(written.mBasePriority == 4);
-            UNSCOPED_INFO("and every joint is given it");
-            CHECK(written.mJointPriorities[0] == 4);
+            {
+                INFO("and every joint is given it");
+                REQUIRE(written.mNumJoints == 2u);
+                CHECK(written.mJointPriorities[0] == 4);
+                CHECK(written.mJointPriorities[1] == 4);
+            }
             UNSCOPED_INFO("the hand pose is the one the table gave");
             CHECK(written.mHandPose == 3u);
             UNSCOPED_INFO("so is the emote");
@@ -939,6 +943,7 @@ namespace
             std::vector<U8> bytes = write(*loader);
             WrittenAnimation written;
             REQUIRE(read_written(bytes, written));
+            REQUIRE(written.mNumJoints == 2u);
             UNSCOPED_INFO("an alias read after a table undoes what it said");
             CHECK(written.mJointNames[1] == std::string("mTorso"));
         }
@@ -957,6 +962,7 @@ namespace
         {
             WrittenAnimation written = build(
                 header + "[GLOBALS]\npriority = 3\n[abdomen]\npriority = 2\n", sample_lines());
+            REQUIRE(written.mNumJoints == 2u);
             UNSCOPED_INFO("the base priority is the global one");
             CHECK(written.mBasePriority == 3);
             UNSCOPED_INFO("a joint with no section of its own takes it");
@@ -967,6 +973,7 @@ namespace
         {
             WrittenAnimation written = build(
                 header + "[GLOBALS]\npriority = 3\n[abdomen]\npriority = -2\n", sample_lines());
+            REQUIRE(written.mNumJoints == 2u);
             UNSCOPED_INFO("a modifier may take it down again");
             CHECK(written.mJointPriorities[1] == 1);
         }
@@ -976,6 +983,7 @@ namespace
             WrittenAnimation written = build(
                 header + "[GLOBALS]\npriority = 2147483647\n[abdomen]\npriority = 2147483647\n",
                 sample_lines());
+            REQUIRE(written.mNumJoints == 2u);
             UNSCOPED_INFO("the global priority is written as it was given");
             CHECK(written.mBasePriority == 2147483647);
             UNSCOPED_INFO("but a joint priority is brought into range");
@@ -985,6 +993,7 @@ namespace
             WrittenAnimation written = build(
                 header + "[GLOBALS]\npriority = -2147483648\n[abdomen]\npriority = -2147483648\n",
                 sample_lines());
+            REQUIRE(written.mNumJoints == 2u);
             UNSCOPED_INFO("from the other end too");
             CHECK(written.mJointPriorities[1] == (S32)LLJoint::USE_MOTION_PRIORITY);
         }
@@ -1075,6 +1084,8 @@ namespace
             // Every position key is moved by this, and five metres is as far
             // as a position key goes, so a hundred puts them all at the end.
             WrittenAnimation written = build(header + "[hip]\nrelativepos = 100 0 0\n", lines);
+            REQUIRE(written.mNumJoints == 2u);
+            REQUIRE_FALSE(written.mPosKeys[0].empty());
             UNSCOPED_INFO("a relative position moves the position keys");
             CHECK(written.mPosKeys[0] != plain.mPosKeys[0]);
             UNSCOPED_INFO("as far as they go");
@@ -1086,6 +1097,7 @@ namespace
         }
         {
             WrittenAnimation written = build(header + "[abdomen]\nrelativerot = firstkey\n", lines);
+            REQUIRE(written.mNumJoints == 2u);
             UNSCOPED_INFO("a rotation taken from the first key moves the rotation keys");
             CHECK(written.mRotKeys[1] != plain.mRotKeys[1]);
             UNSCOPED_INFO("and leaves the other joint alone");
@@ -1094,6 +1106,7 @@ namespace
         {
             WrittenAnimation written = build(
                 header + "[abdomen]\nframe = 1 0 0, 0 1 0, 0 0 1\n", lines);
+            REQUIRE(written.mNumJoints == 2u);
             UNSCOPED_INFO("a frame matrix moves the rotation keys");
             CHECK(written.mRotKeys[1] != plain.mRotKeys[1]);
             UNSCOPED_INFO("and leaves the other joint alone");
@@ -1102,6 +1115,7 @@ namespace
         {
             WrittenAnimation written = build(
                 header + "[abdomen]\noffset = 0 1 0, 0 0 1, 1 0 0\n", lines);
+            REQUIRE(written.mNumJoints == 2u);
             UNSCOPED_INFO("an offset matrix moves the rotation keys");
             CHECK(written.mRotKeys[1] != plain.mRotKeys[1]);
         }
@@ -1115,23 +1129,27 @@ namespace
         const std::string header = "Translations 1.0\n";
         const std::vector<std::string> lines = moving_lines();
         const WrittenAnimation plain = build(header, lines);
+        REQUIRE(plain.mNumJoints == 2u);
 
         {
             WrittenAnimation written = build(header + "[abdomen]\nmergeparent = hip\n", lines);
+            REQUIRE(written.mNumJoints == 2u);
             UNSCOPED_INFO("a merged parent moves the rotation keys");
             CHECK(written.mRotKeys[1] != plain.mRotKeys[1]);
             UNSCOPED_INFO("and leaves the parent alone");
             CHECK(written.mRotKeys[0] == plain.mRotKeys[0]);
         }
         {
             WrittenAnimation written = build(header + "[hip]\nmergechild = abdomen\n", lines);
+            REQUIRE(written.mNumJoints == 2u);
             UNSCOPED_INFO("a merged child moves the rotation keys");
             CHECK(written.mRotKeys[0] != plain.mRotKeys[0]);
         }
         {
             // A name that is nobody is nobody, rather than an error: the table
             // is written once and the files come and go.
             WrittenAnimation written = build(header + "[abdomen]\nmergeparent = elbow\n", lines);
+            REQUIRE(written.mNumJoints == 2u);
             UNSCOPED_INFO("a merge with a joint the file does not have changes nothing");
             CHECK(written.mRotKeys[1] == plain.mRotKeys[1]);
         }
@@ -1151,6 +1169,7 @@ namespace
             CHECK(one_frame.mNumRotKeys[1] == 1);
 
             WrittenAnimation merged = build(header + "[abdomen]\nmergeparent = hip\n", single);
+            REQUIRE(merged.mNumJoints == 2u);
             UNSCOPED_INFO("a merge with no frame behind it is no merge");
             CHECK(merged.mRotKeys[1] == one_frame.mRotKeys[1]);
         }

indra/llcharacter/tests/llcharacter_test.cpp

@@ -173,7 +173,9 @@ namespace
         LLJoint* head = mCharacter.getJoint("mHead");
         REQUIRE(head != nullptr);
         CHECK(head->getJointNum() == 4);
-        CHECK(head->getParent()->getName() == std::string("mNeck"));
+        LLJoint* neck = head->getParent();
+        REQUIRE(neck != nullptr);
+        CHECK(neck->getName() == std::string("mNeck"));
 
         CHECK(mCharacter.getCharacterJoint(2) == mCharacter.getJoint("mChest"));
         CHECK(mCharacter.getJoint("mNope") == nullptr);
@@ -428,6 +430,7 @@ namespace
         UNSCOPED_INFO("the motion behind the starter was updated once");
         CHECK(oldest->mUpdateCount == 2);
         ALTestMotion* started = static_cast<ALTestMotion*>(mCharacter.findMotion(started_id));
+        REQUIRE(started != nullptr);
         UNSCOPED_INFO("the new motion had only its activation update");
         CHECK(started->mUpdateCount == 1);
 

indra/llcharacter/tests/lljoint_test.cpp

@@ -172,7 +172,7 @@ namespace
         CHECK(jnt2 == NULL);
 
         lljointgp.addChild(&lljoint);
-        CHECK(&lljoint== lljoint2.getParent());
+        CHECK(&lljointgp == lljoint.getParent());
         CHECK(&lljointgp == lljoint2.getRoot());
 
         CHECK(&lljoint1 == lljoint.findJoint("child1"));

indra/llcharacter/tests/llkeyframemotion_test.cpp

@@ -314,7 +314,7 @@ namespace
             require_same_value(what, time, sample(curve, time, cursor), reference.getValue(time));
             INFO(what << ": cursor");
             CAPTURE(time);
-            REQUIRE_THAT(cursor, ll_test::LLSDEquals(reference.lowerBound(time)));
+            REQUIRE(cursor == reference.lowerBound(time));
         }
         return wraps;
     }

indra/llcharacter/tests/llmultigesture_test.cpp

@@ -55,10 +55,12 @@ namespace
         LLGestureStepSound* sound = new LLGestureStepSound();
         sound->mSoundName = "chime";
         sound->mSoundAssetID.generate();
+        sound->mFlags = 1;
         gesture->mSteps.push_back(sound);
 
         LLGestureStepChat* chat = new LLGestureStepChat();
         chat->mChatText = "hello";
+        chat->mFlags = 1;
         gesture->mSteps.push_back(chat);
 
         LLGestureStepWait* wait = new LLGestureStepWait();
@@ -138,11 +140,33 @@ namespace
         CHECK(read.mTrigger == written->mTrigger);
         CHECK(read.mReplaceText == written->mReplaceText);
         REQUIRE(read.mSteps.size() == written->mSteps.size());
+        for (size_t i = 0; i < written->mSteps.size(); ++i)
+        {
+            CAPTURE(i);
+            REQUIRE(read.mSteps[i]->getType() == written->mSteps[i]->getType());
+        }
 
-        CHECK(static_cast<LLGestureStepAnimation*>(read.mSteps[0])->mAnimName ==
-              std::string("express_wave"));
-        CHECK(static_cast<LLGestureStepChat*>(read.mSteps[2])->mChatText ==
-              std::string("hello"));
+        const auto& written_anim = *static_cast<LLGestureStepAnimation*>(written->mSteps[0]);
+        const auto& read_anim = *static_cast<LLGestureStepAnimation*>(read.mSteps[0]);
+        CHECK(read_anim.mAnimName == written_anim.mAnimName);
+        CHECK(read_anim.mAnimAssetID == written_anim.mAnimAssetID);
+        CHECK(read_anim.mFlags == written_anim.mFlags);
+
+        const auto& written_sound = *static_cast<LLGestureStepSound*>(written->mSteps[1]);
+        const auto& read_sound = *static_cast<LLGestureStepSound*>(read.mSteps[1]);
+        CHECK(read_sound.mSoundName == written_sound.mSoundName);
+        CHECK(read_sound.mSoundAssetID == written_sound.mSoundAssetID);
+        CHECK(read_sound.mFlags == written_sound.mFlags);
+
+        const auto& written_chat = *static_cast<LLGestureStepChat*>(written->mSteps[2]);
+        const auto& read_chat = *static_cast<LLGestureStepChat*>(read.mSteps[2]);
+        CHECK(read_chat.mChatText == written_chat.mChatText);
+        CHECK(read_chat.mFlags == written_chat.mFlags);
+
+        const auto& written_wait = *static_cast<LLGestureStepWait*>(written->mSteps[3]);
+        const auto& read_wait = *static_cast<LLGestureStepWait*>(read.mSteps[3]);
+        CHECK(read_wait.mWaitSeconds == written_wait.mWaitSeconds);
+        CHECK(read_wait.mFlags == written_wait.mFlags);
     }
 
     TEST_CASE("LLMultiGesture: a gesture cut short anywhere is refused", "[LLMultiGesture]")
@@ -238,6 +262,7 @@ namespace
             UNSCOPED_INFO("the animation step reads back");
             CHECK(step_accepts(whole, bytes));
             CHECK(whole.mAnimName == full.mAnimName);
+            CHECK(whole.mAnimAssetID == full.mAnimAssetID);
             CHECK(whole.mFlags == full.mFlags);
 
             LLGestureStepAnimation cut;
@@ -255,6 +280,8 @@ namespace
             UNSCOPED_INFO("the sound step reads back");
             CHECK(step_accepts(whole, bytes));
             CHECK(whole.mSoundName == full.mSoundName);
+            CHECK(whole.mSoundAssetID == full.mSoundAssetID);
+            CHECK(whole.mFlags == full.mFlags);
 
             LLGestureStepSound cut;
             UNSCOPED_INFO("a sound step missing its flags is refused");
@@ -285,6 +312,7 @@ namespace
             LLGestureStepWait whole;
             UNSCOPED_INFO("the wait step reads back");
             CHECK(step_accepts(whole, bytes));
+            CHECK(whole.mWaitSeconds == full.mWaitSeconds);
             CHECK(whole.mFlags == full.mFlags);
 
             LLGestureStepWait cut;

indra/llcharacter/tests/llpose_test.cpp

@@ -681,10 +681,12 @@ namespace
         require_same_rotation("cleared queue is a no-op", mA.getRotation(), ROT_C);
     }
 
-    TEST_CASE_METHOD(llposeblender_data, "LLPoseBlender: a joint with no number is kept in the motion's pose", "[LLPoseBlender]")
+    TEST_CASE_METHOD(llposeblender_data, "LLPoseBlender: a joint with no number is in the motion's pose but not its signature", "[LLPoseBlender]")
     {
-        // A joint that never received a joint number is kept in the pose but
-        // left out of the motion's joint signature.
+        // A joint that never received a joint number is still kept in the
+        // pose: LLMotion::addJointState inserts the state before it checks the
+        // number's range. It is left out of the motion's joint signature,
+        // which has no slot for it.
         LLJoint unnumbered;
         unnumbered.setup("unnumbered");
         UNSCOPED_INFO("default joint number");
@@ -693,6 +695,8 @@ namespace
         ALTestMotion motion;
         motion.addJoint(&unnumbered, LLJointState::ROT);
         CHECK(motion.getPose()->getNumJointStates() == 1);
+        const U8* signature = &motion.mJointSignature[0][0];
+        CHECK(std::count_if(signature, signature + sizeof(motion.mJointSignature), [](U8 b) { return b != 0; }) == 0);
     }
 
     TEST_CASE_METHOD(llposeblender_data, "LLPoseBlender: a joint with no number is skipped and the rest still blend", "[LLPoseBlender]")

indra/llfilesystem/tests/lldir_test.cpp

@@ -418,7 +418,7 @@ namespace
    {
       CAPTURE(directory, pattern);
 
-      // Scan directory and see if any file1.* files are found
+      // Scan directory for pattern and check that exactly the files correctResult marks are found
       std::string scanResult;
       int   found = 0;
       bool  filesFound[5] = { false, false, false, false, false };
@@ -464,7 +464,7 @@ namespace
       std::string delim = gDirUtilp->getDirDelimiter();
       std::string dirTemp = LLFile::tmpdir();
 
-      // Create the same 5 file names of the two directories
+      // Create the same 5 file names in each of two directories
 
       std::string dir1 = makeTestDir();
       std::string dir2 = makeTestDir();
@@ -481,7 +481,7 @@ namespace
       bool expected1[5] = { true, true, true, true, true };
       require_scan_returns(dir1, "*", expected1);
 
-      // Scan dir2 and see if only the 2 *.xyz files are found
+      // Scan dir1 and see if only the 2 *.xyz files are found
       bool  expected2[5] = { false, false, true, true, false };
       require_scan_returns(dir1, "*.xyz", expected2);
 
@@ -497,15 +497,15 @@ namespace
       bool  expected5[5] = { true, false, true, false, true };
       require_scan_returns(dir1, "file1.*", expected5);
 
-      // Scan dir1 and see if any file1.* files are found
+      // Scan dir1 and see if any file?.abc files are found
       bool  expected6[5] = { true, true, false, false, false };
       require_scan_returns(dir1, "file?.abc", expected6);
 
       // Scan dir2 and see if any file?.x?z files are found
       bool  expected7[5] = { false, false, true, true, false };
       require_scan_returns(dir2, "file?.x?z", expected7);
 
-      // Scan dir2 and see if any file?.??c files are found
+      // Scan dir2 and see if any file?.??c or *.??c files are found
       bool  expected8[5] = { true, true, false, false, false };
       require_scan_returns(dir2, "file?.??c", expected8);
       require_scan_returns(dir2, "*.??c", expected8);

indra/llimage/tests/llimageworker_test.cpp

@@ -36,6 +36,9 @@
 // Catch2 header
 #include <catch2/catch_test_macros.hpp>
 
+#include <atomic>
+#include <memory>
+
 // -------------------------------------------------------------------------------------------
 // Stubbing: Declarations required to link and run the class being tested
 // Notes:
@@ -115,20 +118,23 @@ namespace
     class responder_test : public LLImageDecodeThread::Responder
     {
         public:
-            responder_test(bool* res)
+            responder_test(std::shared_ptr<std::atomic<bool>> res)
+                : done(std::move(res))
             {
-                done = res;
-                *done = false;
             }
             virtual void completed(bool success, const std::string& error_message, LLImageRaw* raw, LLImageRaw* aux, U32 request_id)
             {
-                *done = true;
+                done->store(true);
             }
         private:
             // This is what can be thought of as the minimal implementation of a responder
             // Done will be switched to true when completed() is called and can be tested
             // outside the responder. A better way of doing this is to store a callback here.
-            bool* done;
+            // completed() runs on a decode worker thread while the test polls the flag, so
+            // the flag is atomic; and the responder shares ownership of it, so a responder
+            // that completes after the test has given up waiting writes to a live flag
+            // rather than into a stack frame that is gone.
+            std::shared_ptr<std::atomic<bool>> done;
     };
 
     // Test wrapper declaration : decode thread
@@ -166,20 +172,20 @@ namespace
         mThread = new LLImageDecodeThread(true);
         REQUIRE(mThread != NULL);
         // Insert something in the queue
-        bool done = false;
-        LLImageDecodeThread::handle_t decodeHandle = mThread->decodeImage(NULL, 0, false, new responder_test(&done));
+        const auto done = std::make_shared<std::atomic<bool>>(false);
+        LLImageDecodeThread::handle_t decodeHandle = mThread->decodeImage(NULL, 0, false, new responder_test(done));
         // Verifies we get back a valid handle
         CHECK(decodeHandle != 0);
         // Wait till the thread has time to handle the work order (though it doesn't do much per work order...)
         const U32 INCREMENT_TIME = 500;             // 500 milliseconds
         const U32 MAX_TIME = 20 * INCREMENT_TIME;   // Do the loop 20 times max, i.e. wait 10 seconds but no more
         U32 total_time = 0;
-        while ((done == false) && (total_time < MAX_TIME))
+        while (!done->load() && (total_time < MAX_TIME))
         {
             ms_sleep(INCREMENT_TIME);
             total_time += INCREMENT_TIME;
         }
         // Verifies that the responder has now been called
-        CHECK(done);
+        CHECK(done->load());
     }
 }