Skip to content

Test framework, stage 3: llmessage's and llcorehttp's tests in Catch2 - #455

Merged
RyeMutt merged 11 commits into
developfrom
rye/tests-tm19
Oct 12, 2026
Merged

Test framework, stage 3: llmessage's and llcorehttp's tests in Catch2#455
RyeMutt merged 11 commits into
developfrom
rye/tests-tm19

Conversation

@RyeMutt

@RyeMutt RyeMutt commented Oct 12, 2026

Copy link
Copy Markdown
Member

Description

The second wave of stage 3 of the test framework's move from TUT to Catch2 (TM19): llmessage's and llcorehttp's tests. 31 TUT files with 361 tests become five Catch2 binaries. Each test file still runs in a process of its own. It follows #449 and #450, now on develop, and builds on #449's harness and CMake changes.

Binary Files Notes
llmessage_tests 18 io_test's pipes (llpipeutil.cpp) are an OBJECT library beside them
llcoproceduremanager_test 1 alone: stubs parts of llmessage and llcorehttp, so compiles its file in (UNIT)
lltrustedmessageservice_test 1 the same
lltemplatemessagedispatcher_test 1 the same
llcorehttp_tests 10 run under the Python peer, test_llcorehttp_peer.py, as before

Two production fixes first, found by stage 1:

  • 49b444d, LLMessageSystem and start_net.
    • The leak: ~LLMessageSystem closed its socket only when the system was OK. The template loads before the socket opens, so a system whose template failed kept an open socket and leaked it.
    • The fix: "no socket" is -1 now, not 0, which is stdin's descriptor; the destructor closes whatever socket it has; and end_net closes only an open socket.
    • start_net now closes its socket on its two bind failures.
    • Tests: message_test checks both cases, and both checks fail without the fix. The viewer never noticed the leak: it quits when its message system fails to start.
  • 333b27a, LLMessageConfig::initClass with another directory.
    • The bug: the config file is an LLLiveFile, which watches the one path it was made with. A second initClass moved where it read from but not what it watched.
    • The fix: another directory now gets a new file object.
    • Header statics: the four static variables the header defined, one copy per file that included it, move into the .cpp, or go where nothing used them.
    • Tests: a new llmessageconfig case fails without the fix. The file's six-second reload test had only passed by accident: the first fixture deleted the watched file, which counted as a change.

Then the conversion:

  • 1c6a7d9, b26bb20, tut_parity.py:

    • it finds a TUT entry's executable, and a Catch2 entry's binary, by name wherever a launcher put it, instead of taking the command's first word, the Python interpreter for llcorehttp;
    • a TUT test that skipped before its set_test_name printed no name, and that is listed rather than a mismatch.

  • 625dfae, llcorehttp's ten suites become test files of their own, still under TUT. They were ten .hpp files included into one translation unit, and they now give exactly the same 88 results.

    • The helpers move into httprequest_test.cpp, their only user.
    • Four suites had compiled only because an earlier suite's using namespace LLCore came before them.

  • c334c14, tut2catch.py --names: a list names each #if twin of a test apart, here alnetimpairment's test<1>.

  • 5c7aa17, test/sync.h: it fails a timed-out wait the way its framework does. al_add_test_binary defines AL_TEST_CATCH2 for every Catch2 binary. llcommon's TUT tests use the same header until TM20.

  • ccb051c, al_add_test_binary(... LAUNCHER <command>...):

    • each file's entry and the combined run go through the launcher;
    • per-case mode gets it as the target's CROSSCOMPILING_EMULATOR, which Catch.cmake uses for discovery and each case. I checked that per-case llcorehttp cases run through the peer.

  • 5751fed, the conversion: tut2catch.py's output, with the names the idiom pass chose given through --names. No file carries a hand edit.

  • 0c58e8e, the idiom pass:

    • every case named for what it checks (233 were #N);
    • messages that only restated their assertion removed: 1,244 became 205, and 24 CAPTUREs replace hand-built strings;
    • asserting helpers named require_*;
    • 111 REQUIREs that guarded nothing are CHECKs, and 4 CHECKs that guard a read after them are REQUIREs.

    No case and no assertion was added, removed or moved.

  • ccf8e19, this wave's guard checks:

    • llmessage: gMessageSystem, and whether LLProxy exists;
    • llcorehttp: the HTTP service's instance and state, and whether the websocket manager exists.

    I broke a test for each on purpose, and each check named the case responsible.

Related Issues

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

Issue Link: none. This follows #449 (TM18).

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:
    • the full build is clean;
    • all 447 tests of ctest -LE manual pass;
    • all 8 pending combined runs pass;
    • each of the five TM19 binaries passes 10 random orders and 20 repeats, with no state guard report;
    • no anonymous type is used with a name-keyed registry in two files of one binary, and clang finds no unused internal member, which GCC would reject.
  • Parity: tut_parity.py holds all 31 files to the TUT executables' recorded results (355 ok, 6 skipped). It holds exactly at the conversion commit and under --recounted after the idiom pass.
  • Listed by parity, not failed: alnetimpairment's test for builds without AL_NET_IMPAIRMENT isn't compiled here, and five HttpRequest tests skip before naming themselves.
  • One change from the plan: there is no process-wide message system. Stage 1 gave each case a system of its own on an OS-assigned port, and the guard confirms each case puts gMessageSystem back.
  • Old test bugs the idiom pass found are listed in 0c58e8e's message and left as they were, for a follow-up like The small libraries' tests check what they say they check #450. For example:
    • a disallowed-header check is spelled so that it can never match;
    • the dispatcher tests dereference a null reader;
    • httpstatus assigns status1 twice where it meant status2.

🤖 Generated with Claude Code

RyeMutt and others added 11 commits October 12, 2026 07:03
@RyeMutt @claude
LLMessageSystem closes its socket whatever else failed, and start_net…
49b444d 
… closes one it cannot bind

~LLMessageSystem closed its socket only when the system was OK. But the
constructor loads the message template before it opens the socket, so a
system whose template failed to load had an open socket and was not OK,
and deleting it leaked the socket, and on Windows its WSAStartup. The
guard was there because a system whose start_net failed held socket 0,
stdin's descriptor, which end_net would have closed.

- No socket is -1 now, not 0, and the destructor closes whatever socket
  it has. end_net closes only a socket that is open, and leaves -1
  behind, so a second call does nothing. On Windows it undoes WSAStartup
  only for a socket start_net opened.
- start_net returned without closing its socket when it found no port to
  bind, or bind failed some other way. Both paths close it now, and on
  Windows undo WSAStartup, as the path where socket() itself fails did.

The viewer never noticed: it quits when its message system fails to
start. The tests did. llmessage's test fixture closed the socket itself,
working around the leak; it no longer needs to, and with the fix it
would have closed the same descriptor twice.

message_test checks both. A system made without its template frees its
port when it is deleted, so start_net binds that port again rather than
moving on into the discovery range. And on POSIX, start_net with every
port it would try taken returns 3 and leaves no descriptor open.
Without the fix, the first finds the port still taken and is moved to
13000, and the second finds one descriptor more.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@RyeMutt @claude
LLMessageConfig::initClass with another directory reads and watches t…
333b27a 
…hat directory's file

LLMessageConfigFile is an LLLiveFile, which watches the one path it was
made with, and the file was a function-local static, made on the first
initClass. A later initClass with another directory moved where loadFile
read from but not what was watched: the new directory's file was read
only when the first directory's file changed. The viewer calls initClass
once, so it never saw this; llmessage's tests call it from every
fixture, each with a directory of its own, and their reload test passed
only because the first fixture deleted its file, which counts as a
change.

The file is held in llmessageconfig.cpp now, and made anew when initClass
names another directory, so the file read is the file watched.
initClass with the same directory keeps the file, and what it loaded, as
before.

sServerName, sConfigDir, sServerDefault and sMessages were static
variables defined in the header, so every file including it had copies
of its own; only llmessageconfig.cpp's were used, and the last two
nowhere. The first two move into llmessageconfig.cpp, and the others go.

llmessageconfig_test checks it: initClass with a second directory gets
that directory's config at once. Without the fix it gets none.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@RyeMutt @claude
tut_parity.py finds a TUT entry's executable by its target's name, wh…
1c6a7d9 
…erever a launcher put it

record refuses results from an executable built before its source last
changed, and took the executable to be the first word of the entry's
command. llcorehttp's entry runs its executable under a Python peer, so
the first word is the interpreter, and record refused it whenever the
source was newer than the interpreter, which is always.

The executable is now the word in the command named for the entry's
target, INTEGRATION_TEST_<test> for an INTEGRATION_TEST_RUNNER_<test>
entry and the entry's own name otherwise, and the first word only where
none is. llcorehttp records (83 ok, 5 skipped), and the other entries
find what they found before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@RyeMutt @claude
llcorehttp's ten test suites are test files of their own
625dfae 
llcorehttp's tests were one TUT executable, built from llcorehttp_test.cpp,
which #included ten test_*.hpp suites into one translation unit and
defined the helpers the HttpRequest suite uses. A suite's results could
not be told from another's by the file they ran from, and each suite
compiled only because the ones before it had made its names visible.

Each suite is now <suite>_test.cpp, an executable of its own run under
the Python peer as before, so the stage 3 conversion and its parity
check treat them as any other test file:
- include guards go, and each file includes linden_common.h and lltut.h
  first, as the one file did before them;
- four suites used LLCore names that an earlier suite's using-directive
  had brought in, and say so themselves;
- init_curl, term_curl, ScopedCurlInit, get_base_url and stop_thread
  move into httprequest_test.cpp, the only suite that uses them, and
  llcorehttp_test.cpp and llcorehttp_test.h go.

The ten executables report exactly what the one did: the same 88 tests,
in the same ten groups, with the same results (83 ok, 5 skipped).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@RyeMutt @claude
tut_parity.py checks a launched Catch2 binary, and a test that skippe…
b26bb20 
…d before naming itself

Two things llcorehttp brought out:
- check refused results from a converted file newer than its binary,
  taking the binary to be the first word of the entry's command. A
  binary run under a launcher, as llcorehttp_tests runs under its Python
  peer, has the interpreter there. check now finds the binary by name,
  as record finds a TUT executable, through one helper.
- TUT prints a test's name only once its set_test_name has run. Five of
  HttpRequest's tests skip on this platform before they get there, so
  TUT printed no name, and check held that against the name the
  converter read from the set_test_name. Such a test, one that skipped or
  failed with no name printed, is listed rather than a mismatch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@RyeMutt @claude
tut2catch.py's --names gives each #if twin of a test a name of its own
c334c14 
--names keys a name by group and test number, so the two definitions of
one test<N> in the branches of an #if, as alnetimpairment's test<1> is
with and without AL_NET_IMPAIRMENT, took one name between them, and the
idiom pass could not name them apart: the compiled twin was given the
other's name.

A name may now be a list, one a twin in the order the file declares
them; a plain name is still every twin's. Over TM19's 31 files with
their names, only alnetimpairment's first case comes out differently,
and as its own name.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@RyeMutt @claude
test/sync.h fails a test the way its framework does, for TUT and Catc…
5c7aa17 
…h2 tests alike

Sync, which steps coroutines in turn, failed a timed-out wait with
tut::fail, and so included lltut.h, which a Catch2 test cannot build
with. llmessage's llcoproceduremanager test uses it and is converted in
TM19; llcommon's lleventcoro and llmainthreadtask use it and are TUT
until TM20.

al_add_test_binary defines AL_TEST_CATCH2 for every Catch2 binary, and
Sync FAILs where it is set and calls tut::fail where it is not. The TUT
branch goes with TUT (TM26).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@RyeMutt @claude
al_add_test_binary runs a binary's entries through a launcher, where …
ccb051c 
…one is given

llcorehttp's tests run under a Python peer, test_llcorehttp_peer.py,
which serves the requests they make and starts the test program itself
once its port is known. al_add_test's COMMAND said so; a Catch2 binary
had no way to.

LAUNCHER names a command each entry runs the binary through, the binary's
path and arguments after the command's own: each file's entry and the
combined run. stray_cases runs no case, so it runs the binary bare. In
per-case mode, Catch.cmake runs the discovery and each case through the
target's CROSSCOMPILING_EMULATOR, which CMake's own add_test leaves
alone in a native build, so the launcher is set there too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@RyeMutt @claude
llmessage's and llcorehttp's tests are Catch2 binaries, converted by …
5751fed 
…tut2catch.py

TM19, the second wave of stage 3: 31 TUT files, 361 tests, become five
Catch2 binaries, each file still run in a process of its own:
- llmessage_tests, 18 files linking llmessage, with io_test's pipes
  (llpipeutil.cpp) as an OBJECT library beside them;
- llcoproceduremanager_test, lltrustedmessageservice_test and
  lltemplatemessagedispatcher_test alone: they stub parts of llmessage
  and llcorehttp, so they compile the file they test in (UNIT);
- llcorehttp_tests, its ten suites, run under the Python peer through
  LAUNCHER as the TUT executables were, with http_proxy cleared.

The message-system tests keep the system of their own that stage 1 gave
each, on a port the OS picks, rather than one shared for the process,
which the plan had proposed: a case's system is gone when it ends, and
nothing waits on another's.

The files are tut2catch.py's output with the names the idiom pass that
follows chose, given through --names; none carries a hand edit.
tut_parity.py, against the TUT executables' recorded results (361 tests:
355 ok, 6 skipped), holds for all 31 as the converter wrote them.
alnetimpairment's test without AL_NET_IMPAIRMENT is not compiled here,
and five HttpRequest tests that skip before naming themselves are
listed. All 38 entries pass, the two combined runs among them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@RyeMutt @claude
llmessage's and llcorehttp's tests read as Catch2 a person would write
0c58e8e 
The idiom pass over TM19's 31 converted files, as for llmath and TM18:
- every case named for what it checks: 233 were #N, and many names taken
  from set_test_name or comments ran long, carried commas or asked
  questions;
- messages that only restated their assertion are gone: 1,244 became
  205, and 24 CAPTUREs say what string-building said before;
- asserting helpers are named for what they do now (require_*), with a
  scoped INFO or CAPTURE in place of a message built by hand;
- 111 REQUIREs that guarded nothing are CHECKs, and the 4 CHECKs that
  guard a read after them are REQUIREs.

No case was added, removed or moved, and no assertion: per case, each
assertion compares what it compared before, in the same order. Six
changed only in form: literals moved right, and an UNSCOPED_INFO and
REQUIRE(false) in HttpRequest's handler became a FAIL.

tut_parity.py --recounted holds for all 31 files, and all 38 entries
pass. Bugs in the tests that the pass found are left for their own
change, since a conversion keeps behaviour:
- lldatapacker: sizes taken with sizeof of a pointer, so arbitrary; a
  31-bit tolerance on a value packed with 23; binary round trips never
  check the size read back; buffers that leak.
- llnamevalue: a value built and never checked, and the read-only VEC3
  setter called on the F32 value instead.
- lltemplatemessagebuilder: the "unchanged buffer" case clears the wire
  buffer and never the output; a size read after the index it guards;
  both quaternion round trips use angle 0, the identity.
- llsdmessagebuilder: binary cases that pass a size a fraction of the
  array; an S16 array compared for half its bytes; guards that check
  size > 0 before reading N bytes; an empty case; fixtures that leak.
- io and llbuffer: iterators dereferenced after ++ with no check against
  endSegment(); one write a byte past a full buffer; comments that say
  the opposite of their check (llbuffer, llxorcipher).
- llhttpnode: a node addNode refuses is leaked.
- lltemplatemessagedispatcher: every case dereferences a null reader.
- httpstatus: status1 assigned twice, where status2 was meant.
- bufferarray and bufferstream: a guard-byte check one byte late, and a
  pbackfail case whose expectation reads backwards.
- message_test: Response::mStatus is never initialised.
- llcoproceduremanager: three cases test only boost::fibers channels,
  two of them with no assertion.
- refcounted: a reference leaked, and comments copied from the case
  before.
- httprequest: a disallowed header spelled with an underscore, so it can
  never match; a "Range" case that sends no Range header; the 503 case
  never reaches the peer's Retry-After list.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@RyeMutt @claude
llmessage's and llcorehttp's tests have the state guard watch what th…
ccf8e19 
…ose libraries keep for the process

TM19's domain checks, as OBJECT libraries for every binary that links
the library, beside TM18's for llxml and llcharacter:
- llmessage: gMessageSystem, which a case that makes a system of its own
  must put back, or every later case sends through it; and whether the
  LLProxy singleton exists, which a case that sets curl up ends with.
- llcorehttp: the HTTP service's instance and state, the process's one
  service and worker thread, which a case that makes them must destroy
  before the next case's createService; and whether the websocket
  manager exists, whose servers each run a thread.

All five TM19 binaries pass 10 random orders and 20 repeats with no
report. Each check, with a case made to leak what it watches, names
that case: a message system scope never ended, an LLProxy made and kept,
a service created and never destroyed, a websocket manager left alive.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 59 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: f3177e92-c967-446d-b1bd-729ef69cf4db
📥 Commits

Reviewing files that changed from the base of the PR and between a0682b5 and ccf8e19.

📒 Files selected for processing (47)
  • indra/cmake/ALAddTestBinary.cmake
  • indra/llcorehttp/CMakeLists.txt
  • indra/llcorehttp/tests/bufferarray_test.cpp
  • indra/llcorehttp/tests/bufferstream_test.cpp
  • indra/llcorehttp/tests/httpheaders_test.cpp
  • indra/llcorehttp/tests/httpoperation_test.cpp
  • indra/llcorehttp/tests/httprequest_test.cpp
  • indra/llcorehttp/tests/httprequestqueue_test.cpp
  • indra/llcorehttp/tests/httpstatus_test.cpp
  • indra/llcorehttp/tests/jsonrpcws_test.cpp
  • indra/llcorehttp/tests/llcorehttp_test.cpp
  • indra/llcorehttp/tests/llcorehttp_test.h
  • indra/llcorehttp/tests/llcorehttp_test_checks.cpp
  • indra/llcorehttp/tests/refcounted_test.cpp
  • indra/llcorehttp/tests/test_bufferarray.hpp
  • indra/llcorehttp/tests/websocketmgr_test.cpp
  • indra/llmessage/CMakeLists.txt
  • indra/llmessage/llmessageconfig.cpp
  • indra/llmessage/llmessageconfig.h
  • indra/llmessage/message.cpp
  • indra/llmessage/net.cpp
  • indra/llmessage/tests/alnetimpairment_test.cpp
  • indra/llmessage/tests/io_test.cpp
  • indra/llmessage/tests/llavatarnamecache_test.cpp
  • indra/llmessage/tests/llbuffer_test.cpp
  • indra/llmessage/tests/llcoproceduremanager_test.cpp
  • indra/llmessage/tests/lldatapacker_test.cpp
  • indra/llmessage/tests/llhost_test.cpp
  • indra/llmessage/tests/llhttpnode_test.cpp
  • indra/llmessage/tests/llmessage_test_checks.cpp
  • indra/llmessage/tests/llmessageconfig_test.cpp
  • indra/llmessage/tests/llmessagesystem_fixture.h
  • indra/llmessage/tests/llmessagetemplateparser_test.cpp
  • indra/llmessage/tests/llnamevalue_test.cpp
  • indra/llmessage/tests/llpartdata_test.cpp
  • indra/llmessage/tests/llsdmessagebuilder_test.cpp
  • indra/llmessage/tests/llsdmessagereader_test.cpp
  • indra/llmessage/tests/lltemplatemessagebuilder_test.cpp
  • indra/llmessage/tests/lltemplatemessagedispatcher_test.cpp
  • indra/llmessage/tests/lltrustedmessageservice_test.cpp
  • indra/llmessage/tests/llxfer_file_test.cpp
  • indra/llmessage/tests/llxorcipher_test.cpp
  • indra/llmessage/tests/llzerocode_test.cpp
  • indra/llmessage/tests/message_test.cpp
  • indra/test/sync.h
  • scripts/testing/tut2catch.py
  • scripts/testing/tut_parity.py
  • 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

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

@RyeMutt RyeMutt mentioned this pull request Oct 12, 2026
Merged
4 of 9 tasks
@RyeMutt
RyeMutt added this pull request to stack #457 October 12, 2026 20:39
@RyeMutt RyeMutt mentioned this pull request Oct 12, 2026
Merged
4 of 9 tasks
@RyeMutt
RyeMutt merged commit 8337ab0 into develop Oct 12, 2026
21 checks passed
@RyeMutt
RyeMutt deleted the rye/tests-tm19 branch October 12, 2026 23:59
@RyeMutt RyeMutt mentioned this pull request Oct 13, 2026
Merged
5 of 9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

c/cpp cmake llcorehttp llmessage python

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@RyeMutt