test: remove brittle assertions and a null image default - #177
Open
adityamparikh wants to merge 1 commit into
Open
test: remove brittle assertions and a null image default#177adityamparikh wants to merge 1 commit into
adityamparikh wants to merge 1 commit into
Conversation
Verified findings from a CodeRabbit review. - DockerImageHttpIntegrationTest read solr.test.image with no default, so DockerImageName.parse received null whenever the property was unset. Use the same default as DockerImageMcpClientStdioIntegrationTest. - SampleClient asserted an exact tool count of 8; the server exposes 11, and the list went stale as soon as create-collection, add-fields and add-field-types were added. Assert a lower bound plus per-name presence. - CollectionServiceIntegrationTest required select.errors() and select.timeouts() to be null. Solr may omit these counters or report an explicit 0, and both mean "nothing went wrong"; the test depended on which. - McpClientIntegrationTestBase.assertNotError cast the first content item to TextContent, so a non-text error payload raised ClassCastException and hid the actual failure message. - Renamed SolrConfigTest to SolrConfigIntegrationTest. It is @tag("integration") and starts Testcontainers, so the old name contradicted the project's *Test = unit / *IntegrationTest = integration convention. Co-Authored-By: Claude Opus 5 (1M context) <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.
Verified findings from a CodeRabbit review. Test-only — no production code is touched.
DockerImageHttpIntegrationTestreadsolr.test.imagewith no default, soDockerImageName.parsereceivednullwhenever the property was unset. Now uses the same default asDockerImageMcpClientStdioIntegrationTest.SampleClientasserted an exact tool count of 8; the server exposes 11, and the list went stale as soon ascreate-collection,add-fieldsandadd-field-typeswere added. Now a lower bound plus per-name presence, so adding a tool will not break it.CollectionServiceIntegrationTestrequiredselect.errors()andselect.timeouts()to benull. Solr may omit these counters or report an explicit0, and both mean nothing went wrong — the test depended on which.McpClientIntegrationTestBase.assertNotErrorcast the first content item toTextContent, so a non-text error payload raisedClassCastExceptionand hid the actual failure message.SolrConfigTest→SolrConfigIntegrationTest: it is@Tag("integration")and starts Testcontainers, contradicting the project's*Test= unit /*IntegrationTest= integration convention documented inAGENTS.md.Verification
./gradlew buildon JDK 25, against real Solr via Testcontainers: BUILD SUCCESSFUL — 352 tests, 0 failures, 0 errors, 7 skipped. No spotless drift.🤖 Generated with Claude Code