Test number, curlConfig and isValidInstallId directly

[dmccoystephenson]

Summary

Stage B (unit-test expansion): three trace_client::detail helpers that had no direct test are now tested directly. Only test/test_trace_client.cpp is changed; trace_client.hpp is not touched.

• numberIsTheShortestJsonDecimalThatReadsBack: checks exact output for 0.1, -3, 1.5 and 0.1 + 0.2. For edge values (-0.0, 1e21, 1e-7, 5e-324, 1.7e308, 2^53 + 1, …), checks that every output matches JSON number grammar and reads back unchanged through strtod. Also checks that a global locale with a decimal comma and thousands grouping never reaches the body.
• curlConfigFixesEveryOptionAndQuotesEveryValue: pins the full curl config line for line (url, proto = "=http,https", the four headers, user-agent with TRACE_CLIENT_VERSION, data-binary, max-time/connect-timeout of 5, output, write-out). Any change to what curl is told now fails a test. The test also checks that a quote in the key is escaped and a newline in the application is dropped from the User-Agent. It is guarded the same way as curlConfig itself, with the NUL / /dev/null split for _WIN32.
• isValidInstallIdAcceptsOnlyTheDocumentedCharacters: checks the 1..255 length bounds, the [A-Za-z0-9_.-] alphabet, and whitespace, path, quote, newline, NUL and non-ASCII bytes.

All three are registered in main()'s tests[].

Bug found (not fixed here)

While the number test was being written, a round trip of DBL_MAX failed: number(DBL_MAX) returns 2e+308, which overflows to infinity. A Stage B cycle does not change production code, so the bug was filed as #7, with the cause and a one-line suggested fix. DBL_MAX is left out of the value list, with a comment naming #7.

Test plan

• make test (g++, C++11, -Werror): 44 tests, 0 failed checks (41 before, +3)
• make test STD=c++17: 44 tests, 0 failed checks
• make test STD=c++2a (the local g++ predates the c++20 spelling): 44 tests, 0 failed checks
• CI: linux matrix, linux-libcurl, sanitizers, macOS, Windows/MSVC

Deferred issues

• TRACE_CLIENT_CURL override is ignored on Windows when System32\curl.exe exists #1 (TRACE_CLIENT_CURL ignored on Windows): skipped, because the issue asks the owner to choose between honoring the override and documenting the current behavior.
• detail::number writes DBL_MAX as 2e+308, which overflows to infinity #7: filed during this cycle; left for an implementation cycle.

No tracking issue for the tests: the gap was found during triage.

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

🤖 Generated with Claude Code

---

drafted by Claude on behalf of Daniel Stephenson

[dmccoystephenson]

Self-review rubric (posted as a plain comment; the review below was performed inline in the same session that wrote the change, so it is not an independent review):

• Scope: PASS. git diff --stat origin/main shows one file, test/test_trace_client.cpp, +97 lines. There is no production change, which matches the Stage B rule.
• Tests-new: PASS. No new public method was added; three previously untested detail helpers (number, curlConfig, isValidInstallId) now have a direct test.
• Tests-fix: N/A. Nothing is fixed here. The bug the number test exposed was filed as detail::number writes DBL_MAX as 2e+308, which overflows to infinity #7 instead of being fixed under a test-expansion cycle.
• Sibling structure: PASS. The tests follow the file's own conventions: static void functions named as sentences, CHECK/CHECK_EQ, and the same #if !defined(TRACE_CLIENT_USE_LIBCURL) && !defined(__EMSCRIPTEN__) guard as parseStatusReadsCurlsWriteOutOrReportsNoAnswer.
• Sibling renames: N/A. Nothing was renamed.
• Docs: PASS. No behavior changed, so no README or header row needed updating. The pinned curlConfig lines match the README's "How a report is sent" (proto = "=http,https", 5 s timeouts, key only on stdin).
• Issue resolution: N/A. No Closes; the gap was found during triage.
• CI: PASS. All 11 jobs are green on head 1b72da3 (linux ×6, linux-libcurl, sanitizers ×2, macos, windows).
• Registered: PASS. Each of the three new names appears twice in the file (definition and tests[]), and the local run went from 41 tests to 44 tests, 0 failed checks.
• C++11: PASS. make test (C++11, -Wall -Wextra -pedantic -Werror) is clean locally, and both CI c++11 legs are green.
• No new dependency: PASS. The diff touches only the test file, adds no #include, and does not change Makefile or CMakeLists.txt.
• Never throws / Curl argv fixed / Version triple: N/A. trace_client.hpp is untouched. The user-agent expectation uses the TRACE_CLIENT_VERSION macro, so a version bump will not break it.
• Sanitizers: PASS. Both linux-sanitizers jobs are green.

Judgment notes:

• test/test_trace_client.cpp numberIsTheShortestJsonDecimalThatReadsBack: sets the global C++ locale to a comma-decimal facet and restores it before returning. Tests run one at a time, so no other test sees it. CHECK_EQ does not throw, so the restore always runs.
• test/test_trace_client.cpp curlConfigFixesEveryOptionAndQuotesEveryValue: this test pins the whole config on purpose, so any future change to the security-boundary lines has to update a test. The cost is that a deliberate config change will also need the test edited.

This review was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

---

drafted by Claude on behalf of Daniel Stephenson