From d241423593c1de206e5fb8b57911d2db2d5a7c31 Mon Sep 17 00:00:00 2001 From: Rye Date: Mon, 12 Oct 2026 03:04:17 -0400 Subject: [PATCH 1/3] llcharacter's tests check what they say they check The Catch2 conversion kept these as TUT had them; now they are fixed: - lljoint: after addChild, a check repeated an earlier one where it meant that lljoint's parent is now lljointgp. It checks that. - llcharacter: a joint's parent, and a motion findMotion found, were dereferenced unchecked. Each is REQUIREd first, so a failure fails the case rather than crashing the run. - llpose: a case said an unnumbered joint is left out of the motion's joint signature and never looked. ALTestMotion exposes the signature, and the case checks that no byte of it is set; LLMotion writing one at joint number -1 would set the last byte of the row before. - llmultigesture: the sound step's flags were never read back, and the read-back case looked at two of four steps. Every field of every step is now compared with what was written, and the sample gesture's sound and chat steps carry flags, so a reader that skipped them would show. - llbvhloader: results were indexed with no joint count checked first; each is REQUIREd now. One case said every joint is given the global priority and looked at one. Two refusals shared a message. - llkeyframemotion: a U32 cursor was compared through LLSDEquals, as TUT's overload for a string message chose; it is compared as the integer it is. Each new or corrected check was shown to fail under a small change to the test's own input or expected value, and to pass as written. All of llcharacter_tests passes in 10 random orders: 38,119 assertions in 143 cases (38,083 before). Co-Authored-By: Claude Opus 5.5 --- indra/llcharacter/tests/altestmotion.h | 1 + indra/llcharacter/tests/llbvhloader_test.cpp | 25 +++++++++++-- indra/llcharacter/tests/llcharacter_test.cpp | 5 ++- indra/llcharacter/tests/lljoint_test.cpp | 2 +- .../tests/llkeyframemotion_test.cpp | 2 +- .../llcharacter/tests/llmultigesture_test.cpp | 36 ++++++++++++++++--- indra/llcharacter/tests/llpose_test.cpp | 10 ++++-- 7 files changed, 68 insertions(+), 13 deletions(-) diff --git a/indra/llcharacter/tests/altestmotion.h b/indra/llcharacter/tests/altestmotion.h index 22d89553d1..b4c765868e 100644 --- a/indra/llcharacter/tests/altestmotion.h +++ b/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. diff --git a/indra/llcharacter/tests/llbvhloader_test.cpp b/indra/llcharacter/tests/llbvhloader_test.cpp index cf246028b9..761d306c00 100644 --- a/indra/llcharacter/tests/llbvhloader_test.cpp +++ b/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 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,9 +1129,11 @@ namespace const std::string header = "Translations 1.0\n"; const std::vector 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"); @@ -1125,6 +1141,7 @@ namespace } { 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]); } @@ -1132,6 +1149,7 @@ namespace // 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]); } diff --git a/indra/llcharacter/tests/llcharacter_test.cpp b/indra/llcharacter/tests/llcharacter_test.cpp index 8ef8dd5bb1..e2af8893c3 100644 --- a/indra/llcharacter/tests/llcharacter_test.cpp +++ b/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(mCharacter.findMotion(started_id)); + REQUIRE(started != nullptr); UNSCOPED_INFO("the new motion had only its activation update"); CHECK(started->mUpdateCount == 1); diff --git a/indra/llcharacter/tests/lljoint_test.cpp b/indra/llcharacter/tests/lljoint_test.cpp index b5c785134e..33293f06c8 100644 --- a/indra/llcharacter/tests/lljoint_test.cpp +++ b/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")); diff --git a/indra/llcharacter/tests/llkeyframemotion_test.cpp b/indra/llcharacter/tests/llkeyframemotion_test.cpp index 93e5068571..0e8a044a47 100644 --- a/indra/llcharacter/tests/llkeyframemotion_test.cpp +++ b/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; } diff --git a/indra/llcharacter/tests/llmultigesture_test.cpp b/indra/llcharacter/tests/llmultigesture_test.cpp index b8e6840e62..64562109dc 100644 --- a/indra/llcharacter/tests/llmultigesture_test.cpp +++ b/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(read.mSteps[0])->mAnimName == - std::string("express_wave")); - CHECK(static_cast(read.mSteps[2])->mChatText == - std::string("hello")); + const auto& written_anim = *static_cast(written->mSteps[0]); + const auto& read_anim = *static_cast(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(written->mSteps[1]); + const auto& read_sound = *static_cast(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(written->mSteps[2]); + const auto& read_chat = *static_cast(read.mSteps[2]); + CHECK(read_chat.mChatText == written_chat.mChatText); + CHECK(read_chat.mFlags == written_chat.mFlags); + + const auto& written_wait = *static_cast(written->mSteps[3]); + const auto& read_wait = *static_cast(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; diff --git a/indra/llcharacter/tests/llpose_test.cpp b/indra/llcharacter/tests/llpose_test.cpp index dc6bad1466..e4e3e6338b 100644 --- a/indra/llcharacter/tests/llpose_test.cpp +++ b/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]") From 741c56efa239cf11407e1dd41931ac05aeac0791 Mon Sep 17 00:00:00 2001 From: Rye Date: Mon, 12 Oct 2026 03:04:17 -0400 Subject: [PATCH 2/3] llxml's and llinventory's tests check what they say they check - alxmllayermerge: the check that an observer hears nothing once it is gone looked at an observer never installed, so it could not fail. It checks the one that was installed and then removed. - llcontrol: the listener flag was a static never reset, so a second run of the case in one process passed whether or not the listener fired. It is the fixture's, set by a lambda. - llpermissions: - the allowCopyBy case called allowModifyBy; - allowModifyBy, allowCopyBy and allowMoveBy were asked only under the default masks, where everyone may. Each case now gives the owner, the group and everyone their own masks in turn, and asks the owner, a member as the group, the same member without it, and a stranger. Modify stays refused to everyone, which fix() enforces; - a getValue(PERM_TRANSFER) result was thrown away. The transfer value is checked after each aggregate. - inventorymisc: three fromLLSD results were never checked, and a case left as a placeholder for a removed XML export asserted nothing and goes. - llsaleinfo: a sale info was built and never used; the lookup now goes through it. The placeholder for message packing becomes a round trip through packMessage and unpackMessage's LLSD forms, which need no message system. - llsettingssky: two profiles were indexed with no size checked. - lluserrelations loses a commented-out TUT stub. Six files' @file lines gave their TUT-era names. Each new or corrected check was shown to fail under a small change to the test's own input or expected value; inventorymisc's parse checks cannot be made to, since LLInventoryItem::fromLLSD returns true whatever it is given. llxml_tests passes 223 assertions in 25 cases, and llinventory_tests 378 in 57 (356 in 58 before), in 10 random orders. Co-Authored-By: Claude Opus 5.5 --- .../llinventory/tests/inventorymisc_test.cpp | 13 +-- indra/llinventory/tests/llparcel_test.cpp | 2 +- .../llinventory/tests/llpermissions_test.cpp | 96 +++++++++++++------ indra/llinventory/tests/llsaleinfo_test.cpp | 21 ++-- .../llinventory/tests/llsettingssky_test.cpp | 2 + .../tests/lluserrelations_test.cpp | 9 +- indra/llxml/tests/alxmllayermerge_test.cpp | 3 +- indra/llxml/tests/llcontrol_test.cpp | 15 +-- 8 files changed, 96 insertions(+), 65 deletions(-) diff --git a/indra/llinventory/tests/inventorymisc_test.cpp b/indra/llinventory/tests/inventorymisc_test.cpp index d7c0cab69a..45fa333fdb 100644 --- a/indra/llinventory/tests/inventorymisc_test.cpp +++ b/indra/llinventory/tests/inventorymisc_test.cpp @@ -1,5 +1,5 @@ /** - * @file inventory.cpp + * @file inventorymisc_test.cpp * @author Phoenix * @date 2005-11-15 * @brief Functions for inventory test framework @@ -243,6 +243,7 @@ namespace sd = ll_create_sd_from_inventory_item(src); //LL_INFOS() << "sd: " << *sd << LL_ENDL; successful_parse = dst->fromLLSD(sd); + CHECK(successful_parse); CHECK(dst->getUUID() == src->getUUID()); CHECK(dst->getParentUUID() == src->getParentUUID()); CHECK(dst->getName() == src->getName()); @@ -374,7 +375,7 @@ namespace LL_ERRS()<< "Parsing cache failed" << LL_ENDL; return; } - src2->fromLLSD(s_item); + CHECK(src2->fromLLSD(s_item)); file.close(); @@ -417,18 +418,12 @@ namespace CHECK_FALSE(src2->getIsFavorite()); // not supposed to carry over } - TEST_CASE("LLInventory: placeholder for the removed XML export and import", "[LLInventory]") - { - // Deleted LLInventoryItem::exportFileXML() and LLInventoryItem::importXML() - // because I can't find any non-test code references to it. 2009-05-04 JC - } - TEST_CASE("LLInventory: an item survives asLLSD and fromLLSD", "[LLInventory]") { LLPointer src1 = create_random_inventory_item(); LLSD retSd = src1->asLLSD(); LLPointer src2 = new LLInventoryItem(); - src2->fromLLSD(retSd); + CHECK(src2->fromLLSD(retSd)); CHECK(src1->getUUID() == src2->getUUID()); CHECK(src1->getParentUUID() == src2->getParentUUID()); diff --git a/indra/llinventory/tests/llparcel_test.cpp b/indra/llinventory/tests/llparcel_test.cpp index 50e90663fd..5daab2aff6 100644 --- a/indra/llinventory/tests/llparcel_test.cpp +++ b/indra/llinventory/tests/llparcel_test.cpp @@ -1,5 +1,5 @@ /** - * @file llinventoryparcel_tut.cpp + * @file llparcel_test.cpp * @author Moss * @date 2007-04-17 * diff --git a/indra/llinventory/tests/llpermissions_test.cpp b/indra/llinventory/tests/llpermissions_test.cpp index 0425b9a5b9..f608712f1e 100644 --- a/indra/llinventory/tests/llpermissions_test.cpp +++ b/indra/llinventory/tests/llpermissions_test.cpp @@ -1,5 +1,5 @@ /** - * @file llpermissions_tut.cpp + * @file llpermissions_test.cpp * @author Adroit * @date March 2007 * @brief llpermissions test cases. @@ -298,55 +298,96 @@ namespace CHECK(perm.allowOperationBy(bits,agent1,group)); } - TEST_CASE("permission: allowModifyBy allows any agent under the default masks", "[permission]") + // allowModifyBy, allowCopyBy and allowMoveBy answer the owner by the + // owner mask, a member asking as the object's group by the group mask + // (or the everyone mask), and anyone else by the everyone mask. The + // system, the null agent, may always. initMasks takes base, owner, + // everyone, group, next, in that order. + TEST_CASE("permission: allowModifyBy answers the owner and a group member and a stranger by their masks", "[permission]") { LLPermissions perm; LLUUID creator("abf0d56b-82e5-47a2-a8ad-74741bb2c29e"); - LLUUID owner; + LLUUID owner("68edcf47-ccd7-45b8-9f90-1649d7f12806"); LLUUID lastOwner("5e47a0dc-97bf-44e0-8b40-de06718cee9d"); LLUUID group("9c8eca51-53d5-42a7-bb58-cef070395db8"); + LLUUID member("1f6e2b7c-3d4a-4e5f-8a9b-0c1d2e3f4a5b"); + LLUUID stranger("7d8e9fa0-b1c2-4d3e-9f40-5a6b7c8d9e0f"); perm.init(creator,owner,lastOwner,group); - LLUUID agent; - CHECK(perm.allowModifyBy(agent)); - CHECK(perm.allowModifyBy(agent,group)); - LLUUID agent1("9c8eca51-53d5-42a7-bb58-cef070395db8"); - CHECK(perm.allowModifyBy(agent1)); - CHECK(perm.allowModifyBy(agent1,group)); + // The owner may modify; the group and everyone may not. + perm.initMasks(PERM_ALL, PERM_ALL, PERM_NONE, PERM_NONE, PERM_ALL); + CHECK(perm.allowModifyBy(LLUUID::null)); + CHECK(perm.allowModifyBy(owner)); + CHECK_FALSE(perm.allowModifyBy(member, group)); + CHECK_FALSE(perm.allowModifyBy(stranger)); + + // The group may too, but only to a member asking as the group. + perm.initMasks(PERM_ALL, PERM_ALL, PERM_NONE, PERM_MODIFY, PERM_ALL); + CHECK(perm.allowModifyBy(member, group)); + CHECK_FALSE(perm.allowModifyBy(member)); + CHECK_FALSE(perm.allowModifyBy(stranger)); + + // Everyone never may, whatever its mask asks for. + perm.initMasks(PERM_ALL, PERM_ALL, PERM_ALL, PERM_MODIFY, PERM_ALL); + CHECK_FALSE(perm.allowModifyBy(stranger)); } - TEST_CASE("permission: allowCopyBy allows any agent under the default masks", "[permission]") + TEST_CASE("permission: allowCopyBy answers the owner and a group member and a stranger by their masks", "[permission]") { LLPermissions perm; LLUUID creator("abf0d56b-82e5-47a2-a8ad-74741bb2c29e"); - LLUUID owner; + LLUUID owner("68edcf47-ccd7-45b8-9f90-1649d7f12806"); LLUUID lastOwner("5e47a0dc-97bf-44e0-8b40-de06718cee9d"); LLUUID group("9c8eca51-53d5-42a7-bb58-cef070395db8"); + LLUUID member("1f6e2b7c-3d4a-4e5f-8a9b-0c1d2e3f4a5b"); + LLUUID stranger("7d8e9fa0-b1c2-4d3e-9f40-5a6b7c8d9e0f"); perm.init(creator,owner,lastOwner,group); - LLUUID agent; - CHECK(perm.allowModifyBy(agent)); - CHECK(perm.allowModifyBy(agent,group)); - LLUUID agent1("9c8eca51-53d5-42a7-bb58-cef070395db8"); - CHECK(perm.allowCopyBy(agent1)); - CHECK(perm.allowCopyBy(agent1,group)); + // The owner may copy; the group and everyone may not. + perm.initMasks(PERM_ALL, PERM_ALL, PERM_NONE, PERM_NONE, PERM_ALL); + CHECK(perm.allowCopyBy(LLUUID::null)); + CHECK(perm.allowCopyBy(owner)); + CHECK_FALSE(perm.allowCopyBy(member, group)); + CHECK_FALSE(perm.allowCopyBy(stranger)); + + // The group may too, but only to a member asking as the group. + perm.initMasks(PERM_ALL, PERM_ALL, PERM_NONE, PERM_COPY, PERM_ALL); + CHECK(perm.allowCopyBy(member, group)); + CHECK_FALSE(perm.allowCopyBy(member)); + CHECK_FALSE(perm.allowCopyBy(stranger)); + + // And once everyone may, so may a stranger. + perm.initMasks(PERM_ALL, PERM_ALL, PERM_COPY, PERM_COPY, PERM_ALL); + CHECK(perm.allowCopyBy(stranger)); } - TEST_CASE("permission: allowMoveBy allows any agent under the default masks", "[permission]") + TEST_CASE("permission: allowMoveBy answers the owner and a group member and a stranger by their masks", "[permission]") { LLPermissions perm; LLUUID creator("abf0d56b-82e5-47a2-a8ad-74741bb2c29e"); - LLUUID owner; + LLUUID owner("68edcf47-ccd7-45b8-9f90-1649d7f12806"); LLUUID lastOwner("5e47a0dc-97bf-44e0-8b40-de06718cee9d"); LLUUID group("9c8eca51-53d5-42a7-bb58-cef070395db8"); + LLUUID member("1f6e2b7c-3d4a-4e5f-8a9b-0c1d2e3f4a5b"); + LLUUID stranger("7d8e9fa0-b1c2-4d3e-9f40-5a6b7c8d9e0f"); perm.init(creator,owner,lastOwner,group); - LLUUID agent; - CHECK(perm.allowMoveBy(agent)); - CHECK(perm.allowMoveBy(agent,group)); - LLUUID agent1("9c8eca51-53d5-42a7-bb58-cef070395db8"); - CHECK(perm.allowMoveBy(agent1)); - CHECK(perm.allowMoveBy(agent1,group)); + // The owner may move; the group and everyone may not. + perm.initMasks(PERM_ALL, PERM_ALL, PERM_NONE, PERM_NONE, PERM_ALL); + CHECK(perm.allowMoveBy(LLUUID::null)); + CHECK(perm.allowMoveBy(owner)); + CHECK_FALSE(perm.allowMoveBy(member, group)); + CHECK_FALSE(perm.allowMoveBy(stranger)); + + // The group may too, but only to a member asking as the group. + perm.initMasks(PERM_ALL, PERM_ALL, PERM_NONE, PERM_MOVE, PERM_ALL); + CHECK(perm.allowMoveBy(member, group)); + CHECK_FALSE(perm.allowMoveBy(member)); + CHECK_FALSE(perm.allowMoveBy(stranger)); + + // And once everyone may, so may a stranger. + perm.initMasks(PERM_ALL, PERM_ALL, PERM_MOVE, PERM_MOVE, PERM_ALL); + CHECK(perm.allowMoveBy(stranger)); } TEST_CASE("permission: allowTransferTo allows the null agent while the owner is null", "[permission]") @@ -456,14 +497,15 @@ namespace LLAggregatePermissions AggrPermission1; CHECK(AggrPermission.getU8() == 0); CHECK(AggrPermission.isEmpty()); - AggrPermission.getValue(PERM_TRANSFER); - CHECK(AggrPermission.getValue(PERM_TRANSFER) == 0x00); + CHECK(AggrPermission.getValue(PERM_TRANSFER) == LLAggregatePermissions::AP_EMPTY); AggrPermission.aggregate(PERM_ITEM_UNRESTRICTED); CHECK_FALSE(AggrPermission.isEmpty()); + CHECK(AggrPermission.getValue(PERM_TRANSFER) == LLAggregatePermissions::AP_ALL); AggrPermission1.aggregate(AggrPermission); CHECK_FALSE(AggrPermission1.isEmpty()); + CHECK(AggrPermission1.getValue(PERM_TRANSFER) == LLAggregatePermissions::AP_ALL); std::ostringstream stream1; stream1 << AggrPermission; diff --git a/indra/llinventory/tests/llsaleinfo_test.cpp b/indra/llinventory/tests/llsaleinfo_test.cpp index c41d938013..7faf5c83fa 100644 --- a/indra/llinventory/tests/llsaleinfo_test.cpp +++ b/indra/llinventory/tests/llsaleinfo_test.cpp @@ -1,5 +1,5 @@ /** - * @file LLSaleInfo_tut.cpp + * @file llsaleinfo_test.cpp * @author Adroit * @date 2007-03 * @brief Test cases of llsaleinfo.h @@ -140,7 +140,8 @@ namespace CHECK(ret_type == LLSaleInfo::FS_ORIGINAL); LLSaleInfo saleinfo(LLSaleInfo::FS_COPY, sale_price); - const char* result = LLSaleInfo::lookup(LLSaleInfo::FS_COPY); + const char* result = LLSaleInfo::lookup(saleinfo.getSaleType()); + REQUIRE(result != nullptr); CHECK(std::string_view(result) == "copy"); } @@ -168,12 +169,18 @@ namespace CHECK(saleinfo != saleinfonotequal); } - TEST_CASE("llsaleinfo: placeholder for the untested message packing", "[llsaleinfo]") + // The LLSD forms of packMessage and unpackMessage. The LLMessageSystem + // forms need a message system built from the message template, which + // this test does not have. + TEST_CASE("llsaleinfo: the LLSD packMessage and unpackMessage round-trip", "[llsaleinfo]") { + LLSaleInfo saleinfo(LLSaleInfo::FS_CONTENTS, 4321); + LLSD message = saleinfo.packMessage(); - //TBD: void LLSaleInfo::packMessage(LLMessageSystem* msg) const - //TBD: void LLSaleInfo::unpackMessage(LLMessageSystem* msg, const char* block) - //TBD: void LLSaleInfo::unpackMultiMessage(LLMessageSystem* msg, const char* block, S32 block_num) - } + LLSaleInfo saleinfo1; + saleinfo1.unpackMessage(message); + CHECK(saleinfo1.getSaleType() == LLSaleInfo::FS_CONTENTS); + CHECK(saleinfo1.getSalePrice() == 4321); + } } diff --git a/indra/llinventory/tests/llsettingssky_test.cpp b/indra/llinventory/tests/llsettingssky_test.cpp index a775f5a18b..a7b24d224f 100644 --- a/indra/llinventory/tests/llsettingssky_test.cpp +++ b/indra/llinventory/tests/llsettingssky_test.cpp @@ -146,6 +146,7 @@ namespace LLSettingsSky::ptr_t half = make_sky(from); half->blend(end, 0.5); LLSD at_half = half->getAbsorptionConfigs(); + REQUIRE(at_half.size() == size_t(2)); require_close("boundary at 0.5", term(at_half[0], LLSettingsSky::SETTING_DENSITY_PROFILE_WIDTH), 25000.0f); require_close("below the boundary at 0.5", term(at_half[0], LLSettingsSky::SETTING_DENSITY_PROFILE_CONSTANT_TERM), 0.3f); require_close("above the boundary at 0.5", term(at_half[1], LLSettingsSky::SETTING_DENSITY_PROFILE_CONSTANT_TERM), 0.5f); @@ -175,6 +176,7 @@ namespace sky->validate(); LLSD stored = sky->getAbsorptionConfigs(); + REQUIRE(stored.size() == size_t(2)); require_close("the layer after the nested array is clamped", term(stored[1], LLSettingsSky::SETTING_DENSITY_PROFILE_CONSTANT_TERM), 10.0f); LLSD flat = LLSettingsSky::flattenDensityProfile(stored); diff --git a/indra/llinventory/tests/lluserrelations_test.cpp b/indra/llinventory/tests/lluserrelations_test.cpp index 5520fa62f8..1aedc3dca7 100644 --- a/indra/llinventory/tests/lluserrelations_test.cpp +++ b/indra/llinventory/tests/lluserrelations_test.cpp @@ -1,5 +1,5 @@ /** - * @file lluserrelations_tut.cpp + * @file lluserrelations_test.cpp * @author Phoenix * @date 2006-10-12 * @brief Unit tests for the LLRelationship class. @@ -104,11 +104,4 @@ namespace mRelationship.online(false); CHECK_FALSE(mRelationship.isOnline()); } - -/* - template<> template<> - void user_relationship_object_t::test<>() - { - } -*/ } diff --git a/indra/llxml/tests/alxmllayermerge_test.cpp b/indra/llxml/tests/alxmllayermerge_test.cpp index 05865cbfbe..9a1a43aa99 100644 --- a/indra/llxml/tests/alxmllayermerge_test.cpp +++ b/indra/llxml/tests/alxmllayermerge_test.cpp @@ -382,11 +382,10 @@ namespace CHECK(installed.roots == 1); ALXmlLayerMerge::setDefaultObserver(nullptr); - RecordingObserver silent; LLXMLNodePtr again; CHECK(ALXmlLayerMerge::load({ base, overlay }, again)); UNSCOPED_INFO("and hears nothing once it is gone"); - CHECK(silent.roots == 0); + CHECK(installed.roots == 1); } } diff --git a/indra/llxml/tests/llcontrol_test.cpp b/indra/llxml/tests/llcontrol_test.cpp index 96b27731e4..902b3df95c 100644 --- a/indra/llxml/tests/llcontrol_test.cpp +++ b/indra/llxml/tests/llcontrol_test.cpp @@ -1,5 +1,5 @@ /** - * @file llcontrol_tut.cpp + * @file llcontrol_test.cpp * @date February 2008 * @brief control group unit tests * @@ -44,7 +44,7 @@ namespace std::string mTestConfigDir; std::string mTestConfigFile; std::vector mCleanups; - static bool mListenerFired; + bool mListenerFired = false; control_group() { mCG.reset(new LLControlGroup("foo")); @@ -80,16 +80,8 @@ namespace } file.close(); } - static bool handleListenerTest() - { - control_group::mListenerFired = true; - return true; - } }; - bool control_group::mListenerFired = false; - - //load settings from files - LLSD TEST_CASE_METHOD(control_group, "control_group: loadFromFile reads the settings in an LLSD file", "[control_group]") { @@ -143,7 +135,8 @@ namespace int results = mCG->loadFromFile(mTestConfigFile.c_str()); UNSCOPED_INFO("number of settings"); REQUIRE(results == 1); - mCG->getControl("TestSetting")->getSignal()->connect(boost::bind(&this->handleListenerTest)); + mCG->getControl("TestSetting")->getSignal()->connect( + [this](LLControlVariable*, const LLSD&, const LLSD&) { mListenerFired = true; }); mCG->setU32("TestSetting", 13); CHECK(mListenerFired); } From 4b5331b01269412eaaedfe5e9db1c245e4606a83 Mon Sep 17 00:00:00 2001 From: Rye Date: Mon, 12 Oct 2026 03:04:17 -0400 Subject: [PATCH 3/3] llprimitive's, llimage's and lldir's tests check what they say they check - llgltfmaterial: - the texture-transforms-only material was given texture ids, so its JSON round trip was never tested; it is given transforms; - the block meant to show that flagged default overrides put a non-default material back set the override material twice and left the render material default. It sets the render material. A third block shows that the same defaults without the flags change nothing; - a comment pointed at "test<12>"; it names the case. - llmaterial: the case named for fromLLSD invalidating the hash cache could not fail for that reason: the cache was dirty already when fromLLSD ran, and its last check went through a setter. The hash is cached before each fromLLSD now, and the second goes through LLSD. - llprimitive: createPrimitive's result leaked; a unique_ptr holds it. - llimageworker: the responder's done flag was a plain bool written on a worker thread and polled on the main one, through a pointer into the test's stack that a responder finishing after the timeout would write through. It is an atomic the responder shares ownership of. - lldir: comments in the directory scan case named directories and patterns the code does not use. Each new or corrected check was shown to fail under a small change to the test's own input or expected value. llprimitive_tests passes 335 assertions in 45 cases (333 before), and with llprimitive_test, llimageworker_test and llfilesystem_tests passes in 10 random orders. Co-Authored-By: Claude Opus 5.5 --- indra/llfilesystem/tests/lldir_test.cpp | 10 +++--- indra/llimage/tests/llimageworker_test.cpp | 24 ++++++++----- .../llprimitive/tests/llgltfmaterial_test.cpp | 34 +++++++++++++++---- indra/llprimitive/tests/llmaterial_test.cpp | 10 +++--- indra/llprimitive/tests/llprimitive_test.cpp | 6 ++-- 5 files changed, 57 insertions(+), 27 deletions(-) diff --git a/indra/llfilesystem/tests/lldir_test.cpp b/indra/llfilesystem/tests/lldir_test.cpp index adc8c793a6..880d657c5d 100644 --- a/indra/llfilesystem/tests/lldir_test.cpp +++ b/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,7 +497,7 @@ 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); @@ -505,7 +505,7 @@ namespace 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); diff --git a/indra/llimage/tests/llimageworker_test.cpp b/indra/llimage/tests/llimageworker_test.cpp index ec27e2e1cc..ef75cc4205 100644 --- a/indra/llimage/tests/llimageworker_test.cpp +++ b/indra/llimage/tests/llimageworker_test.cpp @@ -36,6 +36,9 @@ // Catch2 header #include +#include +#include + // ------------------------------------------------------------------------------------------- // 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> 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> 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>(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()); } } diff --git a/indra/llprimitive/tests/llgltfmaterial_test.cpp b/indra/llprimitive/tests/llgltfmaterial_test.cpp index 6d7c9f1a9b..c5bb81dcfa 100644 --- a/indra/llprimitive/tests/llgltfmaterial_test.cpp +++ b/indra/llprimitive/tests/llgltfmaterial_test.cpp @@ -135,8 +135,9 @@ namespace // written. The previous sizeof(LLGLTFMaterial) tripwire was removed when // the canonical hash packing was added — total class size now varies with // the hash snapshot/cache members (which aren't part of the hashed - // payload), so it stopped being a useful regression check. test<12> - // exercises the actual hash invariant directly. + // payload), so it stopped being a useful regression check. The case + // "getHash covers every field but the local texture tracking map", at the + // end of this file, exercises the actual hash invariant directly. TEST_CASE("llgltfmaterial: there are four texture infos", "[llgltfmaterial]") { CHECK((U32)LLGLTFMaterial::GLTF_TEXTURE_INFO_COUNT == 4); @@ -232,7 +233,7 @@ namespace { LLGLTFMaterial texture_transforms_only_material; - apply_test_material_texture_ids(texture_transforms_only_material); + apply_test_material_texture_transforms(texture_transforms_only_material); require_json_round_trip("material with texture transforms only", texture_transforms_only_material); } @@ -277,22 +278,41 @@ namespace CHECK(render_material == override_material); } { + // The default values, flagged as overrides, set a non-default + // material back to the defaults LLGLTFMaterial override_material; override_material.setAlphaMode(LLGLTFMaterial::ALPHA_MODE_OPAQUE, true); override_material.setDoubleSided(false, true); LLGLTFMaterial render_material; - override_material.setAlphaMode(LLGLTFMaterial::ALPHA_MODE_BLEND, false); - override_material.setDoubleSided(true, false); + render_material.setAlphaMode(LLGLTFMaterial::ALPHA_MODE_BLEND, false); + render_material.setDoubleSided(true, false); render_material.applyOverride(override_material); - // Not interested in these flags for equality comparison + // applyOverride takes the values, not the override flags override_material.mOverrideDoubleSided = false; override_material.mOverrideAlphaMode = false; - UNSCOPED_INFO("default overrides applied over non-defaults"); + UNSCOPED_INFO("flagged default overrides applied over non-defaults"); CHECK(render_material == override_material); } + { + // Without the flags, the same default values are no override at + // all and leave a non-default material alone + LLGLTFMaterial override_material; + override_material.setAlphaMode(LLGLTFMaterial::ALPHA_MODE_OPAQUE, false); + override_material.setDoubleSided(false, false); + + LLGLTFMaterial render_material; + render_material.setAlphaMode(LLGLTFMaterial::ALPHA_MODE_BLEND, false); + render_material.setDoubleSided(true, false); + const LLGLTFMaterial before_override = render_material; + + render_material.applyOverride(override_material); + + UNSCOPED_INFO("unflagged default values applied over non-defaults"); + CHECK(render_material == before_override); + } } // Test application of texture overrides diff --git a/indra/llprimitive/tests/llmaterial_test.cpp b/indra/llprimitive/tests/llmaterial_test.cpp index bf7b61fee3..02cb76f8dd 100644 --- a/indra/llprimitive/tests/llmaterial_test.cpp +++ b/indra/llprimitive/tests/llmaterial_test.cpp @@ -220,16 +220,18 @@ namespace const LLSD sd = m.asLLSD(); // Roundtripping the same LLSD should restore the same hash. LLMaterial back; - back.setAlphaMaskCutoff(99); // make cache dirty + state altered + back.setAlphaMaskCutoff(99); // state altered + CHECK(back.getHash() != before); // and its hash cached back.fromLLSD(sd); CHECK(back.getHash() == before); // Now perturb a field through LLSD and check the hash moves. LLMaterial perturbed; perturbed.fromLLSD(sd); - CHECK(perturbed.getHash() == before); - perturbed.setEnvironmentIntensity(42); - UNSCOPED_INFO("a setter after fromLLSD still invalidates"); + CHECK(perturbed.getHash() == before); // caches the unperturbed hash + LLSD perturbed_sd = sd; + perturbed_sd["EnvIntensity"] = 42; + perturbed.fromLLSD(perturbed_sd); CHECK(perturbed.getHash() != before); } diff --git a/indra/llprimitive/tests/llprimitive_test.cpp b/indra/llprimitive/tests/llprimitive_test.cpp index 71f91ba2a9..305f9f6691 100644 --- a/indra/llprimitive/tests/llprimitive_test.cpp +++ b/indra/llprimitive/tests/llprimitive_test.cpp @@ -28,6 +28,8 @@ #include +#include + #include "../llprimitive.h" #include "../../llmath/llvolumemgr.h" @@ -164,8 +166,8 @@ namespace TEST_CASE_METHOD(llprimitive, "LLPrimitive: createPrimitive makes a primitive with the PCode", "[LLPrimitive]") { LLPCode code = 1; - LLPrimitive * primitive = LLPrimitive::createPrimitive(code); - REQUIRE(primitive != NULL); + const std::unique_ptr primitive(LLPrimitive::createPrimitive(code)); + REQUIRE(primitive != nullptr); CHECK(primitive->getPCode() == code); }