Add a unit test for MultiTermsAggregator.buildEmptyAggregations (#7089)

[austintlee]

Description

A new unit test for the implementation of buildEmptyAggregation in MultiTermsAggregator

Issues Resolved

#7089

Check List

• New functionality includes testing.
  • [x ] All tests pass
• New functionality has been documented.
  • New functionality has javadoc added
• [x ] Commits are signed per the DCO using --signoff
• Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: FAILURE ❌
• URL: https://build.ci.opensearch.org/job/gradle-check/14621/
• CommitID: b511b78
  Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
  Is the failure a flaky test unrelated to your change?

[dblock]

@austintlee Thanks! What are your concerns with this UT? The amount of setup you had to do to get to the actual aggregation?

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: FAILURE ❌
• URL: https://build.ci.opensearch.org/job/gradle-check/14637/
• CommitID: b511b78
  Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
  Is the failure a flaky test unrelated to your change?

[austintlee]

@dblock yeah, the current implementation of MultiTermsAggregator makes it quite difficult to unit test it due to very heavy setup. Maybe this is generally the case for many classes in OpenSearch? Is there an issue for that that I can take a look at? Coming back to this, I think having a unit test is still worth it.

The reason why this PR is failing checks is because this commit needs to be built on top of the other commit that has the actual fix for the NPE which is in another PR. I can add this commit to #7318

[dblock]

@dblock yeah, the current implementation of MultiTermsAggregator makes it quite difficult to unit test it due to very heavy setup. Maybe this is generally the case for many classes in OpenSearch? Is there an issue for that that I can take a look at? Coming back to this, I think having a unit test is still worth it.

Absolutely. And thank you.

I don't know of an open issue for this, feel free to make one or just do it. Any code deleted will be appreciated!

The reason why this PR is failing checks is because this commit needs to be built on top of the other commit that has the actual fix for the NPE which is in another PR. I can add this commit to #7318

Yep, you should. A failing unit test that demonstrates a problem is a great step 1 for a fix, but you already did the step 2 of fixing the issue, so you're all set 😅 Close this and tack the UT to the other PR and iterate to green.

[opensearch-trigger-bot]

This PR is stalled because it has been open for 30 days with no activity. Remove stalled label or comment or this will be closed in 7 days.

[opensearch-trigger-bot]

This PR was closed because it has been stalled for 7 days with no activity.

[kotwanikunal]

Apologies. This PR was auto closed without reaching a resolution from the maintainers.
Re-opening to move it forward.
Thanks for your contributions to OpenSearch!

[github-actions]

Compatibility status:

Checks if related components are compatible with change b511b78

Incompatible components

Skipped components

Compatible components

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: FAILURE ❌
• URL: https://build.ci.opensearch.org/job/gradle-check/25629/
• CommitID: b511b78
  Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
  Is the failure a flaky test unrelated to your change?

[ashking94]

Closing this PR as the marked issue is closed already

[peternied]

@reta looks like you closed out the issue related to this change, what do you think of this change?

@austintlee did you still want to see about getting this change merged?

[austintlee]

We can close this one. As I mentioned above, I merged the change in this PR into another PR.