fix(test): restore docker image tag suffix for native integration tests - #173
Open
adityamparikh wants to merge 2 commits into
Open
fix(test): restore docker image tag suffix for native integration tests#173adityamparikh wants to merge 2 commits into
adityamparikh wants to merge 2 commits into
Conversation
The native dockerIntegrationTest matrix (stdio/http) was failing because the three Docker integration tests resolved their image as `solr-mcp:<version>` with no `-native-<profile>` suffix, so Testcontainers 404'd pulling an image the native build never produces (it builds `solr-mcp:<version>-native-stdio` / `-native-http`). The MCP stdio test's `docker run` subprocess then exited immediately, surfacing as a 20s `initialize()` timeout. PR apache#139 removed the caller-side `+ System.getProperty("solr.mcp.docker.image.tag.suffix")` concatenation on the premise that `BuildInfoReader.getDockerImageName()` "already appends the same system property internally" — but it never did (it was just `String.format("%s:%s", artifact, version)`), so the suffix stopped being applied entirely. Make `getDockerImageName()` append the suffix internally, matching apache#139's documented intent and the test Javadoc. The property is unset on the JVM (Jib) path, defaulting to an empty string, so that tag is unchanged; the suffix is now applied exactly once. Also add the containerization tests and BuildInfoReader to native.yml's trigger paths so future changes to these files exercise the native matrix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Aditya Parikh <aditya.m.parikh@gmail.com>
Review follow-up on this PR. The suffix handling is otherwise exercised only by dockerIntegrationTest, which regular CI never runs — that gap is how PR apache#139 could silently drop the suffix and break the native image matrix. BuildInfoReaderTest pins the contract in the default test task. Also corrects DockerImageMcpClientStdioIntegrationTest's javadoc, which still described the pre-apache#139 call-site concatenation and a "-native" suffix value that never existed — the same doc-vs-code drift that caused the original regression. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Aditya Parikh <aditya.m.parikh@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The two
native Docker image + integrationjobs innative.ymlfail on every branch and PR (e.g. PR #159's run) with:The Paketo native build tags its image
solr-mcp:<version>-native-stdio/-native-http, but all three Docker integration tests resolve the plainsolr-mcp:<version>tag, which the native path never produces. The MCP stdio protocol test'sdocker runsubprocess exits immediately on the missing image, surfacing as a 20sinitialize()timeout instead.Root cause
PR #139 removed the caller-side
+ System.getProperty("solr.mcp.docker.image.tag.suffix")concatenation from the three test classes on the premise thatBuildInfoReader.getDockerImageName()"already appends the same system property internally" — but it never did (it was justString.format("%s:%s", artifact, version)). The suffix stopped being applied at all, even thoughbuild.gradle.ktsstill setssolr.mcp.docker.image.tag.suffixfordockerIntegrationTest -Pnativeand the test Javadoc still documents it as applied.Changes
BuildInfoReader.getDockerImageName()now appends thesolr.mcp.docker.image.tag.suffixsystem property (default""), so the suffix is applied exactly once, matching fix(test): stop double-appending docker image tag suffix #139's documented intent. The JVM (Jib) path is unchanged: the property is unset there, so the tag stayssolr-mcp:<version>.BuildInfoReadertonative.yml's trigger paths so future changes to the native validation suite re-run the native matrix.Verification (local, macOS arm64)
./gradlew spotlessCheck buildpasses (unit + integration tests; the JVM path resolves the same tag as before).bootBuildImage -Pnative+dockerIntegrationTest -Pnative→ tests resolvesolr-mcp:1.0.0-SNAPSHOT-native-stdioand pass:DockerImageMcpClientStdioIntegrationTest(39 tests, full MCP JSON-RPC workflow) andDockerImageStdioIntegrationTest(4 tests). The http leg as well:dockerIntegrationTest -Pnative -Pprofile=http→DockerImageHttpIntegrationTest(6 tests) passes againstsolr-mcp:1.0.0-SNAPSHOT-native-http.Merge order
This fixes the image resolution failure. With the tag fixed, the docker tests reach the actual native binary — which then needs the
DefaultMetaProviderreflection hint from companion PR #172 (theresourceSpecsbean is part of the real application context). Suggest merging the companion first, then this PR turns the native matrix fully green.🤖 Generated with Claude Code