Reduce test-suite compile time: extract tiny per-standard test content; drop redundant legacy-comparison CI job

[nlohmann]

Summary

This addresses two of the smallest, lowest-risk items from the larger compile-time audit in #5419. It does not attempt the intra-file de-duplication items from that issue (the unit-items.cpp iterator_wrapper/items() duplication, unicode helper de-dup, iterators1/2 templating, binary-format corpus list sharing, etc.) — those are left for a future PR.

Fixes #5419 (partially — see "Out of scope" below for what is intentionally not done here).

Item 1 (partial): stop rebuilding two large files just for a few C++17-gated lines

• tests/src/unit-items.cpp (1433 lines) was compiled twice per CI configuration (test-items_cpp11 and test-items_cpp17) solely because it contained one small JSON_HAS_CPP_17-gated SECTION("structured bindings") (14 lines). That section is moved into a new, dedicated file, tests/src/unit-items-cpp17.cpp. No tests/CMakeLists.txt changes were needed: the existing file(GLOB ... src/unit-*.cpp) + json_test_add_test_for() machinery already auto-registers any unit-*.cpp file and auto-detects which C++ standards to build it for by checking whether the file text contains JSON_HAS_CPP_<N> — exactly the same mechanism already used by the existing tests/src/unit-iterators3.cpp (a small file dedicated to one JSON_HAS_CPP_14-gated test), which the new file mirrors.

  Result: unit-items.cpp now only produces test-items_cpp11 (the test-items_cpp17 target is gone). The new unit-items-cpp17.cpp produces test-items-cpp17_cpp11 (compiles to an intentionally empty translation unit under C++11 — 0 tests, 0 assertions, still a clean pass) and test-items-cpp17_cpp17 (1 test case / 1 assertion, identical in behavior to what ran before).

• tests/src/unit-regression1.cpp (1530 lines) was likewise built twice (test-regression1_cpp11 and test-regression1_cpp17) because it contained the substring JSON_HAS_CPP_17. On inspection, this turned out to be dead code: an orphaned

  #ifdef JSON_HAS_CPP_17
      #include <variant>
  #endif

  left over from when the actual std::variant-based regression test (issue Serializing std::variant causes stack overflow #1292, "Serializing std::variant causes stack overflow") was relocated to unit-regression2.cpp in an earlier split — unit-regression2.cpp still contains that SECTION and #include <variant> today. Nothing else in unit-regression1.cpp references variant, so there was no test content to extract; the dead include is simply deleted. unit-regression1.cpp now only produces test-regression1_cpp11.

  Both changes were verified with clang++ directly (-std=c++11/14/17/20, doctest test-case/assertion counts compared before/after — unchanged in every case) and with a real local CMake configure + build confirming the exact target lists described above.

Item 2: ci_test_legacycomparison — investigated, not removed

The issue suggested ci_test_legacycomparison (.github/workflows/ubuntu.yml, defined in cmake/ci.cmake) is redundant with the test-comparison_legacy target that tests/CMakeLists.txt already builds in every normal configuration (both use JSON_USE_LEGACY_DISCARDED_VALUE_COMPARISON=1).

On closer inspection, they are not redundant:

• test-comparison_legacy (in tests/CMakeLists.txt) applies the define as a per-target COMPILE_DEFINITIONS override to exactly one translation unit: unit-comparison.cpp. Every other test file in the suite is still compiled with the default (non-legacy) comparison semantics.
• ci_test_legacycomparison (in cmake/ci.cmake) instead configures the whole build with -DJSON_LegacyDiscardedValueComparison=ON, which (via CMakeLists.txt) adds JSON_USE_LEGACY_DISCARDED_VALUE_COMPARISON=1 as an INTERFACE compile definition on the main nlohmann_json target itself. Because every test target links against that target (transitively through test_main), this define propagates to every unit-*.cpp file in the entire suite, across all default C++ standards. Since JSON_USE_LEGACY_DISCARDED_VALUE_COMPARISON changes the behavior of operator== for discarded values inside json.hpp itself (see include/nlohmann/json.hpp), this exercises the legacy code path across the whole test suite (every CHECK/comparison in every file), not just the handful of comparisons in unit-comparison.cpp.

So ci_test_legacycomparison provides meaningfully broader coverage than test-comparison_legacy (whole-suite exercise of the legacy macro vs. one file), and I left it in place rather than removing it, per the issue's own caveat about not removing jobs that turn out to do something meaningfully different.

Breaking change?

No. This PR only touches files under tests/ (test-infrastructure only); nothing under include/ was changed, so there is no change to the public API and no ABI/API impact.

No test behavior or coverage was removed — every TEST_CASE/SECTION that existed before this PR still exists and still runs under the exact same C++-standard gating as before, just physically located in a different .cpp file (unit-items-cpp17.cpp) in the structured bindings case, or removed as genuinely dead/unused code in the unit-regression1.cpp <variant>-include case (confirmed via git log --follow that the corresponding test itself lives on, unchanged, in unit-regression2.cpp).

Out of scope (left for a future PR)

Per the issue's larger audit, none of the following are attempted here:

• The unit-items.cpp iterator_wrapper/items() intra-file duplication.
• Similar C++17-gated-content extraction for unit-regression2.cpp, unit-deserialization.cpp, unit-conversions.cpp, unit-element_access2.cpp, unit-msgpack.cpp (their gated portions are much larger and extracting them safely needs more care than fits this PR).
• Unicode helper de-duplication, iterators1/iterators2 templating, and binary-format corpus list sharing.

Test plan

• clang++ -std=c++11/14/17/20 compiled unit-regression1.cpp, unit-items.cpp, and the new unit-items-cpp17.cpp cleanly.
• Ran each resulting binary and confirmed doctest test-case/assertion counts are unchanged from the pre-change file for every standard (unit-regression1.cpp: 3 test cases under all of 11/14/17/20; unit-items.cpp: 2 test cases / 222 assertions under all of 11/14/17/20; unit-items-cpp17.cpp: 0 tests under 11, 1 test case / 1 assertion under 17/20).
• Confirmed against the unmodified origin/develop copy of unit-regression1.cpp that an unrelated, pre-existing local-environment failure (issue Parse throw std::ios_base::failure exception when failbit set to true #714, std::ios_base::failure behavior under this machine's libc++) is present identically before and after this change, i.e. not introduced by it.
• Local CMake configure (-DJSON_BuildTests=ON) + cmake --build . --target help confirmed the exact expected target-list changes: test-regression1_cpp17 and test-items_cpp17 no longer exist; test-items-cpp17_cpp11 and test-items-cpp17_cpp17 now exist and build/pass.

— opened by Claude Code on behalf of @nlohmann

[github-actions]

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated and/or formatted correctly.

📎 A ready-to-apply patch is attached to the failed workflow run as the amalgamation-patch artifact. Download it, then apply it locally from the repository root with:

git apply amalgamation.patch

This does not require installing astyle yourself.