Mock http server for LLM; Integration test for visualization tool #92

Hailong-am · 2024-01-02T06:27:18Z

Description

mock LLM
integration test for visualization tool

Issues Resolved

[List any issues this PR will resolve]

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
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.

Signed-off-by: Hailong Cui <[email protected]>

codecov · 2024-01-02T06:31:20Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (34ae75f) 81.29% compared to head (0bb88eb) 83.68%.
Report is 4 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main      #92      +/-   ##
============================================
+ Coverage     81.29%   83.68%   +2.39%     
- Complexity      152      187      +35     
============================================
  Files            12       13       +1     
  Lines           882      944      +62     
  Branches        115      128      +13     
============================================
+ Hits            717      790      +73     
  Misses          106      106              
+ Partials         59       48      -11

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

xinyual · 2024-01-02T07:11:56Z

src/test/java/org/opensearch/integTest/ToolIntegrationTest.java

+        return JsonParser.parseString(resp).getAsJsonObject().get("model_id").getAsString();
+    }
+
+    private String setupConversationalAgent(String modelId) throws IOException {


I think a flow agent is enough to cover tool IT test like

skills/src/test/java/org/opensearch/integTest/NeuralSparseSearchToolIT.java

Line 94 in 34ae75f

String agentId = createAgent(registerAgentRequestBody);

you are right, conversational agent is used to test mock LLM server. To run integration test for tools, flow agent is enough.

zane-neo · 2024-01-03T07:02:38Z

src/test/java/org/opensearch/integTest/PromptHandler.java

+    }
+
+    String response(String prompt) {
+        if (prompt.contains("TOOL RESPONSE: ")) {


Will this TOOL RESPONSE: be used when writing actual ITs for a tool? If so, should we extract it to a common constant?

That depends. For most tools that use flow agent to do integration test, they will not interact with LLM. This default implementation is used to test conversational agent flow with mock LLM server. Based on this assumption, we can leave it as is.

zane-neo · 2024-01-03T07:04:01Z

src/test/java/org/opensearch/integTest/PromptHandler.java

+    static class ToolInput {
+        String question;
+        String toolType;
+        String toolInput;


Can we change toolType and toolInput to action and actionInput to be consistency?

sure, good suggestion.

zane-neo · 2024-01-03T07:11:44Z

src/test/java/org/opensearch/integTest/ToolIntegrationTest.java

+        StringBuilder sb = new StringBuilder();
+
+        try (BufferedReader br = new BufferedReader(new InputStreamReader(response.getEntity().getContent()))) {
+            String line;
+            while ((line = br.readLine()) != null) {
+                sb.append(line);
+            }
+        }
+        return sb.toString();


Is it possible to use third party method like IOUtils.toString() to simplify this code block?

done, update to OpenSearch built-in class instead org.opensearch.common.io.Streams.readAllLines(ins)

Signed-off-by: Hailong Cui <[email protected]>

* mock server and integTest for visualization Signed-off-by: Hailong Cui <[email protected]> * update rest status Signed-off-by: Hailong Cui <[email protected]> * add refresh Signed-off-by: Hailong Cui <[email protected]> * merge from main Signed-off-by: Hailong Cui <[email protected]> * rename variable name Signed-off-by: Hailong Cui <[email protected]> --------- Signed-off-by: Hailong Cui <[email protected]> (cherry picked from commit 4c76e4c) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

… (#102) * mock server and integTest for visualization * update rest status * add refresh * merge from main * rename variable name --------- (cherry picked from commit 4c76e4c) Signed-off-by: Hailong Cui <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…ensearch-project#92) (opensearch-project#102) * mock server and integTest for visualization * update rest status * add refresh * merge from main * rename variable name --------- (cherry picked from commit 4c76e4c) Signed-off-by: Hailong Cui <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Signed-off-by: yuye-aws <[email protected]>

Hailong-am added 4 commits January 2, 2024 13:01

mock server and integTest for visualization

943ee01

Signed-off-by: Hailong Cui <[email protected]>

update rest status

da5e961

Signed-off-by: Hailong Cui <[email protected]>

add refresh

bbe44fb

Signed-off-by: Hailong Cui <[email protected]>

merge from main

a91e705

Signed-off-by: Hailong Cui <[email protected]>

Hailong-am requested review from b4sjoo, dhrubo-os, jngz-es, model-collapse, rbhavna, ylwu-amzn, zane-neo, Zhangxunmt, xinyual, amitgalitz, jackiehanyang, owaiskazi19, ohltyler, joshpalis, dbwiddis, kaituo, lezzago, eirsep, sbcd90, zhichao-aws and joshuali925 as code owners January 2, 2024 06:27

opensearch-trigger-bot bot added backport 2.x infra labels Jan 2, 2024

xinyual approved these changes Jan 2, 2024

View reviewed changes

xinyual requested changes Jan 2, 2024

View reviewed changes

xinyual approved these changes Jan 2, 2024

View reviewed changes

zane-neo reviewed Jan 3, 2024

View reviewed changes

rename variable name

0bb88eb

Signed-off-by: Hailong Cui <[email protected]>

zane-neo approved these changes Jan 3, 2024

View reviewed changes

zane-neo merged commit 4c76e4c into opensearch-project:main Jan 3, 2024
13 checks passed

opensearch-trigger-bot bot mentioned this pull request Jan 3, 2024

[Backport 2.x] Mock http server for LLM; Integration test for visualization tool #102

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mock http server for LLM; Integration test for visualization tool #92

Mock http server for LLM; Integration test for visualization tool #92

Hailong-am commented Jan 2, 2024

codecov bot commented Jan 2, 2024 •

edited

Loading

xinyual Jan 2, 2024

Hailong-am Jan 2, 2024

zane-neo Jan 3, 2024

Hailong-am Jan 3, 2024

zane-neo Jan 3, 2024

Hailong-am Jan 3, 2024

zane-neo Jan 3, 2024

Hailong-am Jan 3, 2024

Mock http server for LLM; Integration test for visualization tool #92

Mock http server for LLM; Integration test for visualization tool #92

Conversation

Hailong-am commented Jan 2, 2024

Description

Issues Resolved

Check List

codecov bot commented Jan 2, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jan 2, 2024 •

edited

Loading