Repository navigation
Test infrastructure: unused vendored libFuzzer and 2016 reports, copied test helpers, stale fuzzer driver comments, missing round-trip coverage, drifted CMake integration tests #5714
Description
These items came out of a technical-debt review of develop at 633de8e. Each item is independent and can be fixed in its own PR that references this issue. All items touch only tests, fuzzers, test CMake code or vendored test-side files, so library behavior, the public API and the ABI stay unchanged. The one exception on the test side is part of item 6: forwarding -A/-T to the inner CMake projects changes which platform and toolset the Win32 and ClangCL CI jobs actually build, and may surface new failures there.
Checklist
- 1. Run the CBOR, MessagePack, BSON and BON8 fuzzer round-trip invariants on the round-trip corpus in CI
- 2. Move test helpers copied between test files into shared headers (
SaxCountdownx6,value_in_range_oftrait test,sax_no_exception) - 3. Remove the unused vendored libFuzzer copy in
tests/thirdparty/Fuzzer - 4. Fix fuzzer driver step lists that describe
j1 == j2checks the code does not perform; drop a deadis_discarded()check and unused includes — overlaps Recover from parse errors when parse_error() returns true #5601 - 5.
cmake_fetch_content: silence the CMP0169 deprecation on purpose and fix the stale version guards - 6. Make the CMake integration-test wrappers consistent (platform/toolset forwarding, unused
-D, deadset()) (behavior change) - 7. Turn the Segmentation fault on G++ when trying to assign json string literal to custom json type. #972
fifo_mapregression test into a real test and takefifo_mapoff every test target's include path - 8. Remove the unreferenced 2016 fuzz and benchmark reports in
tests/reports - 9. Document the local patch to vendored
doctest.hand fix stale comments indoctest_compatibility.h
Items
1. Run the CBOR, MessagePack, BSON and BON8 fuzzer round-trip invariants on the round-trip corpus in CI
tests/src/round_trip_corpus.hpp L22-L28 exists so that the fuzzer round-trip checks also run in CI instead of only at OSS-Fuzz. Only two test cases use it: "UBJSON round-trip invariants" (unit-ubjson.cpp L2886) and "BJData round-trip invariants" (unit-bjdata.cpp L4405).
The CBOR, MessagePack, BSON and BON8 drivers assert the same byte-stability invariant (for example fuzzer-parse_cbor.cpp L52: assert(json::to_cbor(j2) == vec2);, likewise msgpack L52, bson L57, bon8 L80), but no unit test checks it for these formats. A regression there still shows up only as an OSS-Fuzz report. tests/fuzzing.md L96-L98 also names only UBJSON and BJData.
A local probe (seed j1 = from_X(to_X(j0)) for each corpus value, then run the driver steps) found no violations: CBOR 3849 values, MessagePack 3909, BSON 2958 (objects only), BON8 3841. The only skips were expected seeding rejections: CBOR tag 0xD8 under the default tag handler, and BON8 uint64 values above INT64_MAX.
Proposed change: Add "CBOR/MessagePack/BSON/BON8 round-trip invariants" test cases to unit-cbor.cpp, unit-msgpack.cpp, unit-bson.cpp and unit-bon8.cpp, modeled on the UBJSON case: iterate utils::round_trip_corpus::values(), seed j1 (skip values whose seeding throws, as the driver does), require from_X(to_X(j1)) not to throw and check to_X(j2) == to_X(j1). BSON uses object values only. Update the comment in round_trip_corpus.hpp and tests/fuzzing.md to name all six formats. The stream-versus-contiguous check (BON8 driver L62 only) is out of scope because #5601 reworks it.
Verification: New cases pass in the unit test jobs, ci_test_clang_sanitizer and ci_test_valgrind; each stays within the FastTests budget (a few hundred ms at -O1, compare with the UBJSON/BJData cases).
2. Move test helpers copied between test files into shared headers
Several helpers are copied verbatim between test translation units instead of living next to test_utils.hpp and round_trip_corpus.hpp:
SaxCountdown(74 lines, byte-identical) appears six times, about 370 redundant lines: unit-cbor.cpp L25-L98, unit-msgpack.cpp L28-L101, unit-ubjson.cpp L23-L96, unit-bjdata.cpp L27-L100, unit-bon8.cpp L28-L101 (the newest, from Add BON8 support #2998), unit-bson.cpp L862-L935. (unit-class_parser.cpp has a different variant derived fromjson_sax_t; it is not part of this set.)trait_test_argandTEST_CASE_TEMPLATE_DEFINE("value_in_range_of trait", ...)are identical in unit-32bit.cpp L17-L89 and unit-bjdata.cpp L104-L176. TheTEST_CASE_TEMPLATE_INVOKElists differ (unit-32bit.cpp L95; unit-bjdata.cpp L178/L197/L204), so only the definition is shared.sax_no_exceptionand the "issue encoding of json::exception::what() #2824 - encoding of json::exception::what()" section appear in unit-regression2.cpp L206-L222/L726-L734 and unit-disabled_exceptions.cpp L22-L49. encoding of json::exception::what() #2824 was about exceptions disabled, so the latter copy is the one that matters.
Proposed change: Move SaxCountdown into a new tests/src/sax_countdown.hpp in namespace utils and include it from the six files. Move the trait test definition into a small shared header (e.g. tests/src/value_in_range_of_test.hpp), keeping each file's TEST_CASE_TEMPLATE_INVOKE; unit-32bit.cpp must keep it because JSON_32bitTest=ONLY builds only that file. Remove the #2824 class, its static member and SECTION from unit-regression2.cpp (ci_test_noexceptions reruns the suite without exceptions anyway).
Verification: Assertion counts of the affected tests stay the same except for the three checks removed from unit-regression2; check ci_test_clang (-Weverything) for -Wunused-member-function/-Wweak-vtables, and ci_clang_tidy on one changed TU.
Sequencing: No open PR touches these hunks, but #5382, #5328, #5707, #5703 and #5596 edit nearby lines (includes or code right after the class) and will need a small rebase.
3. Remove the unused vendored libFuzzer copy in tests/thirdparty/Fuzzer
tests/thirdparty/Fuzzer is a vendored LLVM libFuzzer (155 files, 776 KB, Apache-2.0) from the original OSS-Fuzz integration, touched since only by #3462 (reorganization) and #4109 (typo). Nothing builds or includes it:
git grep -n thirdparty/Fuzzeroutside the directory finds only the REUSE entry at.reuse/dep5L26-L28.tests/MakefileL12 defaults to the repo's own driver (FUZZER_ENGINE = src/fuzzer-driver_afl.cpp); for libFuzzer,tests/fuzzing.mdL29-L41 uses the compiler's-fsanitize=fuzzer.tests/CMakeLists.txtonly addsthirdparty/doctestandthirdparty/fifo_map; no workflow or script refers to the directory. ItsREADME.txtis a single "moved to llvm.org" line.
It needs its own license entry, attracts update PRs (#4933, closed), and adds noise to the flawfinder workflow, which scans the whole tree (flawfinder.yml L46); a local flawfinder --dataonly . put 66 of about 176 hits in this directory (not re-checked).
Proposed change: git rm -r tests/thirdparty/Fuzzer and delete the Files: tests/thirdparty/Fuzzer/* block (plus its trailing blank line) from .reuse/dep5. Keep LICENSES/Apache-2.0.txt, which include/nlohmann/detail/meta/cpp_future.hpp still needs. Formatting and amalgamation checks already exclude tests/thirdparty/*.
Verification: git grep thirdparty/Fuzzer is empty; reuse lint passes; make -C tests fuzzers builds with the default engine and with FUZZER_ENGINE="-fsanitize=fuzzer" CXX=clang++; CIFuzz and flawfinder stay green. (#4663 also edits .reuse/dep5, but only after L38.)
4. Fix fuzzer driver step lists that describe checks the code does not perform
The header comment of six of the seven binary-format fuzzer drivers lists an invariant the code does not check, which misleads readers and crash triagers:
- CBOR, MessagePack, BSON and BON8 say
- assert(j1 == j2)(fuzzer-parse_cbor.cpp L13-L16); the code never comparesj1withj2but checks byte stability,assert(json::to_cbor(j2) == vec2);(cbor/msgpack L52, bson L57, bon8 L80). - UBJSON lists
assert(j1 == j2/j3/j4)(fuzzer-parse_ubjson.cpp L13-L22); the code checksto_ubjson(jN, ...) == vecN(L69-L71). - BJData's step list (L13-L22) still says
assert(j1 == j2/j3/j4), although the paragraph added below it in Fall back to plain-object encoding when to_bjdata()'s _ArrayType_ annotation is not a string #5494 (L24-L43) explains theis_value_stable(...)check the code performs (L99-L101). - The JSON driver's list is correct.
Also: fuzzer-parse_bson.cpp L43-L46 has an unreachable if (j1.is_discarded()) { return 0; } (from_bson is called with allow_exceptions = true, so it throws instead). All seven drivers include <iostream> and <sstream>; none uses <iostream>, and only BON8 uses <sstream> (std::istringstream, L61).
Proposed change: Rewrite the step lists to match the code (for CBOR/MessagePack/BSON/BON8: j1 = from_X(data), vec = to_X(j1), j2 = from_X(vec), assert(to_X(j2) == vec); UBJSON the same per use_size/use_type setting; BJData describing the value-stability check). Remove the dead BSON branch, #include <iostream> from all seven drivers, and #include <sstream> from all but BON8.
Verification: make -C tests fuzzers FUZZER_ENGINE=-fsanitize=fuzzer CXX=clang++ builds (no reliance on transitive includes); a short run on the json_test_data corpus; CIFuzz stays green.
Sequencing: #5601 (draft) edits all seven drivers in the same hunks, including these step lists and the BSON is_discarded() block. Fold this into #5601 or do it after #5601 is merged.
5. cmake_fetch_content: silence the CMP0169 deprecation on purpose and fix the stale version guards
The cmake_fetch_content test project calls single-argument FetchContent_Populate(json) after FetchContent_Declare() (tests/cmake_fetch_content/project/CMakeLists.txt L8-L14). CMake 3.30 deprecated this form (CMP0169); since the project declares cmake_minimum_required(VERSION 3.11...3.14), the policy stays unset and every configure with the latest CMake (installed by lukka/get-cmake in CI) prints a deprecation warning. CMake plans to remove the form, at which point the test fails, and nothing says the old pattern is kept on purpose.
The test should stay: it is the only coverage of the FetchContent_Populate + add_subdirectory pattern for CMake 3.11-3.13 users, and the docs still say FetchContent works since 3.11. cmake_fetch_content2 covers FetchContent_MakeAvailable (3.14+).
The version guards are stale:
- tests/cmake_fetch_content/CMakeLists.txt L1:
VERSION_GREATER "3.11.0"is dead, since tests/CMakeLists.txt L1 requires 3.13. - tests/cmake_fetch_content2/CMakeLists.txt L1:
VERSION_GREATER "3.14.0"skips exactly 3.14.0, the first version withFetchContent_MakeAvailable. This guard is still needed because the test tree accepts 3.13.
Proposed change:
include(FetchContent)
+
+# This test deliberately covers the pre-3.14 FetchContent_Populate pattern.
+if(POLICY CMP0169)
+ cmake_policy(SET CMP0169 OLD)
+endif()Remove the dead 3.11 guard; change the cmake_fetch_content2 guard to VERSION_GREATER_EQUAL "3.14". Once the documented FetchContent minimum becomes 3.14, delete cmake_fetch_content.
Verification: Configure with CMake 4.x and confirm the CMP0169 warning is gone; ctest -R cmake_fetch_content passes on Linux, macOS and Windows in a job that does not exclude the git_required;not_reproducible labels. if(POLICY) and VERSION_GREATER_EQUAL work with the 3.13 minimum.
Side note: the FetchContent example in cmake.md L96-L103 declares cmake_minimum_required(VERSION 3.11) but calls FetchContent_MakeAvailable (3.14); it can be fixed here or separately.
6. Make the CMake integration-test wrappers consistent
The tests/cmake_* integration tests are six copied add_test/set_tests_properties wrappers that have drifted:
- Platform/toolset forwarding. Only
cmake_importandcmake_import_minverpass-A "${CMAKE_GENERATOR_PLATFORM}"to the inner configure.cmake_add_subdirectory,cmake_fetch_content,cmake_fetch_content2andcmake_target_include_directoriesdo not; none forwards-T, and all build withcmake --build .without--config. The Windows workflow configures with-A Win32and-T ClangCL(windows.ymlL183-L190, also L46, L83), so in those jobs the inner projects of these four tests build with the generator defaults instead. - Unused
-Dnlohmann_json_source. Both fetch_content wrappers pass it, but their inner projects never read it (CMake warns "manually-specified variables were not used"). - Dead
set().tests/CMakeLists.txtL13 setsJSON_FORCED_GLOBAL_COMPILE_OPTIONS, which is never read; the next lines read$ENV{...}directly. - Diverged MSVC options (minor).
tests/abi/CMakeLists.txtL7-L17 uses/W4;$<$<CONFIG:Release>:/wd4702>, whiletest_mainuses/W4;/wd4566;/wd4996;/wd4702;/wd4503;/wd5285plus/bigobj(L86-L88). The ABI tests pass today; this is only a drift risk.
(The off-by-one cmake_fetch_content2 guard is covered by item 5.)
Proposed change: Trivial part: remove the dead set() and the unused -Dnlohmann_json_source from both fetch_content wrappers. Separate PR, validated on the Windows jobs: forward -A "${CMAKE_GENERATOR_PLATFORM}" and -T "${CMAKE_GENERATOR_TOOLSET}" consistently (optionally --config $<CONFIG>), ideally via a helper in cmake/test.cmake such as json_add_cmake_integration_test(<name> <project_dir> [ARGS ...] [LABELS ...]). This changes what the Win32/ClangCL jobs build and may surface new failures. Optional: one INTERFACE target with the shared test compile options, linked by test_main and abi_compat_common.
Verification: ctest -R "cmake_" on Linux/macOS with the unused-variable warning gone; for forwarding, the Win32 and ClangCL jobs build the inner projects for Win32/clang-cl, and an empty -A ""/-T "" with Ninja/Makefile generators must be checked or guarded.
7. Turn the #972 fifo_map regression test into a real test and narrow its include path
The vendored tests/thirdparty/fifo_map/fifo_map.hpp has one user: the #972 regression test in unit-regression1.cpp (include and my_json alias, L34-L42; section, L1483-L1486). That section only builds an array (my_json const foo = R"([1, 2, 3])"_json;), so the fifo_map object type is never exercised, and it has no CHECK.
Still, the directory is a PUBLIC include directory of test_main (tests/CMakeLists.txt L96-L99), hence on the include path of every test-* target. Meanwhile the docs recommend fifo_map via an adapter for insertion order (object_order.md L54, template_parameters.md L234), and no test checks that recommendation.
Proposed change: Keep fifo_map. Extend the #972 section: keep the array-literal assignment (the original crash), then parse an object such as {"z":1,"a":2,"m":{"y":1,"b":2}} and CHECK that dump() keeps insertion order and that erase() and inserting a new key keep the remaining order. Narrow the include directory to test-regression1: json_test_set_test_options() (cmake/test.cmake L64-L101) has no include-directory keyword, so add one forwarding to target_include_directories(${test_interface} INTERFACE ...), or call target_include_directories() on the test-regression1 targets directly; then drop thirdparty/fifo_map from test_main.
Verification: test-regression1 passes for all configured standards and in ci_test_gcc/ci_test_clang; afterwards git grep fifo_map tests finds only unit-regression1.cpp and the vendored header, and no other test fails to compile. The fifo_map comparator is stateful and deep copies are fragile (#1763, #5649), so the new checks should not rely on deep copies.
8. Remove the unreferenced 2016 fuzz and benchmark reports in tests/reports
tests/reports holds 18 files (about 1.6 MB, mostly PNG and TIFF) from 2016: two AFL status snapshots (2016-08-29-fuzz/, 2016-10-02-fuzz/) and a nativejson-benchmark snapshot for the 2.0.x release (2016-09-09-nativejson_benchmark/, whose README.md links to the defunct rawgit.com).
Nothing refers to them: git grep -n "tests/reports\|reports/20" has no matches, and no workflow, CMake file, script, docs page or .reuse/dep5 entry mentions them. The last change was the move in #3462. They describe a library version a major version and ten years old, yet every clone and source archive carries them.
Proposed change: git rm -r tests/reports. No replacement is needed: current performance is measured by tests/benchmarks, and the README and design goals page already link the upstream nativejson-benchmark results.
Verification: git grep -n "reports/" shows no dangling references; docs build, htmlproofer and reuse lint are unaffected.
9. Document the local patch to vendored doctest.h and fix stale comments in doctest_compatibility.h
tests/thirdparty/doctest/doctest.h is doctest 2.4.12 (latest upstream), imported in #4771. Two weeks later #4801 ("Adjust warnings flags") hand-edited translateActiveException() (L4498-L4506) to declare String res; inside the translator loop instead of before it. Nothing records this, so re-vendoring doctest would silently drop it.
doctest_compatibility.h has two stale comments:
- L5:
#define DOCTEST_THREAD_LOCAL // enable single-threaded builds on XCode 6/7. Xcode 6/7 is no longer supported, but the define should stay: it keeps doctest offthread_local, which matters forci_test_no_thread_localand the Clang/MinGW crashes noted in macro_scope.hpp L189-L196. - L29-L32:
#include <iosfwd>is justified by tests that defineprivateaspublic; no test undertests/srcdoes that any more.
Proposed change: Record the upstream version (v2.4.12) and the #4801 patch with its reason, either as a comment at the patched site or in a tests/thirdparty/doctest/README.md. Reword the DOCTEST_THREAD_LOCAL comment and keep the define. Remove or reword the <iosfwd> comment; drop the include only if the full CI matrix (including MSVC 2015+) stays green without it.
Verification: A diff of the vendored doctest.h against upstream v2.4.12 shows only the documented hunk; if <iosfwd> is removed, the full CI matrix passes.
Related
- Recover from parse errors when parse_error() returns true #5601 (draft) edits all seven fuzzer drivers; item 4 must be sequenced with it, and item 1's unit tests should follow any driver-check changes it makes, as
tests/fuzzing.mdrequires. - Add deprecated from_bon8/from_bjdata(ptr, len) overloads #5688 appends to the BJData and BON8 round-trip tests (same files as items 1 and 2, different hunks).
This issue was written by Claude Code on behalf of @nlohmann.
Activity
Run the CBOR, MessagePack, BSON and BON8 round-trip invariants in CI
Fix fuzzer driver step lists to match the checks the code performs
Silence the CMP0169 deprecation in cmake_fetch_content, fix stale guards
Make the CMake integration-test wrappers consistent
Turn the #972 fifo_map regression test into a real test
Document the vendored doctest.h patch; fix stale doctest_compatibilit…