Skip to content

Commit b3ead80

Browse files
committed
Clean up test code for better separation and reduce repetition
1 parent 2b4b12b commit b3ead80

File tree

3 files changed

+105
-168
lines changed

3 files changed

+105
-168
lines changed

foundation-models/openai/src/test/java/com/sap/ai/sdk/foundationmodels/openai/BaseOpenAiClientTest.java

Lines changed: 93 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,38 @@
11
package com.sap.ai.sdk.foundationmodels.openai;
22

33
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
4+
import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl;
5+
import static com.github.tomakehurst.wiremock.client.WireMock.badRequest;
46
import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
7+
import static com.github.tomakehurst.wiremock.client.WireMock.noContent;
8+
import static com.github.tomakehurst.wiremock.client.WireMock.okXml;
59
import static com.github.tomakehurst.wiremock.client.WireMock.post;
10+
import static com.github.tomakehurst.wiremock.client.WireMock.serverError;
611
import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
712
import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
813
import static org.mockito.ArgumentMatchers.any;
914
import static org.mockito.Mockito.doReturn;
1015
import static org.mockito.Mockito.mock;
1116
import static org.mockito.Mockito.spy;
1217

18+
import com.fasterxml.jackson.core.JsonParseException;
1319
import com.fasterxml.jackson.databind.ObjectMapper;
1420
import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
1521
import com.github.tomakehurst.wiremock.junit5.WireMockTest;
22+
import com.github.tomakehurst.wiremock.stubbing.Scenario;
1623
import com.sap.cloud.sdk.cloudplatform.connectivity.ApacheHttpClient5Accessor;
1724
import com.sap.cloud.sdk.cloudplatform.connectivity.ApacheHttpClient5Cache;
1825
import com.sap.cloud.sdk.cloudplatform.connectivity.DefaultHttpDestination;
1926
import java.io.IOException;
2027
import java.io.InputStream;
2128
import java.util.Objects;
2229
import java.util.function.Function;
30+
import javax.annotation.Nonnull;
2331
import org.apache.hc.client5.http.classic.HttpClient;
2432
import org.apache.hc.core5.http.ContentType;
2533
import org.apache.hc.core5.http.io.entity.InputStreamEntity;
2634
import org.apache.hc.core5.http.message.BasicClassicHttpResponse;
35+
import org.assertj.core.api.SoftAssertions;
2736
import org.junit.jupiter.api.AfterEach;
2837
import org.junit.jupiter.api.BeforeEach;
2938

@@ -55,6 +64,90 @@ static void stubForEmbedding() {
5564
.withHeader("Content-Type", "application/json")));
5665
}
5766

67+
static void stubForChatCompletionTool() {
68+
stubFor(
69+
post(urlPathEqualTo("/chat/completions"))
70+
.willReturn(
71+
aResponse()
72+
.withHeader("Content-Type", "application/json")
73+
.withBodyFile("chatCompletionToolResponse.json")));
74+
}
75+
76+
static void stubForErrorHandling() {
77+
final var errorJson =
78+
"""
79+
{ "error": { "code": null, "message": "foo", "type": "invalid stuff" } }
80+
""";
81+
stubFor(
82+
post(anyUrl())
83+
.inScenario("Errors")
84+
.whenScenarioStateIs(Scenario.STARTED)
85+
.willReturn(serverError())
86+
.willSetStateTo("1"));
87+
stubFor(
88+
post(anyUrl())
89+
.inScenario("Errors")
90+
.whenScenarioStateIs("1")
91+
.willReturn(
92+
badRequest().withBody(errorJson).withHeader("Content-type", "application/json"))
93+
.willSetStateTo("2"));
94+
stubFor(
95+
post(anyUrl())
96+
.inScenario("Errors")
97+
.whenScenarioStateIs("2")
98+
.willReturn(
99+
badRequest()
100+
.withBody("{ broken json")
101+
.withHeader("Content-type", "application/json"))
102+
.willSetStateTo("3"));
103+
stubFor(
104+
post(anyUrl())
105+
.inScenario("Errors")
106+
.whenScenarioStateIs("3")
107+
.willReturn(okXml("<xml></xml>"))
108+
.willSetStateTo("4"));
109+
stubFor(post(anyUrl()).inScenario("Errors").whenScenarioStateIs("4").willReturn(noContent()));
110+
}
111+
112+
static void assertForErrorHandling(@Nonnull final Runnable request) {
113+
114+
final var softly = new SoftAssertions();
115+
116+
softly
117+
.assertThatThrownBy(request::run)
118+
.describedAs("Server errors should be handled")
119+
.isInstanceOf(OpenAiClientException.class)
120+
.hasMessageContaining("500");
121+
122+
softly
123+
.assertThatThrownBy(request::run)
124+
.describedAs("Error objects from OpenAI should be interpreted")
125+
.isInstanceOf(OpenAiClientException.class)
126+
.hasMessageContaining("error message: 'foo'");
127+
128+
softly
129+
.assertThatThrownBy(request::run)
130+
.describedAs("Failures while parsing error message should be handled")
131+
.isInstanceOf(OpenAiClientException.class)
132+
.hasMessageContaining("400")
133+
.extracting(e -> e.getSuppressed()[0])
134+
.isInstanceOf(JsonParseException.class);
135+
136+
softly
137+
.assertThatThrownBy(request::run)
138+
.describedAs("Non-JSON responses should be handled")
139+
.isInstanceOf(OpenAiClientException.class)
140+
.hasMessageContaining("Failed to parse");
141+
142+
softly
143+
.assertThatThrownBy(request::run)
144+
.describedAs("Empty responses should be handled")
145+
.isInstanceOf(OpenAiClientException.class)
146+
.hasMessageContaining("was empty");
147+
148+
softly.assertAll();
149+
}
150+
58151
@BeforeEach
59152
void setup(WireMockRuntimeInfo server) {
60153
final DefaultHttpDestination destination =
@@ -86,13 +179,4 @@ InputStream stubStreamChatCompletion(String responseFile) throws IOException {
86179

87180
return inputStream;
88181
}
89-
90-
static void stubForChatCompletionTool() {
91-
stubFor(
92-
post(urlPathEqualTo("/chat/completions"))
93-
.willReturn(
94-
aResponse()
95-
.withHeader("Content-Type", "application/json")
96-
.withBodyFile("chatCompletionToolResponse.json")));
97-
}
98182
}

foundation-models/openai/src/test/java/com/sap/ai/sdk/foundationmodels/openai/NewOpenAiClientTest.java

Lines changed: 3 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,12 @@
22

33
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
44
import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl;
5-
import static com.github.tomakehurst.wiremock.client.WireMock.badRequest;
65
import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
76
import static com.github.tomakehurst.wiremock.client.WireMock.equalToJson;
87
import static com.github.tomakehurst.wiremock.client.WireMock.exactly;
9-
import static com.github.tomakehurst.wiremock.client.WireMock.noContent;
108
import static com.github.tomakehurst.wiremock.client.WireMock.okJson;
11-
import static com.github.tomakehurst.wiremock.client.WireMock.okXml;
129
import static com.github.tomakehurst.wiremock.client.WireMock.post;
1310
import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor;
14-
import static com.github.tomakehurst.wiremock.client.WireMock.serverError;
1511
import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
1612
import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
1713
import static com.github.tomakehurst.wiremock.client.WireMock.verify;
@@ -25,10 +21,6 @@
2521
import static org.mockito.Mockito.times;
2622
import static org.mockito.Mockito.when;
2723

28-
import com.fasterxml.jackson.core.JsonParseException;
29-
import com.github.tomakehurst.wiremock.stubbing.Scenario;
30-
import com.sap.ai.sdk.foundationmodels.openai.model.OpenAiChatCompletionParameters;
31-
import com.sap.ai.sdk.foundationmodels.openai.model.OpenAiChatMessage;
3224
import com.sap.ai.sdk.foundationmodels.openai.model2.ChatCompletionNamedToolChoice;
3325
import com.sap.ai.sdk.foundationmodels.openai.model2.ChatCompletionNamedToolChoiceFunction;
3426
import com.sap.ai.sdk.foundationmodels.openai.model2.ChatCompletionTool;
@@ -48,7 +40,6 @@
4840
import java.util.stream.Stream;
4941
import javax.annotation.Nonnull;
5042
import lombok.SneakyThrows;
51-
import org.assertj.core.api.SoftAssertions;
5243
import org.junit.jupiter.api.DisplayName;
5344
import org.junit.jupiter.api.Test;
5445
import org.junit.jupiter.params.ParameterizedTest;
@@ -78,87 +69,15 @@ private static Runnable[] errorHandlingCalls() {
7869
client
7970
.streamChatCompletionDeltas(new OpenAiChatCompletionRequest(""))
8071
// the stream needs to be consumed to parse the response
81-
.forEach(System.out::println),
82-
() -> client.chatCompletion(""),
83-
() ->
84-
client.chatCompletion(
85-
new OpenAiChatCompletionParameters()
86-
.addMessages(new OpenAiChatMessage.OpenAiChatUserMessage().addText("")))
72+
.forEach(System.out::println)
8773
};
8874
}
8975

9076
@ParameterizedTest
9177
@MethodSource("errorHandlingCalls")
9278
void chatCompletionErrorHandling(@Nonnull final Runnable request) {
93-
final var errorJson =
94-
"""
95-
{ "error": { "code": null, "message": "foo", "type": "invalid stuff" } }
96-
""";
97-
stubFor(
98-
post(anyUrl())
99-
.inScenario("Errors")
100-
.whenScenarioStateIs(Scenario.STARTED)
101-
.willReturn(serverError())
102-
.willSetStateTo("1"));
103-
stubFor(
104-
post(anyUrl())
105-
.inScenario("Errors")
106-
.whenScenarioStateIs("1")
107-
.willReturn(
108-
badRequest().withBody(errorJson).withHeader("Content-type", "application/json"))
109-
.willSetStateTo("2"));
110-
stubFor(
111-
post(anyUrl())
112-
.inScenario("Errors")
113-
.whenScenarioStateIs("2")
114-
.willReturn(
115-
badRequest()
116-
.withBody("{ broken json")
117-
.withHeader("Content-type", "application/json"))
118-
.willSetStateTo("3"));
119-
stubFor(
120-
post(anyUrl())
121-
.inScenario("Errors")
122-
.whenScenarioStateIs("3")
123-
.willReturn(okXml("<xml></xml>"))
124-
.willSetStateTo("4"));
125-
stubFor(post(anyUrl()).inScenario("Errors").whenScenarioStateIs("4").willReturn(noContent()));
126-
127-
final var softly = new SoftAssertions();
128-
129-
softly
130-
.assertThatThrownBy(request::run)
131-
.describedAs("Server errors should be handled")
132-
.isInstanceOf(OpenAiClientException.class)
133-
.hasMessageContaining("500");
134-
135-
softly
136-
.assertThatThrownBy(request::run)
137-
.describedAs("Error objects from OpenAI should be interpreted")
138-
.isInstanceOf(OpenAiClientException.class)
139-
.hasMessageContaining("error message: 'foo'");
140-
141-
softly
142-
.assertThatThrownBy(request::run)
143-
.describedAs("Failures while parsing error message should be handled")
144-
.isInstanceOf(OpenAiClientException.class)
145-
.hasMessageContaining("400")
146-
.extracting(e -> e.getSuppressed()[0])
147-
.isInstanceOf(JsonParseException.class);
148-
149-
softly
150-
.assertThatThrownBy(request::run)
151-
.describedAs("Non-JSON responses should be handled")
152-
.isInstanceOf(OpenAiClientException.class)
153-
.hasMessageContaining("Failed to parse");
154-
155-
softly
156-
.assertThatThrownBy(request::run)
157-
.describedAs("Empty responses should be handled")
158-
.isInstanceOf(OpenAiClientException.class)
159-
.hasMessageContaining("was empty");
160-
161-
softly.assertAll();
79+
stubForErrorHandling();
80+
assertForErrorHandling(request);
16281
}
16382

16483
@Test

foundation-models/openai/src/test/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClientTest.java

Lines changed: 9 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@
77
import static org.assertj.core.api.Assertions.assertThatThrownBy;
88
import static org.mockito.Mockito.times;
99

10-
import com.fasterxml.jackson.core.JsonParseException;
11-
import com.github.tomakehurst.wiremock.stubbing.Scenario;
1210
import com.sap.ai.sdk.foundationmodels.openai.model.OpenAiChatCompletionDelta;
1311
import com.sap.ai.sdk.foundationmodels.openai.model.OpenAiChatCompletionFunction;
1412
import com.sap.ai.sdk.foundationmodels.openai.model.OpenAiChatCompletionOutput;
@@ -23,7 +21,6 @@
2321
import java.util.stream.Stream;
2422
import javax.annotation.Nonnull;
2523
import lombok.SneakyThrows;
26-
import org.assertj.core.api.SoftAssertions;
2724
import org.junit.jupiter.api.Test;
2825
import org.junit.jupiter.params.ParameterizedTest;
2926
import org.junit.jupiter.params.provider.MethodSource;
@@ -35,84 +32,21 @@ private static Runnable[] errorHandlingCalls() {
3532
return new Runnable[] {
3633
() -> client.chatCompletion(""),
3734
() ->
38-
client.chatCompletion(
39-
new OpenAiChatCompletionParameters()
40-
.addMessages(new OpenAiChatUserMessage().addText("")))
35+
client
36+
.streamChatCompletionDeltas(
37+
new OpenAiChatCompletionParameters()
38+
.addMessages(new OpenAiChatUserMessage().addText("")))
39+
// the stream needs to be consumed to parse the response
40+
.forEach(System.out::println),
4141
};
4242
}
4343

4444
@ParameterizedTest
4545
@MethodSource("errorHandlingCalls")
4646
void chatCompletionErrorHandling(@Nonnull final Runnable request) {
47-
final var errorJson =
48-
"""
49-
{ "error": { "code": null, "message": "foo", "type": "invalid stuff" } }
50-
""";
51-
stubFor(
52-
post(anyUrl())
53-
.inScenario("Errors")
54-
.whenScenarioStateIs(Scenario.STARTED)
55-
.willReturn(serverError())
56-
.willSetStateTo("1"));
57-
stubFor(
58-
post(anyUrl())
59-
.inScenario("Errors")
60-
.whenScenarioStateIs("1")
61-
.willReturn(
62-
badRequest().withBody(errorJson).withHeader("Content-type", "application/json"))
63-
.willSetStateTo("2"));
64-
stubFor(
65-
post(anyUrl())
66-
.inScenario("Errors")
67-
.whenScenarioStateIs("2")
68-
.willReturn(
69-
badRequest()
70-
.withBody("{ broken json")
71-
.withHeader("Content-type", "application/json"))
72-
.willSetStateTo("3"));
73-
stubFor(
74-
post(anyUrl())
75-
.inScenario("Errors")
76-
.whenScenarioStateIs("3")
77-
.willReturn(okXml("<xml></xml>"))
78-
.willSetStateTo("4"));
79-
stubFor(post(anyUrl()).inScenario("Errors").whenScenarioStateIs("4").willReturn(noContent()));
80-
81-
final var softly = new SoftAssertions();
82-
83-
softly
84-
.assertThatThrownBy(request::run)
85-
.describedAs("Server errors should be handled")
86-
.isInstanceOf(OpenAiClientException.class)
87-
.hasMessageContaining("500");
88-
89-
softly
90-
.assertThatThrownBy(request::run)
91-
.describedAs("Error objects from OpenAI should be interpreted")
92-
.isInstanceOf(OpenAiClientException.class)
93-
.hasMessageContaining("error message: 'foo'");
94-
95-
softly
96-
.assertThatThrownBy(request::run)
97-
.describedAs("Failures while parsing error message should be handled")
98-
.isInstanceOf(OpenAiClientException.class)
99-
.hasMessageContaining("400")
100-
.extracting(e -> e.getSuppressed()[0])
101-
.isInstanceOf(JsonParseException.class);
102-
103-
softly
104-
.assertThatThrownBy(request::run)
105-
.describedAs("Non-JSON responses should be handled")
106-
.isInstanceOf(OpenAiClientException.class)
107-
.hasMessageContaining("Failed to parse");
108-
109-
softly
110-
.assertThatThrownBy(request::run)
111-
.describedAs("Empty responses should be handled")
112-
.isInstanceOf(OpenAiClientException.class)
113-
.hasMessageContaining("was empty");
114-
115-
softly.assertAll();
47+
48+
stubForErrorHandling();
49+
assertForErrorHandling(request);
11650
}
11751

11852
private static Callable<?>[] chatCompletionCalls() {

0 commit comments

Comments
 (0)