Skip to content

tests: serialize shared mock server access#815

Merged
jbeckwith-oai merged 1 commit into
mainfrom
codex/serialize-shared-mock-server
Jul 24, 2026
Merged

tests: serialize shared mock server access#815
jbeckwith-oai merged 1 commit into
mainfrom
codex/serialize-shared-mock-server

Conversation

@jbeckwith-oai

Copy link
Copy Markdown
Contributor

Summary

  • Serialize every test using the shared Steady mock server across JUnit threads and Gradle worker JVMs.
  • Add a process-local lock plus a URL-scoped OS file lock in TestServerExtension.
  • Keep the fix entirely in the test harness; no CI workflow changes.
  • Generalize and supersede the conversation-item-only approach in tests: serialize conversation item service tests #813.

Root cause

The generated output_text payload is valid Steady data. Seed 1 makes the failure reproducible under concurrent blocking/async service classes, but the isolated async test and high-volume in-process deserialization both pass. The failure occurs when separate Gradle workers access the persistent mock concurrently, so a JUnit-only resource lock cannot fully coordinate it. This is the same underlying race topology as the conversation-item flake covered by #813.

Validation

  • Reproduced the response service pair failure before the fix (immediate at seed 1).
  • Response blocking/async pair: 30/30 post-fix passes.
  • Conversation-item blocking/async pair: 30/30 post-fix passes.
  • :openai-java-core:lintKotlin passes.
  • Full JDK 21 core suite passes: 6,268 tests, 0 failures, 20 skipped.
  • Full Jackson compatibility suite passes: 5,629 tests, 0 failures.

Copilot AI review requested due to automatic review settings July 24, 2026 00:41

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the JUnit test harness to prevent concurrent access to the shared Steady mock server across both JUnit parallel execution and separate Gradle test-worker JVMs, addressing cross-process race flakes without changing CI workflows.

Changes:

  • Add a process-local fair ReentrantLock to serialize mock-server tests within a single JVM.
  • Add a URL-scoped OS-backed file lock (in ${java.io.tmpdir}) to coordinate serialization across Gradle worker JVMs.
  • Apply serialization by implementing InvocationInterceptor in TestServerExtension to wrap each test method invocation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jbeckwith-oai
jbeckwith-oai force-pushed the codex/serialize-shared-mock-server branch from 2a2d98f to 9405e5b Compare July 24, 2026 01:03
Copilot AI review requested due to automatic review settings July 24, 2026 01:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread openai-java-core/src/test/kotlin/com/openai/TestServerExtension.kt Outdated
Copilot AI review requested due to automatic review settings July 24, 2026 01:32
@jbeckwith-oai
jbeckwith-oai force-pushed the codex/serialize-shared-mock-server branch from 9405e5b to be3672b Compare July 24, 2026 01:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@jbeckwith-oai
jbeckwith-oai marked this pull request as ready for review July 24, 2026 01:46
@jbeckwith-oai
jbeckwith-oai requested a review from a team as a code owner July 24, 2026 01:46
@openai-sdks

openai-sdks Bot commented Jul 24, 2026

Copy link
Copy Markdown

OkTest Summary

237/237 SDK tests passed in 17.276s for Java SDK PR #815.

Test results — 42 files
Test Result Time
tests/chat-completions-complex-body.test.ts ✅ Passed 209ms
tests/chat-completions-create.test.ts ✅ Passed 669ms
tests/chat-completions-stream.test.ts ✅ Passed 379ms
tests/files-content-binary.test.ts ✅ Passed 229ms
tests/files-create-multipart.test.ts ✅ Passed 395ms
tests/files-list-pagination.test.ts ✅ Passed 330ms
tests/initialize-config.test.ts ✅ Passed 201ms
tests/instance-isolation.test.ts ✅ Passed 281ms
tests/models-list.test.ts ✅ Passed 174ms
tests/responses-background-lifecycle.test.ts ✅ Passed 867ms
tests/responses-body-method-errors.test.ts ✅ Passed 476ms
tests/responses-cancel-timeout.test.ts ✅ Passed 245ms
tests/responses-cancel.test.ts ✅ Passed 357ms
tests/responses-compact-retries.test.ts ✅ Passed 593ms
tests/responses-compact.test.ts ✅ Passed 256ms
tests/responses-create-advanced-stream.test.ts ✅ Passed 360ms
tests/responses-create-advanced.test.ts ✅ Passed 1.377s
tests/responses-create-disconnect.test.ts ✅ Passed 1.14s
tests/responses-create-errors.test.ts ✅ Passed 342ms
tests/responses-create-malformed-api-responses.test.ts ✅ Passed 280ms
tests/responses-create-retries.test.ts ✅ Passed 480ms
tests/responses-create-stream-failures.test.ts ✅ Passed 201ms
tests/responses-create-stream-timeout.test.ts ✅ Passed 244ms
tests/responses-create-stream-wire.test.ts ✅ Passed 6.51s
tests/responses-create-stream.test.ts ✅ Passed 687ms
tests/responses-create-terminal-states.test.ts ✅ Passed 328ms
tests/responses-create-timeout.test.ts ✅ Passed 291ms
tests/responses-create.test.ts ✅ Passed 249ms
tests/responses-delete.test.ts ✅ Passed 257ms
tests/responses-input-items-errors.test.ts ✅ Passed 266ms
tests/responses-input-items-list.test.ts ✅ Passed 299ms
tests/responses-input-items-options.test.ts ✅ Passed 636ms
tests/responses-input-tokens-count-timeout.test.ts ✅ Passed 231ms
tests/responses-input-tokens-count.test.ts ✅ Passed 402ms
tests/responses-malformed-inputs.test.ts ✅ Passed 5.351s
tests/responses-not-found-errors.test.ts ✅ Passed 323ms
tests/responses-parse.test.ts ✅ Passed 837ms
tests/responses-retrieve-retries.test.ts ✅ Passed 262ms
tests/responses-retrieve.test.ts ✅ Passed 316ms
tests/responses-stored-method-errors.test.ts ✅ Passed 1s
tests/retry-behavior.test.ts ✅ Passed 3.54s
tests/sdk-error-shape.test.ts ✅ Passed 461ms

View OkTest run #30059874669

SDK merge (4bebb0f71e05) · head (be3672b435aa) · base (a3a50ec79890) · OkTest (d2b5921fc22a)

@HAYDEN-OAI HAYDEN-OAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the shared mock-server locking path and the #813 context. The URL-scoped OS lock plus process-local lock cleanly serializes the current TestServerExtension-backed test methods across worker JVMs and JUnit threads. No actionable correctness or maintainability issues found.

@jbeckwith-oai
jbeckwith-oai merged commit f99db89 into main Jul 24, 2026
16 checks passed
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.

3 participants