tests: introduce doctest infra + migrate llcommon/llmath/llcorehttp subsets (ctest green) [relates to #4445] by mayconbekkers · Pull Request #4852 · secondlife/viewer

mayconbekkers · 2025-10-17T23:23:06Z

Relates to #4445.

This PR adds a header-only doctest setup and migrates a first wave of unit tests while keeping the legacy TUT harness for suites not yet ported. The goal is to land a minimal, working baseline that the team can iterate on module by module.
This affects only unit-test targets behind LL_TESTS=ON; viewer runtime and packages are unchanged.

What’s included

New doctest targets: llcommon_doctest, llmath_doctest, llcorehttp_doctest
Shared helpers (LL_CHECK_* for floats, ranges, buffers, wide strings) + small Windows wide-string shim
Deterministic llcorehttp fakes (zero-latency transport, monotonic clock, queued per-handle responses, redirects/retries/cancels) to keep tests network/IO-free
Suites green locally:
- llcommon_doctest: 100 cases
- llmath_doctest: 93 cases
- llcorehttp_doctest: 34 cases
Quickstart doc: docs/testing/doctest_quickstart.md

How to run locally

# Enable tests
autobuild configure -c RelWithDebInfoOS -- -DLL_TESTS=ON

# Build (VS generator)
"C:\Program Files\Microsoft Visual Studio\2022\Community\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe" --build build-vc170-64 --config RelWithDebInfo --target llcommon_doctest llmath_doctest llcorehttp_doctest -- /p:BuildProjectReferences=false

# Run
"C:\Program Files\Microsoft Visual Studio\2022\Community\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\ctest.exe" -C RelWithDebInfo -R "(llcommon_doctest|llmath_doctest|llcorehttp_doctest)" -V --test-dir build-vc170-64

Scope / non-goals

Does not change viewer runtime or packaging.
TUT remains for non-migrated suites; removal will happen after follow-ups when coverage is sufficient.

Proposed follow-ups

Continue migrating remaining suites per module (llcommon -> llmath -> llcorehttp -> newview/test).
Once coverage is high enough, remove Tut.cmake glue and clean up LLAddBuildTest.cmake usage.

github-actions · 2025-10-17T23:23:23Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

mayconbekkers · 2025-10-17T23:23:34Z

I have read the CLA Document and I hereby sign the CLA

Ansariel · 2025-10-18T11:49:20Z

What are the changes to .gitignore and .gitattributes for? They seem in no relation to doctest and counterproductive,

mayconbekkers · 2025-10-20T23:37:57Z

@Ansariel Thanks for the note! I’ve dropped the unrelated .gitignore/.gitattributes edits so this PR stays focused on doctest.
Also added the standard $LicenseInfo headers to the new test files and a third-party MIT notice for indra/extern/doctest/doctest.h.
pre-commit run -a passes locally; once workflows are approved for this fork, CI should reflect that.
Happy to adjust anything else if the maintainers prefer.

Copilot

Pull Request Overview

This PR introduces a doctest-based unit testing framework to complement the existing TUT infrastructure, migrating initial test suites for llcommon, llmath, and llcorehttp. The changes keep the current TUT framework intact while establishing new doctest targets for gradual migration.

Key Changes

Added header-only doctest framework with shared test helpers (LL_CHECK_* macros for floating-point, range, and buffer comparisons)
Migrated 227 test cases across three modules: llcommon_doctest (100 cases), llmath_doctest (93 cases), llcorehttp_doctest (34 cases)
Created deterministic HTTP test fakes (zero-latency transport, monotonic clock, queued responses) for network-free testing

Reviewed Changes

Copilot reviewed 66 out of 68 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
`tools/testing/gen_tut_to_doctest.py`	Python script to auto-generate doctest stubs from TUT sources
`indra/test/{doctest_main.cpp,ll_doctest_helpers.h,tut_compat_doctest.h}`	Shared doctest infrastructure and TUT compatibility layer
`indra/llmath/tests_doctest/*.cpp`	Migrated vector/matrix/quaternion math tests to doctest
`indra/llcorehttp/tests_doctest/{http_fakes.h,http_fakes.cpp}`	Deterministic HTTP testing infrastructure
`indra/llcorehttp/tests_doctest/*_test_doctest.cpp`	Migrated HTTP component tests with fake transport
`indra/llcommon/tests_doctest/*_test_doctest.cpp`	Auto-generated TODO stubs for future llcommon migration
`indra/viewer_components/login/tests/lllogin_doctest.cpp`	New doctest-based login workflow tests
`indra/*/CMakeLists.txt`	Build configuration for new doctest targets

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mayconbekkers · 2025-11-22T02:05:22Z

Hey @akleshchev

Thanks a lot for taking a look and for requesting the review here.

Just to clarify: this PR is only meant to be a first step for a doctest-based test setup, not the full migration. I wanted to get early feedback before going any further.

I also saw on Discord that Signal is stepping away, and I really appreciate that @Geenz is keeping the open source program moving forward. If this approach to the doctest infra, the CMake wiring and the way I’m migrating these first tests looks reasonable from your side, I’m happy to continue in follow-up PRs and adjust things to match what you’d like to see in the test suite.

No rush at all, I know there’s a lot happening right now. Just wanted to make the intent clear and let you know I’m around to iterate on this.

akleshchev · 2025-11-24T16:29:27Z

this PR is only meant to be a first step for a doctest-based test setup, not the full migration

That's obvious from the todo) We do need a replacement, but with signal away I don't know who is responsible for the commisions, will ask around.

P.S. You probably can make an AI do the bulk of the work here

SntaxLinden

LGTM. Approved.

…ubsets (ctest green) Signed-off-by: Maycon Bekkers <mayconbekkers@gmail.com>

… for doctest.h

mayconbekkers · 2025-12-01T11:42:34Z

Quick follow-up on the doctest infra:

I shared a common lltest_harness between the TUT runner and doctest_main (APR, logging with LLReplayLog, LLTrace).
I stabilized the doctest targets on MSVC by wiring:
- llcommon_doctest (tuple/workqueue subset),
- llmath_doctest (v2/v3/v4/llquaternion),
- llcorehttp_doctest (HTTP fakes + header helpers),
- login_doctest (login workflow with XML-RPC fakes).
I added a small docs/testing/doctest_quickstart.md to document how to enable and run the doctest targets locally on Windows.

Locally (RelWithDebInfoOS, LL_TESTS=ON) I ran:

llcommon_doctest – 7 test cases / 24 assertions
llmath_doctest – 102 test cases / 253 assertions
llcorehttp_doctest – 22 test cases / 105 assertions
login_doctest – 3 test cases / 5 assertions

All green (Status: SUCCESS).

mayconbekkers · 2026-03-29T01:20:51Z

Updated this with the cleaned stack. I also kept only the minimal llui.h fix needed to keep the branch buildable. Would appreciate a review when you have a moment.

Copilot

Pull request overview

Copilot reviewed 90 out of 144 changed files in this pull request and generated 10 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-29T09:15:07Z

indra/llimage/tests_doctest/llimageworker_test_doctest.cpp

+#include "../llcommon/lltimer.h"
+// for lltrace class
+#include "../llcommon/lltrace.h"


These relative includes look incorrect for indra/llimage/tests_doctest (they would resolve to indra/llimage/llcommon/...). Since the target already adds ${CMAKE_SOURCE_DIR}/llcommon to include paths, include the headers via the public include form (e.g., #include \"lltimer.h\" / #include \"lltrace.h\") or another correct path to avoid build breaks on clean checkouts.

Suggested change

#include "../llcommon/lltimer.h"

// for lltrace class

#include "../llcommon/lltrace.h"

#include "lltimer.h"

// for lltrace class

#include "lltrace.h"

Copilot · 2026-03-29T09:15:07Z

indra/llimage/tests_doctest/llimageworker_test_doctest.cpp

+            responder_test(bool* 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 is written from the decode thread and read from the main test thread without synchronization, which is a data race (UB) even if it 'usually works'. Use std::atomic_bool (or a condition_variable) for cross-thread completion signaling, and consider replacing the polling sleep loop with a proper wait/notify to reduce flakiness and runtime.

Copilot · 2026-03-29T09:15:07Z

indra/llimage/tests_doctest/llimageworker_test_doctest.cpp

+        mThread = new LLImageDecodeThread(true);
+        ensure("LLImageDecodeThread: threaded constructor failed", mThread != NULL);
+        // Insert something in the queue
+        bool done = false;


done is written from the decode thread and read from the main test thread without synchronization, which is a data race (UB) even if it 'usually works'. Use std::atomic_bool (or a condition_variable) for cross-thread completion signaling, and consider replacing the polling sleep loop with a proper wait/notify to reduce flakiness and runtime.

Copilot · 2026-03-29T09:15:08Z

indra/llimage/tests_doctest/llimageworker_test_doctest.cpp

+        while ((done == false) && (total_time < MAX_TIME))
+        {
+            ms_sleep(INCREMENT_TIME);
+            total_time += INCREMENT_TIME;
+        }


done is written from the decode thread and read from the main test thread without synchronization, which is a data race (UB) even if it 'usually works'. Use std::atomic_bool (or a condition_variable) for cross-thread completion signaling, and consider replacing the polling sleep loop with a proper wait/notify to reduce flakiness and runtime.

Copilot · 2026-03-29T09:15:08Z

indra/llfilesystem/tests_doctest/lldiriterator_test_doctest.cpp

+        //  Check a selection of bad group names from the crash reports
+        LLDirIterator iter(".","+bad-group-name]+?\?-??.*");
+        LLDirIterator iter1(".","))--@---bad-group-name2((?\?-??.*\\.txt");
+        LLDirIterator iter2(".","__^v--x)Cuide d sua vida(x--v^__?\?-??.*");


These variables are intentionally unused (construction-only), which can trigger -Wunused-variable warnings and fail builds when warnings are treated as errors. Mark each as [[maybe_unused]] or explicitly cast to void (e.g., (void)iter;) after construction.

Suggested change

LLDirIterator iter2(".","__^v--x)Cuide d sua vida(x--v^__?\?-??.*");

LLDirIterator iter2(".","__^v--x)Cuide d sua vida(x--v^__?\?-??.*");

(void)iter;

(void)iter1;

(void)iter2;

Copilot · 2026-03-29T09:15:08Z

indra/llmath/tests_doctest/alignment_test_doctest.cpp

+#       ifdef LL_DEBUG
+//      skip("This test fails on Windows when compiled in debug mode.");
+#       endif


This test suite contains (1) a known debug-mode failure note that is currently commented out, and (2) std::cout output inside a loop. Both can cause noisy or flaky CI runs (especially under Debug builds). Prefer a proper doctest skip in LL_DEBUG (e.g., DOCTEST_SKIP) and remove the console output (or gate it behind a debug flag) to keep test runs deterministic and quiet.

Copilot · 2026-03-29T09:15:08Z

indra/llmath/tests_doctest/alignment_test_doctest.cpp

+        TUT_ENSURE("LLAligment veca base", is_aligned(veca,16));
+        for(int i=0; i<ARR_SIZE; i++)
+        {
+            std::cout << "veca[" << i << "]" << std::endl;


This test suite contains (1) a known debug-mode failure note that is currently commented out, and (2) std::cout output inside a loop. Both can cause noisy or flaky CI runs (especially under Debug builds). Prefer a proper doctest skip in LL_DEBUG (e.g., DOCTEST_SKIP) and remove the console output (or gate it behind a debug flag) to keep test runs deterministic and quiet.

Copilot · 2026-03-29T09:15:08Z

docs/testing/doctest_quickstart.md

+- Build targets: `llcommon_doctest` (subset of llcommon tests), `llmath_doctest`, `llcorehttp_doctest`, `login_doctest`
+- Run: `ctest -C RelWithDebInfo -R "(llcommon_doctest|llmath_doctest|llcorehttp_doctest|login_doctest)" -V`


The quickstart references login_doctest, but this PR's CMake changes only add doctest executables for llcommon/llmath/llcorehttp (plus a few other modules) and no login_doctest target is introduced in the provided diffs. Either add the missing target or remove/update login_doctest from the quickstart so instructions match what actually builds.

Copilot · 2026-03-29T09:15:09Z

indra/llcorehttp/tests_doctest/httpoperation_test_doctest.cpp

+                    recorded_locations.push_back(headers->find("Location") ? *headers->find("Location") : std::string());
+                    recorded_content_types.push_back(headers->find("Content-Type") ? *headers->find("Content-Type") : std::string());


headers->find(...) is called twice per header name in the ternary expressions. Store the result of find() in a local pointer/optional and reuse it to avoid redundant lookups and keep the code easier to read.

Suggested change

recorded_locations.push_back(headers->find("Location") ? *headers->find("Location") : std::string());

recorded_content_types.push_back(headers->find("Content-Type") ? *headers->find("Content-Type") : std::string());

auto location = headers->find("Location");

recorded_locations.push_back(location ? *location : std::string());

auto content_type = headers->find("Content-Type");

recorded_content_types.push_back(content_type ? *content_type : std::string());

Copilot · 2026-03-29T09:15:09Z

indra/llcorehttp/tests_doctest/http_fakes.cpp

+LLCore::HttpHandle FakeTransport::nextHandle()
+{
+    ++mNextId;
+    return reinterpret_cast<LLCore::HttpHandle>(mNextId);


Casting an incrementing integer to HttpHandle with reinterpret_cast is implementation-defined (and potentially problematic if HttpHandle is a pointer type on some platforms/ABIs). Prefer generating handles in the same representation as HttpHandle (e.g., if it is integral, use static_cast; if it is pointer-like, consider storing an internal uintptr_t and converting via a well-defined helper, or use the production handle generator if available).

Suggested change

return reinterpret_cast<LLCore::HttpHandle>(mNextId);

return static_cast<LLCore::HttpHandle>(mNextId);

akleshchev · 2026-03-29T09:28:08Z

Lots of 'todos'.
Some tests are poorly commented, but that appears to be the fault on viewer's side (ex: all original color test files have effectively no comments, so I had to read the function to understand what's being tested)

github-actions bot added c/cpp cmake python llcommon llcorehttp llmath labels Oct 17, 2025

mayconbekkers force-pushed the feat/doctest-poc-clean branch 3 times, most recently from 5fb6af0 to dddc098 Compare October 20, 2025 23:34

akleshchev requested review from Geenz and Copilot November 17, 2025 13:54

Copilot AI reviewed Nov 17, 2025

View reviewed changes

akleshchev requested a review from kylelinden November 24, 2025 16:23

akleshchev linked an issue Nov 24, 2025 that may be closed by this pull request

Replace tut with doctest #4445

Open

kylelinden requested review from SntaxLinden and removed request for kylelinden November 25, 2025 20:41

SntaxLinden approved these changes Nov 25, 2025

View reviewed changes

mayconbekkers added 4 commits November 29, 2025 23:27

tests: introduce doctest infra + migrate llcommon/llmath/llcorehttp s…

d3f86dc

…ubsets (ctest green) Signed-off-by: Maycon Bekkers <mayconbekkers@gmail.com>

chore: drop unrelated .gitignore/.gitattributes changes

f0cc844

style: satisfy pre-commit hooks

788a7e4

chore(license): add viewer OSS headers to doctest suites and note MIT…

4e3aeda

… for doctest.h

mayconbekkers force-pushed the feat/doctest-poc-clean branch from dddc098 to 4e3aeda Compare November 30, 2025 02:30

tests: share lltest harness and stabilize doctest targets on MSVC

3548f4c

mayconbekkers added 17 commits March 28, 2026 18:46

llcommon tests: fix doctest include path

7dbae78

test: stop building the legacy lltest driver

708b9ed

build: stop registering the dormant indra/test subdir

4c6b31c

llxml: drop the old LL_ADD_* test hooks

b4dd99e

llimage: drop the old LL_ADD_PROJECT_UNIT_TESTS hook

832e93b

llfilesystem: drop the old LL_ADD_* test hooks

a5140c4

llinventory: drop the old LL_ADD_* test hooks

2f39dae

login tests: drop the old LL_ADD_PROJECT_UNIT_TESTS hook

b042b98

llprimitive: drop the old LL_ADD_PROJECT_UNIT_TESTS hook

a4b7f8b

llui: drop the old LL_ADD_* test hooks

d2273a7

llcorehttp: drop the old LL_ADD_INTEGRATION_TEST hook

c875e5e

llmessage: drop the old LL_ADD_* test hooks

a8b2242

llmath: drop the old LL_ADD_* test hooks

fa1ec14

newview: drop the old LL_ADD_* test hooks

d7afdbd

llcommon: drop the old tuple and workqueue hooks

1122042

cleanup: trim wrapper comments and eof noise

383db4f

llui: include boost bind placeholders

3006e09

github-actions bot added llcharacter llfilesystem llimage llinventory llmessage llprimitive llui llxml labels Mar 29, 2026

akleshchev requested a review from Copilot March 29, 2026 09:13

Copilot AI reviewed Mar 29, 2026

View reviewed changes

tests: trim placeholder text and review nits

6b7f4f0

-        LLDirIterator iter2(".","__^v--x)Cuide d sua vida(x--v^__?\?-??.*");
+        LLDirIterator iter2(".","__^v--x)Cuide d sua vida(x--v^__?\?-??.*");
+        (void)iter;
+        (void)iter1;
+        (void)iter2;

		- Build targets: `llcommon_doctest` (subset of llcommon tests), `llmath_doctest`, `llcorehttp_doctest`, `login_doctest`
		- Run: `ctest -C RelWithDebInfo -R "(llcommon_doctest\|llmath_doctest\|llcorehttp_doctest\|login_doctest)" -V`

		recorded_locations.push_back(headers->find("Location") ? *headers->find("Location") : std::string());
		recorded_content_types.push_back(headers->find("Content-Type") ? *headers->find("Content-Type") : std::string());

-                    recorded_locations.push_back(headers->find("Location") ? *headers->find("Location") : std::string());
-                    recorded_content_types.push_back(headers->find("Content-Type") ? *headers->find("Content-Type") : std::string());
+                    auto location = headers->find("Location");
+                    recorded_locations.push_back(location ? *location : std::string());
+                    auto content_type = headers->find("Content-Type");
+                    recorded_content_types.push_back(content_type ? *content_type : std::string());

	return reinterpret_cast<LLCore::HttpHandle>(mNextId);
	return static_cast<LLCore::HttpHandle>(mNextId);

Conversation

mayconbekkers commented Oct 17, 2025

Relates to #4445.

What’s included

How to run locally

Scope / non-goals

Proposed follow-ups

Uh oh!

github-actions bot commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mayconbekkers commented Oct 17, 2025

Uh oh!

Ansariel commented Oct 18, 2025

Uh oh!

mayconbekkers commented Oct 20, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes

Reviewed Changes

Uh oh!

mayconbekkers commented Nov 22, 2025

Uh oh!

akleshchev commented Nov 24, 2025

Uh oh!

SntaxLinden left a comment

Choose a reason for hiding this comment

Uh oh!

mayconbekkers commented Dec 1, 2025

Uh oh!

mayconbekkers commented Mar 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

akleshchev commented Mar 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

github-actions bot commented Oct 17, 2025 •

edited

Loading