chore: Update docling-serve container image version to v1.29.0 - #615
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the default Docling Serve Testcontainers image tag used by docling-testcontainers to v1.29.0, ensuring the module’s out-of-the-box container startup targets the newer Docling Serve release.
Changes:
- Bump
DoclingServeContainerConfig.DOCLING_IMAGE_VERSIONfromv1.26.0tov1.29.0.
6333941 to
a3626b1
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (1)
docling-testcontainers/src/main/java/ai/docling/testcontainers/serve/config/DoclingServeContainerConfig.java:229
- Brace/else layout here is inconsistent with the repository’s Spotless/Eclipse Java formatter configuration and is likely to be rewritten by
spotlessApply(or failspotlessCheck). Please format this as} else {on the same line to match the rest of the codebase.
if (containerEnv != null) {
this.containerEnv = new HashMap<>(containerEnv);
}
else {
this.containerEnv = null;
Signed-off-by: Eric Deandrea <eric.deandrea@ibm.com>
a3626b1 to
b0b2125
Compare
:java_duke: JaCoCo coverage report
|
|
||||||||||||||
…t builders in test cases
…nsistency Signed-off-by: Eric Deandrea <eric.deandrea@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (4)
docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/AbstractDoclingServeClientTests.java:101
- Import ordering does not match the repository Spotless import order (see
.spotless/import-order.txt:orgthencomthenai). Thecom.github.tomakehurst.wiremock.*imports are currently after theai.docling.*imports, which will causespotlessCheckto fail.
import ai.docling.serve.api.validation.ValidationErrorDetail;
import ai.docling.serve.api.validation.ValidationException;
import ai.docling.serve.client.DoclingServeClient.DoclingServeClientBuilder;
import ai.docling.testcontainers.serve.DoclingServeContainer;
import ai.docling.testcontainers.serve.config.DoclingServeContainerConfig;
docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/AbstractDoclingServeClientTests.java:1839
- This assertion block contains commented-out expectations for
input/context, which makes the test harder to read and maintain. If these fields are no longer stable across docling-serve versions, prefer explicitly ignoring them in the recursive comparison and remove the commented code.
// ValidationErrorContext.builder()
// .expectedSchemes("'http' or 'https'")
// .build()
// ).build()
);
docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/AbstractDoclingServeClientTests.java:1766
- This assertion block contains commented-out expectations for
input/context, which makes the test harder to read and maintain. If these fields are no longer stable across docling-serve versions, prefer explicitly ignoring them in the recursive comparison and remove the commented code.
// ValidationErrorContext.builder()
// .expectedSchemes("'http' or 'https'")
// .build()
// ).build()
);
docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/AbstractDoclingServeClientTests.java:524
- This assertion block contains commented-out expectations for
input/context, which makes the test harder to read and maintain. If these fields are no longer stable across docling-serve versions, prefer explicitly ignoring them in the recursive comparison and remove the commented code.
This issue also appears in the following locations of the same file:
- line 1762
- line 1835
// .expectedSchemes("'http' or 'https'")
// .build()
// ).build()
);
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (4)
docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/AbstractDoclingServeClientTests.java:1764
- This validation-error test still contains commented-out assertions for
input/context, which leaves dead code and obscures what behavior is being verified. Prefer an explicit assertion strategy (e.g., compare only the stable fields) rather than commented code.
// .input(file.toUri().toString())
// .context(
// ValidationErrorContext.builder()
// .expectedSchemes("'http' or 'https'")
// .build()
docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/AbstractDoclingServeClientTests.java:1837
- This validation-error test includes commented-out builder calls for
input/context. Commented code in assertions makes the test intent ambiguous and tends to accumulate. Make the comparison explicit (compare only stable fields, or update expected values) and remove the commented block.
// .input(file.toUri().toString())
// .context(
// ValidationErrorContext.builder()
// .expectedSchemes("'http' or 'https'")
// .build()
docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/AbstractDoclingServeClientTests.java:521
- The validation-error assertion currently contains commented-out builder calls for
input/context, leaving dead code in the test and making it unclear which fields are intended to be asserted. Instead of commented code, make the assertion explicit (e.g., compare onlytype/message/locations, or update expected values to match the actual response).
This issue also appears in the following locations of the same file:
- line 1760
- line 1833
// .input(file.toUri().toString())
// .context(
// ValidationErrorContext.builder()
// .expectedSchemes("'http' or 'https'")
// .build()
docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/AbstractDoclingServeClientTests.java:125
- The PR title/description says this is only a docling-serve container image version bump, but this file includes broad reformatting and behavior changes in tests (e.g., relaxed validation-error assertions). Please split the formatting/test changes into a separate PR (or clearly justify why they are required for the image bump) to keep the upgrade reviewable.
""".formatted(
getClass().getName(), context.getTestMethod().map(Method::getName).orElse(""), Optional.ofNullable(cause).map(Throwable::getMessage).orElse(""), doclingContainer
.getLogs());
|
HTML test reports are available as workflow artifacts (zipped HTML). • Download: Artifacts for this run |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
HTML test reports are available as workflow artifacts (zipped HTML). • Download: Artifacts for this run |
Update docling-serve container image version to v1.29.0