Skip to content

MLE-33565 Resolve the 5 remaining disabled tests - #1984

Open
hmadala001 wants to merge 3 commits into
developfrom
MLE-33565
Open

hmadala001 wants to merge 3 commits into
developfrom
MLE-33565

Conversation

@hmadala001

Copy link
Copy Markdown

Stabilised:

  • TestOpticOnViews.testgroupByUnion now uses order-independent assertions
  • BulkIOInputCallerTest.bulkInputEndpointTestWithAnyDocument now uses its own collections, cleans up beforehand, and asserts on the union of written inputs rather than on how batches are distributed across call contexts

Retired:

  • ClientApiFunctionalTest.TestE2ENumberOfConcurrentUsers: tests the app server concurrent-request-limit feature, not the Java Client; flaky on Jenkins
  • DocumentMetadataHandleTest.testMetadataPropertiesExtraction (and test.bin): tests a v1/documents feature and causes OOM in CI
  • SetMaxBatchesTest.testWithQuery: setMaxBatches with a query is a known limitation

Stabilised:
- TestOpticOnViews.testgroupByUnion now uses order-independent assertions
- BulkIOInputCallerTest.bulkInputEndpointTestWithAnyDocument now uses its own
  collections, cleans up beforehand, and asserts on the union of written
  inputs rather than on how batches are distributed across call contexts

Retired:
- ClientApiFunctionalTest.TestE2ENumberOfConcurrentUsers: tests the app server
  concurrent-request-limit feature, not the Java Client; flaky on Jenkins
- DocumentMetadataHandleTest.testMetadataPropertiesExtraction (and test.bin):
  tests a v1/documents feature and causes OOM in CI
- SetMaxBatchesTest.testWithQuery: setMaxBatches with a query is a known
  limitation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI 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.

🟢 Approval recommended

No active review issues remain for the test-only changes that re-enable two tests and remove three disabled tests.

No open issues 🎉

What changed in this PR

Resolves five disabled tests in the Java Client’s core and functional test suites by stabilizing two and retiring three. No production code changes are included.

Changes:

  • Re-enables the Optic union test with order-independent assertions.
  • Re-enables the bulk-input test with isolated collections, cleanup, and assertions that every input is written exactly once.
  • Removes three disabled tests covering server behavior or a known limitation, plus an unused fixture and helper.
File Description
marklogic-client-api/​src/​test/​resources/​test.bin Removes the retired metadata test’s binary fixture.
marklogic-client-api/​src/​test/​java/​com/​marklogic/​client/​test/​io/​DocumentMetadataHandleTest.java Removes the disabled metadata-extraction test and related imports.
marklogic-client-api/​src/​test/​java/​com/​marklogic/​client/​test/​dataservices/​BulkIOInputCallerTest.java Isolates and re-enables the bulk-input test with distribution-independent assertions.
marklogic-client-api-functionaltests/​src/​test/​java/​com/​marklogic/​client/​fastfunctest/​TestOpticOnViews.java Re-enables the union test and replaces order-sensitive row checks.
marklogic-client-api-functionaltests/​src/​test/​java/​com/​marklogic/​client/​fastfunctest/​datamovement/​SetMaxBatchesTest.java Removes the disabled query-based test while retaining iterator coverage.
marklogic-client-api-functionaltests/​src/​test/​java/​com/​marklogic/​client/​fastfunctest/​ClientApiFunctionalTest.java Removes the disabled concurrent-request-limit test and its configuration helper.

Review effort: Balanced

@hmadala001
hmadala001 marked this pull request as draft November 3, 2026 05:16
}

@AfterAll
public static void cleanup(){

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.

I would rework this test class to extend AbstractClientTest which will clean the database before the test starts instead of after. That's a better practice because if the database is cleaned afterwards and the test fails, it's much harder to debug since the data is all gone. Extending that class also avoids duplication of deletion code like this.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

sure , BulkIOInputCallerTest now extends AbstractClientTest. The class's own deleteTestDocuments helper and its calls (in the test and in @afterall) are removed, so the database is cleaned before each test by the shared base class. @afterall now only removes the endpoint module we upload

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.

Sure, though that's actually not necessary either. I know now the dataservices tests do a lot of that - loading modules and then deleting them - but they could all be setup via the test-app as part of mlDeploy, and then the tests wouldn't need all the additional code. Not important to change, just an FYI.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

sure , oky !!

Addresses review feedback: AbstractClientTest cleans the database before each
test instead of after, so data from a failing test is still available for
debugging, and it removes the duplicated document-deletion code.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@hmadala001
hmadala001 requested a review from rjrudin November 4, 2026 07:37
rjrudin
rjrudin previously approved these changes Nov 4, 2026
@hmadala001
hmadala001 marked this pull request as ready for review November 5, 2026 09:27
The Validate Copyright Headers check requires modified files to carry the
current year.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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