MLE-24685 Trying fixes for failing tests

[rjrudin]

Simplifying nodejs-optic-nodes - it doesn't need to do XML documents, we have other tests for that. Hoping this avoids the mysterious 500 error.

Then had Copilot fix the other two tests where they were failing with a message of "done called multiple times".

[Copilot]

Pull Request Overview

This PR fixes failing tests by simplifying the nodejs-optic-nodes test to remove XML document testing and addressing "done called multiple times" errors in two other tests.

• Simplified nodejs-optic-nodes test by removing XML document construction and related assertions
• Fixed "done called multiple times" error in nodejs-dmsdk-removeAllUris test by adding error handling
• Fixed "done called multiple times" error in nodejs-dmsdk-queryToTransformAll test by removing done() call from onBatchSuccess callback

Reviewed Changes

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

File	Description
test-complete/nodejs-optic-nodes.js	Removed XML document testing code and related assertions to simplify test
test-complete/nodejs-dmsdk-removeAllUris.js	Added error handling to prevent multiple done() calls
test-complete/nodejs-dmsdk-queryToTransformAll.js	Removed done() call from onBatchSuccess to prevent multiple calls
CONTRIBUTING.md	Added documentation about test-complete folder tests

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

Debug console.log statement should be removed before merging to production code.

Suggested change

	console.log(JSON.stringify(output, null, 2));
	expect(output.columns[1].name).to.equal('myJSON');
	expect(output.columns[1].type).to.equal('object');
	expect(output.columns[1].name).to.equal('myJSON');
	expect(output.columns[1].type).to.equal('object');
	expect(output.columns[1].type).to.equal('object');

[github-actions]

Copyright Validation Results
Total: 3 | Passed: 2 | Failed: 0 | Skipped: 1 | at: 2025-10-10 18:35:55 UTC | commit: 83aa6bc

⏭️ Skipped (Excluded) Files

• CONTRIBUTING.md

✅ Valid Files

• lib/responder.js
• test-complete/nodejs-optic-nodes.js

✅ All files have valid copyright headers!

[anu3990]

done is called here only in case of exception. The purpose of adding a try block is so that whenever something goes wrong here, it does not show timeout.

[rjrudin]

This is in regards to this intermittent error:

Copilot's reasoning was that "done()" would be called multiple times if assertions failed in 2 or more batches. Instead, the assertion errors should bubble up automatically , and onCompletion will handle calling "done()".

[anu3990]

We don't need to catch done. I know there are a lot of tests with this statement but I don't see much use.

[rjrudin]

Copilot added it - here's it's reasoning:

Purpose of .catch(done) on line 47

The .catch(done) serves as a safety net for the beforeEach hook to handle errors that occur in the writeAll() method itself (not just the write operation).

Two-layer error handling:

1. Primary: onCompletion callback handles errors during the write operation (via summary.error)
2. Secondary: .catch(done) handles errors from the writeAll() method itself

Without .catch(done), these scenarios would cause the test to hang:

• MarkLogic server connection failures
• Authentication errors
• Network timeouts
• Malformed requests that throw immediately

This prevents the "done() called multiple times" issue by ensuring:

• Only one error path calls done()
• Proper test failure instead of indefinite hanging
• Robust error handling for Jenkins environment differences

This is a standard pattern for async Mocha tests - always include .catch(done) to handle promise rejections outside of your main success/error logic.

[rjrudin]

Closing this. Going to simplify it down with a change to responder to include the request body for a 500.