[RyeMutt]

Description

Fixes the bugs in the small libraries' own tests that the Catch2 conversion in #449 found and kept, since a conversion must not change what a test does. Each was a test that could not fail for the reason it existed, checked something other than what it said, could crash where it should fail, or leaked. None of the corrected tests found a bug in the code they test.

It stacks on #449 (rye/tests-tm18) and changes only test files.

llcharacter (d241423)

• lljoint: after addChild, a check repeated an earlier one. It now checks the parent it meant.
• llcharacter: a joint's parent, and a motion findMotion found, were dereferenced unchecked. Each is now REQUIREd first.
• llpose: a case said an unnumbered joint is left out of the motion's joint signature, but never looked. The test motion now exposes the signature, and the case checks that no byte of it is set.
• 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.
• llbvhloader: results were indexed with no joint count checked; each is now REQUIREd. A case said every joint gets the global priority but looked at one. Two refusals shared a message.
• llkeyframemotion: a U32 cursor was compared through LLSDEquals; it is now compared as an integer.

llxml and llinventory (741c56e)

• alxmllayermerge: "hears nothing once it is gone" checked an observer that was never installed, so it could not fail.
• llcontrol: the listener flag was a static that was never reset. It now belongs to the fixture.
• llpermissions:
  • the allowCopyBy case called allowModifyBy;
  • the three allow*By cases were asked only under the default masks, where everyone may. They now give the owner, the group and everyone masks of their own in turn, and ask the owner, a member asking as the group, the same member without it, and a stranger;
  • a transfer value was read and thrown away. It is now checked.
• inventorymisc: three fromLLSD results were never checked. An empty placeholder case for a removed XML export goes.
• llsaleinfo:
  • a sale info was built and never used;
  • the message-packing placeholder becomes a round trip through the LLSD forms of packMessage and unpackMessage.
• llsettingssky: two profiles were indexed with no size checked.
• Cleanup: a commented-out TUT stub goes, and six files' @file lines lose their TUT-era names.

llprimitive, llimage and lldir (4b5331b)

• llgltfmaterial:
  • the transforms-only material was given texture IDs, so its round trip was never tested;
  • the flagged-default-overrides block set the override material twice and left the render material at its defaults. It now sets the render material, and a new block checks that the same defaults without the flags change nothing.
• llmaterial: the case named for fromLLSD invalidating the hash cache could not fail for that reason. The cache was already dirty when fromLLSD ran.
• llprimitive: createPrimitive's result leaked.
• llimageworker: the responder's done flag was a plain bool written on a worker thread and polled on the main one. It pointed into the test's stack, and a responder finishing after the timeout would write through that pointer. It is now an atomic, which the responder shares ownership of.
• lldir: comments named directories and patterns the code does not use.

Related Issues

• Please link to a relevant GitHub issue for additional context.

Issue Link: none. The bugs were found by #449's idiom pass and listed in 3bbc58d's message.

---

Checklist

• I have provided a clear title and detailed description for this pull request.
• If useful, I have included media such as screenshots and video to show off my changes.
• I have tested the changes locally and verified they work as intended.
• All new and existing tests pass.
• Code follows the project's style guidelines.
• Documentation has been updated if needed.
• Any dependent changes have been merged and published in downstream modules.
• I have reviewed the contributing guidelines.

---

Additional Notes

• Tested on macOS arm64, RelWithDebInfo, on Test framework, stage 3: the small libraries' tests in Catch2 #449's head (fac126b):
  • all 38 entries of TM18's eight binaries pass;
  • each binary passes 10 random orders with no state guard report;
  • assertions changed only where described: llcharacter 38,083 → 38,119; llinventory 356 in 58 cases → 378 in 57; llprimitive 333 → 335.
• Each new or corrected check fails when it should. Each was run under a small change to the test's own input or expected value; it failed, and nothing else did. The changes were then reverted.
  • The exception is inventorymisc's parse checks: LLInventoryItem::fromLLSD returns true whatever it is given.
• Merge order: this should follow Test framework, stage 3: the small libraries' tests in Catch2 #449. Once Test framework, stage 3: the small libraries' tests in Catch2 #449 merges, its base becomes develop.
• Not changed here, found on the way: LLGLTFMaterial::applyOverride calls mTrackingIdToLocalTexture.insert(it, it). That is std::map's range insert over an empty range, so an override's local-texture tracking is never merged in. It is production code and wants its own change.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 3 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 23ed7be6-b5d6-44ca-82fb-59a7555d0abb

---

📥 Commits

Reviewing files that changed from the base of the PR and between 4ddc9d0 and 74800ec.

---

📒 Files selected for processing (20)

• indra/llcharacter/tests/altestmotion.h
• indra/llcharacter/tests/llbvhloader_test.cpp
• indra/llcharacter/tests/llcharacter_test.cpp
• indra/llcharacter/tests/lljoint_test.cpp
• indra/llcharacter/tests/llkeyframemotion_test.cpp
• indra/llcharacter/tests/llmultigesture_test.cpp
• indra/llcharacter/tests/llpose_test.cpp
• indra/llfilesystem/tests/lldir_test.cpp
• indra/llimage/tests/llimageworker_test.cpp
• indra/llinventory/tests/inventorymisc_test.cpp
• indra/llinventory/tests/llparcel_test.cpp
• indra/llinventory/tests/llpermissions_test.cpp
• indra/llinventory/tests/llsaleinfo_test.cpp
• indra/llinventory/tests/llsettingssky_test.cpp
• indra/llinventory/tests/lluserrelations_test.cpp
• indra/llprimitive/tests/llgltfmaterial_test.cpp
• indra/llprimitive/tests/llmaterial_test.cpp
• indra/llprimitive/tests/llprimitive_test.cpp
• indra/llxml/tests/alxmllayermerge_test.cpp
• indra/llxml/tests/llcontrol_test.cpp

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

---

_{Comment @coderabbitai help to get the list of available commands.}