Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Improve test coverage for MLHttpClientFactory.java #3644

Merged

Conversation

akolarkunnu
Copy link
Contributor

Description

Improved the test coverage by adding edge cases.
The if check (bytes.length != 4) is an unreachable code, Inet4Address always will have array of 4 bytes address.

Related Issues

Resolves #1379

Check List

  • New functionality includes testing.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.

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.

Improved the test coverage by adding edge cases.
The if check (bytes.length != 4) is an unreachable code, Inet4Address always will have array of 4 bytes address.

Resolves opensearch-project#1379

Signed-off-by: Abdul Muneer Kolarkunnu <[email protected]>
@akolarkunnu
Copy link
Contributor Author

akolarkunnu commented Mar 13, 2025

Code Coverage report after fix:
Code_Coverage

The if check (bytes.length != 4) is an unreachable code, Inet4Address always will have array of 4 bytes address.
All the misses of branch coverage is due to Hex format of ip addresses. It's not important for Inet4Address type.

@akolarkunnu akolarkunnu had a problem deploying to ml-commons-cicd-env-require-approval March 13, 2025 11:30 — with GitHub Actions Failure
@akolarkunnu akolarkunnu had a problem deploying to ml-commons-cicd-env-require-approval March 13, 2025 11:30 — with GitHub Actions Error
@akolarkunnu akolarkunnu had a problem deploying to ml-commons-cicd-env-require-approval March 13, 2025 11:30 — with GitHub Actions Error
@akolarkunnu akolarkunnu had a problem deploying to ml-commons-cicd-env-require-approval March 13, 2025 11:30 — with GitHub Actions Failure
@akolarkunnu
Copy link
Contributor Author

linux (21) - Known flaky org.opensearch.ml.rest.RestMLRemoteInferenceIT.testPredictWithAutoDeployAndTTL_RemoteModel #3544

Windows (23) - This is a test only change, failures are not related to the code changes.

@akolarkunnu
Copy link
Contributor Author

Hi Maintainers, Any comments, please.

@dhrubo-os
Copy link
Collaborator

Tests with failures:
 - org.opensearch.ml.rest.RestMLRemoteInferenceIT.testPredictWithAutoDeployAndTTL_RemoteModel

Can you take updates from main?

akolarkunnu and others added 2 commits March 18, 2025 10:33
Improved the test coverage by adding edge cases.
The if check (bytes.length != 4) is an unreachable code, Inet4Address always will have array of 4 bytes address.

Resolves opensearch-project#1379

Signed-off-by: Abdul Muneer Kolarkunnu <[email protected]>
@akolarkunnu akolarkunnu had a problem deploying to ml-commons-cicd-env-require-approval March 18, 2025 05:05 — with GitHub Actions Failure
@akolarkunnu akolarkunnu had a problem deploying to ml-commons-cicd-env-require-approval March 18, 2025 05:05 — with GitHub Actions Error
@akolarkunnu akolarkunnu had a problem deploying to ml-commons-cicd-env-require-approval March 18, 2025 05:05 — with GitHub Actions Error
@akolarkunnu akolarkunnu had a problem deploying to ml-commons-cicd-env-require-approval March 18, 2025 05:05 — with GitHub Actions Failure
@akolarkunnu
Copy link
Contributor Author

Tests with failures:
 - org.opensearch.ml.rest.RestMLRemoteInferenceIT.testPredictWithAutoDeployAndTTL_RemoteModel

Can you take updates from main?

Done.

@dhrubo-os
Copy link
Collaborator

* What went wrong:
Execution failed for task ':opensearch-ml-plugin:compileTestJava'.
> Compilation failed; see the compiler output below.
  Note: Some input files use unchecked or unsafe operations.
  Note: Recompile with -Xlint:unchecked for details.
  Note: Recompile with -Xlint:deprecation for details.
  Note: Some input files use or override a deprecated API.
  /__w/ml-commons/ml-commons/plugin/src/test/java/org/opensearch/ml/tools/PromptHandler.java:18: error: cannot find symbol
          return prompt.contains(llmThought().getQuestion());
                                             ^
    symbol:   method getQuestion()
    location: class LLMThought
  /__w/ml-commons/ml-commons/plugin/src/test/java/org/opensearch/ml/tools/PromptHandler.java:35: error: cannot find symbol
                  + this.llmThought().getAction()
                                     ^
    symbol:   method getAction()
    location: class LLMThought
  /__w/ml-commons/ml-commons/plugin/src/test/java/org/opensearch/ml/tools/PromptHandler.java:38: error: cannot find symbol
                  + this.llmThought().getActionInput()
                                     ^
    symbol:   method getActionInput()
    location: class LLMThought
  /__w/ml-commons/ml-commons/plugin/src/test/java/org/opensearch/ml/tools/MockLLM.java:40: error: cannot find symbol
                      llmResponse.setCompletion(promptHandler.response(prompt));
                                 ^
    symbol:   method setCompletion(String)
    location: variable llmResponse of type LLMResponse
  /__w/ml-commons/ml-commons/plugin/src/test/java/org/opensearch/ml/tools/VisualizationsToolIT.java:36: error: cannot find symbol
                      .builder()
                      ^
    symbol:   method builder()
    location: class LLMThought
  /__w/ml-commons/ml-commons/plugin/src/test/java/org/opensearch/ml/tools/VisualizationsToolIT.java:46: error: cannot find symbol
                      .builder()
                      ^
    symbol:   method builder()
    location: class LLMThought
  /__w/ml-commons/ml-commons/plugin/src/test/java/org/opensearch/ml/utils/TestHelper.java:15: error: cannot find symbol
  import static org.opensearch.cluster.node.DiscoveryNodeRole.SEARCH_ROLE;
  ^
    symbol:   static SEARCH_ROLE
    location: class DiscoveryNodeRole

This PR is merged. Please rebase again.

@akolarkunnu akolarkunnu temporarily deployed to ml-commons-cicd-env-require-approval March 19, 2025 03:22 — with GitHub Actions Inactive
@akolarkunnu akolarkunnu temporarily deployed to ml-commons-cicd-env-require-approval March 19, 2025 03:22 — with GitHub Actions Inactive
@akolarkunnu akolarkunnu temporarily deployed to ml-commons-cicd-env-require-approval March 19, 2025 03:22 — with GitHub Actions Inactive
@akolarkunnu akolarkunnu temporarily deployed to ml-commons-cicd-env-require-approval March 19, 2025 03:22 — with GitHub Actions Inactive
@akolarkunnu
Copy link
Contributor Author

This PR is merged. Please rebase again.

Ok, done.

@akolarkunnu akolarkunnu had a problem deploying to ml-commons-cicd-env-require-approval March 19, 2025 08:03 — with GitHub Actions Error
@akolarkunnu akolarkunnu had a problem deploying to ml-commons-cicd-env-require-approval March 19, 2025 08:03 — with GitHub Actions Failure
Improved the test coverage by adding edge cases.
The if check (bytes.length != 4) is an unreachable code, Inet4Address always will have array of 4 bytes address.

Resolves opensearch-project#1379

Signed-off-by: Abdul Muneer Kolarkunnu <[email protected]>
@akolarkunnu akolarkunnu temporarily deployed to ml-commons-cicd-env-require-approval March 19, 2025 14:09 — with GitHub Actions Inactive
@akolarkunnu akolarkunnu temporarily deployed to ml-commons-cicd-env-require-approval March 19, 2025 14:09 — with GitHub Actions Inactive
@akolarkunnu akolarkunnu temporarily deployed to ml-commons-cicd-env-require-approval March 19, 2025 14:10 — with GitHub Actions Inactive
@akolarkunnu akolarkunnu temporarily deployed to ml-commons-cicd-env-require-approval March 19, 2025 14:10 — with GitHub Actions Inactive
@akolarkunnu akolarkunnu had a problem deploying to ml-commons-cicd-env-require-approval March 19, 2025 16:13 — with GitHub Actions Failure
@akolarkunnu akolarkunnu had a problem deploying to ml-commons-cicd-env-require-approval March 19, 2025 16:13 — with GitHub Actions Failure
@dhrubo-os
Copy link
Collaborator

Thanks @akolarkunnu for working on this. Merging the PR.

@dhrubo-os dhrubo-os merged commit 48123e1 into opensearch-project:main Mar 20, 2025
9 of 13 checks passed
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.

[FEATURE] Improve test coverage for MLHttpClientFactory.java
3 participants