Skip to content

Conversation

@mayconbekkers
Copy link

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
Copy link

github-actions bot commented Oct 17, 2025

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

@mayconbekkers
Copy link
Author

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

@Ansariel
Copy link
Contributor

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

@mayconbekkers mayconbekkers force-pushed the feat/doctest-poc-clean branch 2 times, most recently from 2acfa79 to 5fb6af0 Compare October 20, 2025 23:28
@mayconbekkers mayconbekkers force-pushed the feat/doctest-poc-clean branch from 5fb6af0 to dddc098 Compare October 20, 2025 23:34
@mayconbekkers
Copy link
Author

@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.

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

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants