Skip to content

Cover a base URL with a path, a redirecting server, and exponent-form values - #24

Merged
dmccoystephenson merged 1 commit into
mainfrom
feature/wire-format-edge-tests
Oct 21, 2026
Merged

dmccoystephenson merged 1 commit into
mainfrom
feature/wire-format-edge-tests

Conversation

@dmccoystephenson

Copy link
Copy Markdown
Member

Summary

Stage B (unit-test expansion) cycle covering wire-format edges that no existing test drove. Only TraceClientTest.java is changed; production code is untouched.

  • report_keepsAPathInTheBaseUrlInFrontOfApiMetrics: a base URL under a sub-path (.../trace/) is posted to /trace/api/metrics. Only the trailing slashes are dropped.
  • report_aRedirectIsNotFollowedAndIsLoggedFine (characterization): a server answering 301 is not followed. send() streams the body with a fixed length, so HttpURLConnection throws java.net.HttpRetryException: cannot retry due to redirection, in streaming mode. The report is dropped, and a FINE [trace] could not deliver ... line is logged. The redirect target is never requested.
  • json_writesValuesAsDoubleToStringDoesWhichIsStillValidJson: value is written as Double.toString writes it (1.0, 1.0E7, 1.0E20, 1.0E-5, -0.0, Double.MAX_VALUE). These are all valid JSON numbers, but the trace server has to accept the upper-case exponent form.

No tracking issue: these gaps were found during triage by comparing send()/json() with the test file.

Skipped this cycle: #13 and #15 both change what is sent on the wire or how the opt-out reads, and each asks for a maintainer decision. They are left open for that decision.

Test plan

  • mvn -B verify locally (Java 21): Tests run: 81, Failures: 0, Errors: 0, Skipped: 0, including the three new tests
  • Redirect message confirmed empirically on Java 21 before the assertion was written
  • CI matrix build (8), build (17), build (21) green. The Java 8 leg confirms the HttpRetryException behaviour on the floor JDK.

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

… values

Three wire-format edges no test drove: a base URL under a sub-path keeps
the path in front of /api/metrics; a 301 is not followed (the body is
streamed, so HttpURLConnection throws HttpRetryException) and the report
is dropped with a FINE "could not deliver" line; and json() writes values
as Double.toString does, exponent form included, which is still valid JSON.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dmccoystephenson

dmccoystephenson commented Oct 21, 2026 •

Copy link
Copy Markdown
Member Author

Self-review rubric (anchored on CI run 39580534220 and the diff):

  • Scope: PASS. git diff --stat origin/main shows one file, TraceClientTest.java, +72/-0. No production, README or pom change.
  • Tests-new: PASS (not applicable). No public method was added. Three tests were added for existing behaviour.
  • Tests-fix: not applicable. This is a characterization-only Stage B PR with no bug fix. The redirect test pins the current behaviour and does not change it.
  • Sibling structure: PASS. The tests follow the subject_behaviourInPlainWords naming and the Arrange/Act/Assert layout. The redirect test copies the local-HttpServer + RecordingHandler pattern of report_drainsAResponseBodyAndKeepsSendingAfterAnError, including stop(0) and close().
  • Sibling renames: not applicable. Nothing was renamed.
  • Docs: PASS. No behaviour changed, so no row of the docs table is affected. See the observation below.
  • Issue resolution: not applicable. There is no Closes. withVersion pushes an event already at MAX_TAGS over the server's tag limit #13 and A quoted enabled: "false" in the server-wide config does not turn reporting off #15 are skipped because they are maintainer decisions, as the PR body records.
  • CI: PASS. build (8), build (17) and build (21) are all green on the head commit.
  • No dependencies / single file / Java 8: PASS. pom.xml and src/main are untouched. The new code uses only Java 8 APIs (Collections.singletonList, CopyOnWriteArrayList), and the build (8) job confirms this.
  • Never throws / non-blocking / environment seam / version agreement / opt-out contract: not applicable. TraceClient.java is not modified.

Judgment calls flagged for the maintainer:

  • src/test/java/software/stephenson/trace/TraceClientTest.java report_aRedirectIsNotFollowedAndIsLoggedFine: this test pins behaviour that may not be wanted. A base URL that redirects, such as http:// to https://, loses every report, and the only sign is a FINE line, which most host loggers filter out. If redirects are ever followed or rejected at build(), this test is expected to flip. A follow-up issue is filed to document the requirement in the README "The wire format" section.
  • src/test/java/software/stephenson/trace/TraceClientTest.java json_writesValuesAsDoubleToStringDoesWhichIsStillValidJson: the expected strings depend on Double.toString, whose algorithm changed in JDK 19. The chosen values produce the same output on 8, 17 and 21, which the green jobs on all three confirm. A value added to this test later should be checked against the build (8) job.

Summary: three characterization tests on wire-format edges. CI is green across the matrix, and no production change was made.

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


drafted by Claude on behalf of Daniel Stephenson

@dmccoystephenson
dmccoystephenson merged commit d9b9b13 into main Oct 21, 2026
3 checks passed
@dmccoystephenson
dmccoystephenson deleted the feature/wire-format-edge-tests branch October 21, 2026 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant