Repository navigation
Test framework, stage 1: every test ready to share a binary - #436
Merged
Merged
Conversation
Seven unit tests compiled the file they test in place of the library, and seven more listed one of the library's files in SOURCES while linking the library as well, so llquaternion.cpp, for one, was built three times. Every llmath test now links llmath and compiles only its own test file, so the code under test is the code the viewer runs, and two tests can share a binary without defining the same symbols twice. The unit tests' targets are INTEGRATION_TEST_<name> now, like the rest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…their own llcommon's lluuid test registers a TUT group named "LLUUID" too, and TUT throws on a second group of one name from a static initializer, so the two in one binary would end before main. mathmisc's is "mathmisc LLUUID" now. llsdutil_math's group object was tut::tutil, the name llcommon's llsdutil test gives its own. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Test 10 calls set_thread_fp_mode(), as the render thread does, to check a texture index carried in w as a denormal survives flush-to-zero. That mode is the main thread's MXCSR or FPCR, and nothing reset it, so every test run after it in the same process would flush denormals too. A guard in the test saves the control register and writes it back when the test ends, passed or failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each test's TUT data struct, group object and file-local helpers sit in an anonymous namespace inside tut, so the test<N> bodies TUT instantiates for them are internal too. Two tests in one binary could otherwise share a fixture name and run each other's code with no diagnostic. llmatrix4a's same_bytes and ensure_matrix_close were strong and external, and llrender's llrendermatrix test defines both; mathmisc's radius globals and alignment's aligned types were external as well. The test<N> specializations stay in tut, where TUT declares the template. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lxmldocument.cpp As a unit test it built its own copies of two of llxml's files and linked pugixml for them. It links the library now, like llxml's other tests, so it tests the code the viewer runs and could share a binary with them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every fixture deleted and re-created <temp>/alxmllayermerge_test, so two runs of the test at once, as parallel CI shards or a per-file and a combined run would be, deleted each other's files. The folder's name now carries a fresh UUID, as llcontrol's test already does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each test's TUT data struct, group object and helpers, alxmllayermerge's RecordingObserver among them, sit in an anonymous namespace inside tut, so the test<N> bodies TUT instantiates for them are internal too, and no two tests sharing a binary can meet over a name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
It listed lljoint.cpp in SOURCES and linked llcommon and llmath, so it tested its own build of the file. It links llcharacter now, as the rest of llcharacter's tests do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each test's TUT data struct and group object, and llpose's joints base that four of its groups share, sit in an anonymous namespace inside tut, so the test<N> bodies TUT instantiates for them are internal too. The shared ALTestCharacter and ALTestMotion headers keep their one inline definition each. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…le they test llmediaentry, llgltfmaterial, llmaterialid and llmaterial each built their own copy of the file under test. They link the library now; the one tinygltf implementation they need still comes from llgltfmaterial's test. llprimitive's own test and llimageworker's stay unit tests, since they stub classes their libraries define, and their CMake says so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each test's TUT data struct, group object and helpers sit in an anonymous namespace inside tut, and llprimitive's DummyVolumeMgr and PRIMITIVE_TEST_SETUP in one at file scope, so nothing a test defines for itself is visible to another test in the same binary. llmediaentry's ensure_*, whitelist_test and URL constants were strong tut:: symbols; two of those helpers were never called, which the compiler now says, and they are gone. llprimitive's stubs of library members stay as they were. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rator.cpp As a unit test it built its own copy of the file it tests. It links the library now, like lldir's test, so the two could share a binary. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lldiriterator's group object was the strong tut::tf and its CHOP-662 check the strong tut::test_chop_662(); lldir's makeTestFile, makeTestDir and scanTest were strong tut:: functions, and its LLDir_Dummy and LLDir_Relocated global types. All of them, with each TUT data struct and group, sit in anonymous namespaces now. lldir still gets a strong operator<< for vectors of strings from llcommon's StringVec.h, which is llcommon's to make inline. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Tests 7 and 13 wrote linden_file.dat into the working directory and never removed it, so two runs at once wrote over each other's file and the build tree kept it. Each uses a NamedTempFile now, removed when the test ends. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each test's TUT data struct and group object, inventorymisc's strong tut::inv among them, sit in an anonymous namespace inside tut, and inventorymisc's create_random_inventory_item, create_random_inventory_cat and set_random_inventory_metadata, strong global functions, in one at file scope. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The responder class and the TUT data struct and group object sit in an anonymous namespace inside tut. The image class stubs stay external: they stand in for libllimage's members, which is why the test is still a unit test of its own. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
As a unit test it built its own copy of the file and named the libraries that copy needed. It links the library now, which brings those with it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…wer name The fake XMLRPC responder was a global class named LLXMLRPCListener, the name of a different class in newview's llxmlrpclistener.h; in one link with that, one would quietly have replaced the other's inline members and vtable. It is FakeXMLRPCListener now, and it, LoginListener and the TUT data struct and group sit in anonymous namespaces. The fixture still makes an LLTestApp and clears the event pumps as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ne name differently Each test is its own link today, so a duplicate never meets its twin; the moment two of these tests share a binary, each of the following is a link error or, worse, links and runs the other test's code. - The operator<< in StringVec.h and listener.h is inline, as a header's definitions must be. apply_test drops its own copy for StringVec.h's. - The three test groups named tut_singleton (llsingleton, lltrace, llunits) take their own names. - lleventcoro and lleventfilter each had a function template tut::test<PUMP> with a body of its own, instantiated for the same pumps: one group's tests 6 and 7 would have run the other's. Each is now static and says what it checks. - classic_callback and llpounceable each defined a ::Data of a different layout, and their other helpers, at global scope: they move into an anonymous namespace. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Only the names TUT needs stay external: each group's data struct, its test group object and the test<N> specialisations. Every other type, function and variable a test file defines for itself moves into an anonymous namespace or becomes static, so two tests in one binary can no longer clash at link time or, silently, share one weak definition of a same-named type. The helpers of the shared headers (listener.h, wrapllerrs.h, the indra/test ones) keep one inline definition each. - llerror keeps TestAlpha, TestBeta, logFromGlobal and Foo::logFromNamespace as they are: log filtering matches their qualified names, and the global function is what tells it apart from the static and anonymous ones. - An anonymous namespace does not make a type's name unique: LockStatic, LLSingleton and LLInstanceTracker find their statics by typeid(T).name(), the same for every TU's (anonymous namespace)::X. llmainthreadtask's LockStatic key, a generic StaticData, takes a name of its own. - lleventcoro's data struct, the generic tut::test_data, is lleventcoro_data. - Internal linkage shows what nothing calls: llevents' chainEvents and llunits' accept_implicit_quatloos go; lltreeiterators' fallback strings and node printer, there for templates nothing instantiates, are [[maybe_unused]]. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…umps it
A function-static WorkQueue("mainloop") lived from the first test that
pumped to the end of the process. In a binary shared with other tests it
would stand in for the viewer's main loop for all of them: a second
"mainloop" queue anywhere is LL_ERRS, workers that hand back to the main
loop would post into it, and almasterwatch test 11 fails once one exists.
The fixture now makes the queue the first time a test pumps and lets it go
with the test. The watched files go before the fixture, and with the last
of them the watcher's worker is joined, so nothing posts into a dead queue.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each of these left something behind that a later test in the same process would read, or crash on. Each is now restored as the test or its fixture ends, however it ends. - lltrace's fixture ThreadRecorder made itself the main thread's recorder and left it, its timer stack and the current accumulators null: a later Recording on the main thread dereferenced null. They are put back. - llevents left listeners bound to dead fixtures and stack objects on per-frame, login, filter0 (disabled too), filter1 and button. The fixture now deletes the pumps its tests obtain, with their listeners, so each test also finds them new: test 1, which counts the pumps obtain() adds, no longer needs to run before test 2. - llprocess test 24 left its PostendListener on postend; tests 22 and 23 run after it crashed. The listener now holds its connection. - llhttpdate test 4 restored LC_TIME and LLStringUtil's locale only on success, and never the locale LLDate had last set LC_TIME for, so the next date formatted anywhere set LC_TIME to the environment's. A guard restores all three. - lleventcoro built and destroyed an LLApp in every fixture, and ~LLApp ends LLCommon: the APR pool and the LLTrace master recorder were null for every test after it. One app now serves the process and is never destroyed; each test still sees it start running and stop. - llpounceable test 1 left gForward and static_check pointing at a dead local. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… peers write no bytecode - llsd checked LLSD's live and allocated counts in the destructors of SDCleanupCheck and SDAllocationCheck. A destructor is noexcept, so a failed ensure() there ended the process rather than failing the test. Each check now runs the test's body and checks once what it made is gone, in the same order and with the same messages. - llleap's peers import a reader module the test writes to the temp folder, and each run left its bytecode behind in __pycache__ there. The three Python peer tests run with PYTHONDONTWRITEBYTECODE=1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…them - llinitparam test 9 checks that a choice block starts with its first alternative chosen. Only the first block of a type does, the one whose construction builds the descriptor, so test 9 passed only while nothing had built a ChoiceOfThings before it; test 19 builds one. It now has a type of its own. - llfile test 3 wrote into the folder test 2 made, and test 4 needed the file test 3 wrote. Each now makes what it needs. The folders were fixed paths in the temp folder that test 2 left behind; each test now has its own, named by a UUID, and its fixture removes them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both llsdutil_test and llmath's llsdutil_math_test named their test group object tut::tutil, a duplicate symbol the moment the two share a binary. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Nearly every alscript test already kept its helpers in an anonymous namespace or in its fixture; two did not. alincludesearch's RequireCase moves into one, and alpreprocessor's gRuns is static. alincludesearch's fixture keeps its name and namespace: alincludesearch.h befriends tut::alincludesearch_data. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ran before them - alpreprocessor tests 39 and 40 need the grid's builtins, for the optimizer and for const folding, and passed only because test 13 had loaded them into Tailslide's table, which keeps them for the process. Every test that needs them now asks through one helper. - alluauconfig test 9 times a run stopped by the deadline, and such a run is kept a while by its text: the same text asked in the process in the last ten seconds, by another test or by this one run again, answered at once and failed the test. Its text is now its own, each run. - allslservice test 21 wrote its region's builtins to a fixed name in the temp folder; the name is now its own. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The test answers llSay with a stand-in by patching the process's script library, and put the real one back only if the run returned. A guard now restores it, and holds the stand-in for the run alone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
llurlentry.cpp asks the viewer for an anonymised avatar name, so every binary that pulls it needs a definition, and 75 tests and two benchmarks each carried one, alquickopen a second, dead one inside namespace tut. Two of them in one binary would not link. They go, for one definition in llui_test_fakes.cpp, an object library compiled once and linked into every test and benchmark that links llui. alquickopen also loses the includes it had twice. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ALTextView befriends ll_test::TextViewProbe by name, and altextview, alcodeeditor and alvimkeymap each defined it, with different members. In one binary the linker keeps one copy of an inline function, so a test would run the other file's probe without a word said. altextviewprobe.h now holds the one definition, the union of the three, and each test includes it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Important Review skippedToo many files! This PR contains 291 files, which is 191 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configuration
📒 Files selected for processing (291)
You can disable this status message by setting the
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. Comment |
Stage 1's own changes broke two tests that only Windows and Linux build, and this Mac's clang did not object to: - llsd's tests now run their bodies inside the lambdas they hand ensure_cleaned_up and ensure_allocations, and MSVC does not find SDTestData's static helpers, ensureTypeAndValue and ensureTypeAndRefValue, by their bare names there. They are static function templates in tut now, not members. Not in the anonymous namespace: TUT declares its ensure_equals there, and from inside it that would hide lltut.h's overloads for LLSD's binary type. - lleventdispatcher's Vars is internal now, and GCC's -Wunused-function, an error under -Werror, takes in an internal class's out-of-line member that nothing calls. Vars::smethod1 is kept with its set, marked [[maybe_unused]]. Clang's -Wunused-member-function, run over every test file this branch changes, finds no other out-of-line member nothing calls. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
0cdb004 deleted LLWebsocketMgr as each test ended, so that a test that failed before removing its servers would not leave them listening into the next. On Linux x64 CI the suite then ended the binary: std::terminate inside it, with the runtime_error test 16's handlers throw as the current exception. That run was the only one to show it: macOS and Linux arm64 pass, and so did every Linux x64 run of the suite before, with the manager living for the process as it does again now. Why deleting a manager whose servers are already stopped and joined should do that is not found yet. A failed test leaving its servers listening is the lesser harm. The suite's teardown is left for the stage that converts llcorehttp, which can look for it with the crash in hand. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
For the fix to alluaufragment test 2's flake. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nary on Linux x64 is understood On Linux x64 CI, and only there, the llcorehttp test binary ended twice in the WebSocket manager suite with std::terminate, the runtime_error test 16's handlers throw being the exception named. Branches on the same develop pass it, and so did every Linux x64 run before this branch; macOS and Linux arm64 pass it here. Taking back the per-test manager deletion (994a5fb) did not change it, which leaves this branch's other change to the binary: its suites, fixtures and helpers moved into anonymous namespaces (56f2e09). Both of the handler's callers catch std::exception, and no library defines a name those helpers had, so how internal linkage would do it is not found. Until it is, llcorehttp's tests are exactly develop's, so this branch changes nothing that binary is built from; their linkage is left for the stage that converts llcorehttp, with this crash in hand. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…42.1 On Linux x64 CI the llcorehttp test binary ended in std::terminate as test 16's async handler threw: the runtime_error was thrown with a catch for it two frames up, and the unwinder found no unwind entry (FDE) for the handler, so it never looked. GCC 15 split the cold blocks of Beast's buffers_cat_view iterator off into "[clone .cold]" parts that then optimised away, and emitted five of them empty, each with an FDE covering no bytes, at the address of what followed them in .text.unlikely: the handler's invoker, which only throws and so is cold whole. mold before 2.42.1 (rui314/mold#1648) puts those FDEs in .eh_frame_hdr's search table too, sorted on their address alone by an unstable parallel sort, so the six tie in an order of chance. libgcc takes the last entry at or before the PC; where that is an empty one it reports no FDE, phase one of the unwind ends, and __cxa_throw calls std::terminate. Develop's object has the same five empty FDEs at the same function. Its link happened to sort the real one last; the stage 1 branch's anonymous namespaces change which FDEs the link sorts, and wrapping test_httprequest.hpp's alone sorts an empty one last. Reordering only the tied table entries of the two binaries turns the abort on in develop's and off in the branch's. Without splitting there are no empty parts. Built with this under GCC 15.2 and mold 2.40.4, 994a5fb's llcorehttp binary has no zero-length FDE, its WebSocket suite passes five runs in five, and the whole binary passes under its peer. It applies only where CMake finds a mold older than 2.42.1; from 2.42.1 mold writes such entries as tombstones no search lands on. The vcpkg ports keep their empty parts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018TgkpVXucG17x8kCrsXMjy
…2.40 mold before 2.42.1 puts the unwind entries GCC emits for empty cold parts into .eh_frame_hdr's search table, where one can stand in for the real entry of the function at the same address; an exception thrown through that function then ends the program in std::terminate (rui314/mold#1648). It took the llcorehttp test binary down on Linux x64, and could do the same to any function of the viewer or of a vcpkg port, which the GCC flag the build sets for its own code under such a mold does not reach. The release tarball, pinned by digest for x86-64 and arm64, goes under /usr/local, ahead of the image's on the path: GCC's -fuse-ld=mold and CMake's linker detection both find it, so the build sees 2.42.1 and leaves hot/cold splitting on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018TgkpVXucG17x8kCrsXMjy
66bba8b took them back out while the Linux x64 crash in their binary was unexplained. It was never their linkage: mold before 2.42.1 sorts the empty unwind entries GCC 15 leaves at the async handler's invoker into .eh_frame_hdr beside the real one, in an order of chance, and develop's link only escaped by its sort. 8121cab and d09b7ce take that away, and built with them the binary these namespaces make passes. This reverts commit 66bba8b. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018TgkpVXucG17x8kCrsXMjy
…again 994a5fb kept one manager for the run while the deletion was suspected of the std::terminate on Linux x64 CI. The crash came from the linker instead (8121cab), so a test that fails before removing its servers again leaves none listening into the next. This reverts commit 994a5fb. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018TgkpVXucG17x8kCrsXMjy
github-actions
Bot
added
the
github_actions
This was referenced Oct 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Stage 1 of the move from TUT to Catch2: hygiene under TUT. Like stage 0 (#428), it changes no test framework. It removes the reasons two test files could not share a binary. The last attempt at combined binaries met all three: link errors, behaviour silently wrong, and crashes at teardown.
Every test keeps its own executable here, so today's layout proves each change. Every binary reports the same TUT total before and after, and passes. No test checks anything different. Two production files change, each in a commit of its own (below).
The commits go a library at a time, a class of fix each, in this order: llmath, llxml, llcharacter, llprimitive, llfilesystem, llinventory, llimage and login; then llcommon, alscript and lscript; then llui; then llrender, llmessage and llcorehttp; then newview. The next 6 commits do the same for tests that landed on develop since the audit, and the last ones answer CI (below).
One definition of each helper
rlvGetAnonymis defined once for llui's tests (llui_test_fakes) and once for newview's (newview_test_fakes), in place of 77 copies.TextViewProbe. newview's share one settings group (newview_test_settings) and one switchable checking fake.tut::test<PUMP>of its own, and in one binary one group's tests 6 and 7 would have run the other's.Internal linkage
LLSingleton,LLInstanceTrackerorLLThreadLocalSingletonPointeralso has a name of its own across the tree, since those key ontypeid(T).name(), which two same-named anonymous types share.The real library, linked
lljoint,llxmlnodeandalxmldocument, llprimitive's,lldiriterator,lllogin,llnamevalueandllsdmessagebuilder, and all of llui's.llagentaccesstests over the realLLControlGroup, andllremoteparcelrequestover llui's own.Nothing done as the process starts
State put back, however the test ends, in each test or fixture that changes it:
gFloaterView, event pump listeners, LL'sllSay(allslreference), the login instance's notifications;What lies outside the process
Order
One context, one window
ll_test::sharedHeadlessGL(), with every part up. llui's GL tests share one hidden window, andHeadlessUIraises an error if it is asked for a different mode.ll_test::SharedGLScopereads what a test may change of the shared context and puts it back as the test ends, through gGL's and the shader's caches and behind them. The GL tests that landed since the audit hold the context through it:alinfiniteprojection_gl,alrenderstate_gl,llrendertarget_gl,alhuddepth_gl,alselectionoutline_gl.Production code
ALTextFind::closeWorker()closes the find worker and waits for it (6bd0b12). The tests that start it call it.Related Issues
Issue Link: none. It is the second stage of a planned test framework move; #428 was the first.
Checklist
Additional Notes
ninja-ospreset withAL_BUILD_TESTS=ON), on develop at 193bf34. The full build, viewer included, is clean, with no duplicate symbols. 404 of 405 tests pass.alluaufragmenttest 2, a flake on develop that A local declared again in its own scope is offered and called as the one in effect #438 fixed; develop is merged in, and it passes 12 runs of 12.llsd_test's data-struct helpers from inside the lambdas stage 1 put its bodies in, and GCC's-Wunused-functiontook inlleventdispatcher's now-internalVars::smethod1. e60a50e fixes both.std::terminate, naming the exception test 16's handlers throw (which both of their callers catch). Branches on the same develop pass it. Taking back the per-test manager deletion did not change it, so 66bba8b puts all of llcorehttp's tests back to develop's, and this PR changes nothing that binary is built from. Its internal linkage, and this crash, wait for the stage that converts llcorehttp.nmwas used instead), and newview's production files compiled into more than one test, which wait for one newview test binary.🤖 Generated with Claude Code