fix tests

Author: MarketDataDev03

CLAUDE.md

@@ -94,6 +94,30 @@ When picking up new work, check this list before reaching for the SDK requiremen
 - `HttpTransport.buildUri` URL-encodes query-param values with `URLEncoder.encode(..., UTF_8)`, which is form-encoding semantics: spaces become `+`, not `%20`. Fine for today's typed params (dates, numerics) but a future endpoint that takes an arbitrary string (e.g. `symbol="BRK A"`) would round-trip differently against an RFC-3986-strict server. Switch to a path/query-segment-aware encoder when the first such param lands. Tracked as Issue #10 of the 2026-05-11 review.
 - `Retry-After` server header is parsed and respected by neither `RetryPolicy` nor `HttpTransport`. Today every retry uses the calculated exponential backoff (`min(1s × 2^N, 30s)`). Implementing the override needs the response headers to reach `RetryPolicy.backoffDelay`, which today only sees the attempt index — most natural path is to surface a `Duration` on `ServerError` (or thread it through a separate channel) when 5xx responses carry the header. Follow-up of the §9 work.
 
+## Test policy: unit tests are network-free
+
+Unit tests under `src/test/` **must not** make real network calls. Anything that needs the live
+API goes to `src/integrationTest/` (gated by `MARKETDATA_RUN_INTEGRATION_TESTS=true`).
+
+This matters because §5 made `MarketDataClient`'s constructor perform a `GET /user/` when
+`validateOnStartup=true` (the default). A test that builds `new MarketDataClient("any-token",
+null, null, true)` now hits `api.marketdata.app` over the network — exactly the failure mode
+this policy prevents.
+
+Concrete rules for unit tests that construct `MarketDataClient`:
+
+- Use the 4-arg constructor with `validateOnStartup=false` when verifying field wiring only.
+- Use a local server (`com.sun.net.httpserver.HttpServer` in-process) and point `baseUrl` at it
+  when you genuinely need to exercise the validate-on-startup flow — see
+  `MarketDataClientStartupValidationTest` for the pattern.
+- Tests that need the **real** API for smoke verification (e.g. the no-arg ctor end-to-end) go to
+  `src/integrationTest/.../MarketDataClientIT.java`.
+
+No automated enforcement today (a future ArchUnit rule or a grep step in CI could close that
+gap). Code review is the gate. Whenever you introduce a test that touches `new
+MarketDataClient(..., true)` with a default URL, ask: "does this need to live in `integrationTest`
+instead?"
+
 ## Acceptance checklist
 
 `docs/java-sdk-requirements.md` ends with an "Acceptance Checklist" mapping each Java-specific requirements section to verifiable items. Treat it as the definition of done for v1: when implementing, work toward making each box checkable, and use it as a self-review pass before declaring a section complete.

src/main/java/com/marketdata/sdk/HttpTransport.java

@@ -121,6 +121,14 @@ private static HttpClient defaultHttpClient() {
    * RetryPolicy}: retries 501–599 and IOException-shaped {@link NetworkError}s with exponential
    * backoff, surfaces every other failure immediately. Cancellation of the returned future bails
    * out of any pending backoff and propagates to the current in-flight attempt.
+   *
+   * <p><strong>Interaction between the §8 pre-flight check and the retry chain:</strong> each
+   * attempt re-runs the pre-flight check against the latest rate-limit snapshot. If a transient 5xx
+   * triggers a backoff and another concurrent caller's response drains the snapshot to {@code
+   * remaining=0} during that window, the next attempt's pre-flight fires and the chain ends with
+   * {@link com.marketdata.sdk.exception.RateLimitError} — not the upstream 5xx. The synthetic error
+   * reflects what the server would have returned anyway (a guaranteed 429 once the quota was gone),
+   * but callers that grep for "last server status" should know the substitution can happen.
    */
   <T> CompletableFuture<T> executeAsync(RequestSpec spec, Class<T> responseType) {
     CompletableFuture<T> result = new CompletableFuture<>();
@@ -219,11 +227,17 @@ private <T> CompletableFuture<T> executeOnce(RequestSpec spec, Class<T> response
     // seeds it, so this is a no-op for cold clients.
     RateLimits snapshot = latestRateLimits.get();
     if (snapshot != null && snapshot.remaining() <= 0) {
+      // `reset == EPOCH` means the response carried partial rate-limit headers (e.g.
+      // remaining without reset) and `RateLimitHeaders.parse` defaulted the missing field to
+      // 0. Rendering "1970-01-01" in the user-facing message looks like a bug; omit the
+      // suffix when the value is meaningless.
+      String resetSuffix =
+          snapshot.reset().equals(java.time.Instant.EPOCH)
+              ? ""
+              : " (resets at " + snapshot.reset() + ")";
       return CompletableFuture.failedFuture(
           new RateLimitError(
-              "Pre-flight rate-limit check failed: 0 credits remaining (resets at "
-                  + snapshot.reset()
-                  + ")",
+              "Pre-flight rate-limit check failed: 0 credits remaining" + resetSuffix,
               new ErrorContext(null, uri.toString(), null)));
     }
 

src/main/java/com/marketdata/sdk/MarketDataClient.java

@@ -214,6 +214,32 @@ private static String trimTrailingSlash(String url) {
    * <p>When the env var is unset (or blank, or malformed), the method is a no-op — the SDK inherits
    * whatever {@code java.util.logging} configuration the host JVM has, which is what library code
    * is expected to do.
+   *
+   * <p><strong>Side effect when the env var is set:</strong> the SDK logger's {@code
+   * useParentHandlers} is flipped to {@code false}, so handlers attached to the root logger no
+   * longer receive {@code com.marketdata.sdk.*} records. That prevents double emission (root
+   * handler with the JVM default formatter + our handler with the spec format) but means consumers
+   * that previously intercepted all logs through a root handler will stop seeing SDK records. To
+   * re-route, attach the consumer's handler directly to the SDK logger:
+   *
+   * <pre>{@code
+   * Logger.getLogger("com.marketdata.sdk").addHandler(myHandler);
+   * }</pre>
+   *
+   * <p><strong>Records emitted before this method completes</strong> use whatever formatter the JVM
+   * has by default, not {@link MarketDataLogFormatter}. Two paths produce such records:
+   *
+   * <ul>
+   *   <li>{@link Configuration#loadFromProcess()} runs first in the constructor and may warn about
+   *       a malformed {@code .env} file before {@code configureLogging} has had a chance to install
+   *       the formatter.
+   *   <li>The {@code "Ignoring invalid MARKETDATA_LOGGING_LEVEL=…"} warning below fires while the
+   *       method is bailing out — by definition before any handler gets installed.
+   * </ul>
+   *
+   * These edges are accepted: warnings still reach the user (default JVM level is {@code INFO} ≤
+   * {@code WARNING}); they just don't carry the spec timestamp prefix. Spec-shaped output starts
+   * with the first record emitted after this method returns normally.
    */
   static void configureLogging(Configuration config) {
     String requestedLevel = config.resolve(null, EnvVars.LOGGING_LEVEL);
@@ -230,16 +256,15 @@ static void configureLogging(Configuration config) {
     Logger sdkLogger = Logger.getLogger(SDK_LOGGER_NAME);
     sdkLogger.setLevel(parsed);
     if (!hasSdkHandler(sdkLogger)) {
-      Handler handler = new java.util.logging.ConsoleHandler();
-      handler.setFormatter(new MarketDataLogFormatter());
+      Handler handler = new MarketDataConsoleHandler();
       handler.setLevel(parsed);
       sdkLogger.addHandler(handler);
       // Bypass the root logger's default handlers; they would otherwise re-emit each record
       // with the JVM default formatter and produce duplicate, badly-shaped lines.
       sdkLogger.setUseParentHandlers(false);
     } else {
       for (Handler h : sdkLogger.getHandlers()) {
-        if (isSdkHandler(h)) {
+        if (h instanceof MarketDataConsoleHandler) {
           h.setLevel(parsed);
         }
       }
@@ -248,14 +273,10 @@ static void configureLogging(Configuration config) {
 
   private static boolean hasSdkHandler(Logger logger) {
     for (Handler h : logger.getHandlers()) {
-      if (isSdkHandler(h)) {
+      if (h instanceof MarketDataConsoleHandler) {
         return true;
       }
     }
     return false;
   }
-
-  private static boolean isSdkHandler(Handler h) {
-    return h.getFormatter() instanceof MarketDataLogFormatter;
-  }
 }

src/main/java/com/marketdata/sdk/MarketDataConsoleHandler.java

@@ -0,0 +1,19 @@
+package com.marketdata.sdk;
+
+import java.util.logging.ConsoleHandler;
+
+/**
+ * Marker subclass of {@link ConsoleHandler} so {@link MarketDataClient#configureLogging} can tell
+ * its own handler apart from anything the host application or another library installed on the same
+ * logger. The previous discriminator inspected {@code getFormatter() instanceof
+ * MarketDataLogFormatter}, which is structurally accurate today but trips up the rare case of a
+ * consumer attaching {@code MarketDataLogFormatter} to a vanilla {@code ConsoleHandler}.
+ *
+ * <p>No behavior beyond {@link ConsoleHandler}; the type identity is the whole point.
+ */
+final class MarketDataConsoleHandler extends ConsoleHandler {
+
+  MarketDataConsoleHandler() {
+    setFormatter(new MarketDataLogFormatter());
+  }
+}

src/test/java/com/marketdata/sdk/HttpTransportRetryTest.java

@@ -309,6 +309,29 @@ void preflightFailsImmediatelyWhenRemainingIsZero() {
         .isEqualTo(1);
   }
 
+  @Test
+  void preflightMessageOmitsResetSuffixWhenResetIsEpoch() {
+    // Partial rate-limit headers: server returned `remaining` without a `reset`, so
+    // RateLimitHeaders.parse defaulted reset to Instant.EPOCH. Rendering "(resets at
+    // 1970-01-01T00:00:00Z)" in the user-facing message looks like a bug — verify we omit it.
+    MultiResponseHttpClient client =
+        new MultiResponseHttpClient(
+            response(
+                200,
+                "{\"value\":\"ok\"}",
+                Map.of(
+                    "x-api-ratelimit-limit", "50000",
+                    "x-api-ratelimit-remaining", "0",
+                    "x-api-ratelimit-consumed", "50000")));
+
+    HttpTransport transport = newTransport(client, fastPolicy(3));
+    transport.executeSync(RequestSpec.get("ping").build(), Echo.class);
+
+    assertThatThrownBy(() -> transport.executeSync(RequestSpec.get("ping").build(), Echo.class))
+        .isInstanceOf(RateLimitError.class)
+        .hasMessage("Pre-flight rate-limit check failed: 0 credits remaining");
+  }
+
   @Test
   void preflightAllowsRequestWhenRemainingPositive() {
     MultiResponseHttpClient client =

src/test/java/com/marketdata/sdk/MarketDataClientLoggingTest.java

@@ -68,7 +68,7 @@ void appliesLevelFromEnvVar() {
 
     assertThat(sdkLogger.getLevel()).isEqualTo(Level.FINE);
     assertThat(Arrays.stream(sdkLogger.getHandlers()))
-        .anyMatch(h -> h.getFormatter() instanceof MarketDataLogFormatter);
+        .anyMatch(h -> h instanceof MarketDataConsoleHandler);
     assertThat(sdkLogger.getUseParentHandlers())
         .as("our handler bypasses parent so we don't double-emit with the JVM default formatter")
         .isFalse();
@@ -90,15 +90,15 @@ void calledTwiceDoesNotDuplicateHandler() {
 
     long sdkHandlers =
         Arrays.stream(sdkLogger.getHandlers())
-            .filter(h -> h.getFormatter() instanceof MarketDataLogFormatter)
+            .filter(h -> h instanceof MarketDataConsoleHandler)
             .count();
     assertThat(sdkHandlers)
         .as("second call should refresh the level, not add a second handler")
         .isEqualTo(1);
     assertThat(sdkLogger.getLevel()).isEqualTo(Level.WARNING);
     // The handler's own level should track the latest call too.
     Arrays.stream(sdkLogger.getHandlers())
-        .filter(h -> h.getFormatter() instanceof MarketDataLogFormatter)
+        .filter(h -> h instanceof MarketDataConsoleHandler)
         .forEach(h -> assertThat(h.getLevel()).isEqualTo(Level.WARNING));
   }
 
@@ -122,7 +122,72 @@ void noOpWhenEnvVarIsBlank() {
     MarketDataClient.configureLogging(newConfig(Map.of(EnvVars.LOGGING_LEVEL, "   ")));
 
     assertThat(Arrays.stream(sdkLogger.getHandlers()))
-        .noneMatch(h -> h.getFormatter() instanceof MarketDataLogFormatter);
+        .noneMatch(h -> h instanceof MarketDataConsoleHandler);
+  }
+
+  // ---------- end-to-end: env var → real log emission with spec shape ----------
+
+  /**
+   * Closes the loop between {@link MarketDataClient#configureLogging} and {@link
+   * MarketDataLogFormatter}. The other tests in this class verify the mechanics of configureLogging
+   * (level applied, handler installed) and {@code MarketDataLogFormatterTest} verifies the
+   * formatter shape in isolation — but neither alone catches a regression where the two stop
+   * composing (e.g. configureLogging starts using a different formatter, or the level filter blocks
+   * records the formatter would have rendered).
+   *
+   * <p>Strategy: let {@code configureLogging} install its handler, then add a parallel capturing
+   * handler that reuses the same {@link MarketDataLogFormatter} so we observe the same line that
+   * goes to stderr. Emit a record on a child of the SDK logger and assert the captured line matches
+   * the spec shape exactly.
+   */
+  @Test
+  void emittedRecordsAreFormattedPerSpec() {
+    MarketDataClient.configureLogging(newConfig(Map.of(EnvVars.LOGGING_LEVEL, "FINE")));
+
+    java.util.logging.Handler installed =
+        Arrays.stream(sdkLogger.getHandlers())
+            .filter(h -> h instanceof MarketDataConsoleHandler)
+            .findFirst()
+            .orElseThrow(() -> new AssertionError("configureLogging did not install its handler"));
+
+    CapturingHandler capture = new CapturingHandler();
+    capture.setFormatter(installed.getFormatter());
+    capture.setLevel(Level.ALL);
+    sdkLogger.addHandler(capture);
+
+    Logger child = Logger.getLogger("com.marketdata.sdk.example");
+    child.fine("hello world");
+
+    assertThat(capture.formattedLines)
+        .as("end-to-end logging must produce the spec-mandated shape")
+        .anyMatch(
+            line ->
+                line.matches(
+                    "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}Z"
+                        + " - com\\.marketdata\\.sdk\\.example - FINE - hello world"
+                        + java.util.regex.Pattern.quote(System.lineSeparator())));
+  }
+
+  /**
+   * Minimal {@link java.util.logging.Handler} that runs the configured formatter and stashes the
+   * rendered string. Tests assert against {@link #formattedLines}; the raw records are not exposed
+   * because the formatter is what we actually care about end-to-end.
+   */
+  private static final class CapturingHandler extends java.util.logging.Handler {
+    final java.util.List<String> formattedLines = new java.util.ArrayList<>();
+
+    @Override
+    public void publish(java.util.logging.LogRecord record) {
+      if (isLoggable(record)) {
+        formattedLines.add(getFormatter().format(record));
+      }
+    }
+
+    @Override
+    public void flush() {}
+
+    @Override
+    public void close() {}
   }
 
   @Test
@@ -133,6 +198,6 @@ void noOpAndWarnsWhenEnvVarIsInvalid() {
 
     assertThat(Arrays.stream(sdkLogger.getHandlers()))
         .as("invalid level must not install a handler — that would lie about the SDK's config")
-        .noneMatch(h -> h.getFormatter() instanceof MarketDataLogFormatter);
+        .noneMatch(h -> h instanceof MarketDataConsoleHandler);
   }
 }

src/test/java/com/marketdata/sdk/MarketDataClientStartupValidationTest.java

@@ -6,6 +6,7 @@
 
 import com.marketdata.sdk.exception.AuthenticationError;
 import com.marketdata.sdk.exception.NetworkError;
+import com.marketdata.sdk.exception.ServerError;
 import com.sun.net.httpserver.HttpHandler;
 import com.sun.net.httpserver.HttpServer;
 import java.io.IOException;
@@ -84,6 +85,29 @@ void surfaceAuthenticationErrorImmediatelyWhenTokenIsRejected() {
         .isEqualTo(1);
   }
 
+  /**
+   * Regression: {@code validateToken} must be single-attempt. {@code HttpTransport.executeAsync}
+   * normally retries 503 with exponential backoff, so if a refactor accidentally routes the startup
+   * probe through it the server would see 3 calls and (in this test) eventually succeed with a 200.
+   * Forcing the test to script enough responses to satisfy a 3-attempt chain — and then asserting
+   * {@code requestCount == 1} — fails determinístically the moment the path stops being
+   * single-shot.
+   */
+  @Test
+  void validateTokenDoesNotRetryTransient5xx() {
+    route(503, "{}");
+
+    assertThatThrownBy(() -> new MarketDataClient("token", baseUrl(), null, true))
+        .isInstanceOf(ServerError.class)
+        .satisfies(t -> assertThat(((ServerError) t).getStatusCode()).isEqualTo(503));
+
+    assertThat(requestCount.get())
+        .as(
+            "validateToken must call executeOnce (not executeAsync) so a refactor that"
+                + " accidentally enables retry on the startup path is caught here")
+        .isEqualTo(1);
+  }
+
   @Test
   void surfacesNetworkErrorWhenServerUnreachable() throws IOException {
     // Bind+close an ephemeral port to get a "nothing is listening" URL.