Skip to content

Commit 6d39e5c

Browse files
DonalEvansDonalEvans
committed
Address review comments
1 parent ab8606f commit 6d39e5c

File tree

33 files changed

+116
-163
lines changed

33 files changed

+116
-163
lines changed

‎x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/common/amazon/AwsSecretSettingsTests.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,11 @@ protected AwsSecretSettings createTestInstance() {
119119
@Override
120120
protected AwsSecretSettings mutateInstance(AwsSecretSettings instance) throws IOException {
121121
if (randomBoolean()) {
122-
return new AwsSecretSettings(new SecureString(randomAlphaOfLength(10).toCharArray()), instance.secretKey());
122+
var accessKey = randomValueOtherThan(instance.accessKey().toString(), () -> randomAlphaOfLength(10));
123+
return new AwsSecretSettings(new SecureString(accessKey.toCharArray()), instance.secretKey());
123124
} else {
124-
return new AwsSecretSettings(instance.accessKey(), new SecureString(randomAlphaOfLength(10).toCharArray()));
125+
var secretKey = randomValueOtherThan(instance.secretKey().toString(), () -> randomAlphaOfLength(10));
126+
return new AwsSecretSettings(instance.accessKey(), new SecureString(secretKey.toCharArray()));
125127
}
126128
}
127129

‎x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/amazonbedrock/completion/AmazonBedrockChatCompletionTaskSettingsTests.java‎

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.elasticsearch.common.ValidationException;
1313
import org.elasticsearch.common.io.stream.Writeable;
1414
import org.elasticsearch.core.Nullable;
15+
import org.elasticsearch.test.ESTestCase;
1516
import org.elasticsearch.xcontent.XContentBuilder;
1617
import org.elasticsearch.xcontent.XContentFactory;
1718
import org.elasticsearch.xcontent.XContentType;
@@ -285,29 +286,22 @@ protected AmazonBedrockChatCompletionTaskSettings mutateInstance(AmazonBedrockCh
285286
var topK = instance.topK();
286287
var maxNewTokens = instance.maxNewTokens();
287288
switch (randomInt(3)) {
288-
case 0 -> temperature = randomValueOtherThan(temperature, AmazonBedrockChatCompletionTaskSettingsTests::randomDoubleOrNull);
289-
case 1 -> topP = randomValueOtherThan(topP, AmazonBedrockChatCompletionTaskSettingsTests::randomDoubleOrNull);
290-
case 2 -> topK = randomValueOtherThan(topK, AmazonBedrockChatCompletionTaskSettingsTests::randomDoubleOrNull);
291-
case 3 -> maxNewTokens = randomValueOtherThan(maxNewTokens, AmazonBedrockChatCompletionTaskSettingsTests::randomIntegerOrNull);
289+
case 0 -> temperature = randomValueOtherThan(temperature, ESTestCase::randomOptionalDouble);
290+
case 1 -> topP = randomValueOtherThan(topP, ESTestCase::randomOptionalDouble);
291+
case 2 -> topK = randomValueOtherThan(topK, ESTestCase::randomOptionalDouble);
292+
case 3 -> maxNewTokens = randomValueOtherThan(maxNewTokens, ESTestCase::randomNonNegativeIntOrNull);
292293
default -> throw new AssertionError("Illegal randomisation branch");
293294
}
294295
return new AmazonBedrockChatCompletionTaskSettings(temperature, topP, topK, maxNewTokens);
295296
}
296297

297298
private static AmazonBedrockChatCompletionTaskSettings createRandom() {
298299
return new AmazonBedrockChatCompletionTaskSettings(
299-
randomDoubleOrNull(),
300-
randomDoubleOrNull(),
301-
randomDoubleOrNull(),
302-
randomIntegerOrNull()
300+
randomOptionalDouble(),
301+
randomOptionalDouble(),
302+
randomOptionalDouble(),
303+
randomNonNegativeIntOrNull()
303304
);
304305
}
305306

306-
private static Double randomDoubleOrNull() {
307-
return randomFrom(new Double[] { null, randomDouble() });
308-
}
309-
310-
private static Integer randomIntegerOrNull() {
311-
return randomFrom(new Integer[] { null, randomNonNegativeInt() });
312-
}
313307
}

‎x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/amazonbedrock/embeddings/AmazonBedrockEmbeddingsServiceSettingsTests.java‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -400,12 +400,9 @@ protected AmazonBedrockEmbeddingsServiceSettings mutateInstance(AmazonBedrockEmb
400400
case 0 -> region = randomValueOtherThan(region, () -> randomAlphaOfLength(10));
401401
case 1 -> modelId = randomValueOtherThan(modelId, () -> randomAlphaOfLength(10));
402402
case 2 -> provider = randomValueOtherThan(provider, () -> randomFrom(AmazonBedrockProvider.values()));
403-
case 3 -> dimensions = randomValueOtherThan(dimensions, AmazonBedrockEmbeddingsServiceSettingsTests::randomIntegerOrNull);
403+
case 3 -> dimensions = randomValueOtherThan(dimensions, ESTestCase::randomNonNegativeIntOrNull);
404404
case 4 -> dimensionsSetByUser = randomValueOtherThan(dimensionsSetByUser, ESTestCase::randomBoolean);
405-
case 5 -> maxInputTokens = randomValueOtherThan(
406-
maxInputTokens,
407-
AmazonBedrockEmbeddingsServiceSettingsTests::randomIntegerOrNull
408-
);
405+
case 5 -> maxInputTokens = randomValueOtherThan(maxInputTokens, ESTestCase::randomNonNegativeIntOrNull);
409406
case 6 -> similarity = randomValueOtherThan(similarity, AmazonBedrockEmbeddingsServiceSettingsTests::randomSimilarityOrNull);
410407
case 7 -> rateLimitSettings = randomValueOtherThan(rateLimitSettings, RateLimitSettingsTests::createRandom);
411408
default -> throw new AssertionError("Illegal randomisation branch");
@@ -427,18 +424,14 @@ private static AmazonBedrockEmbeddingsServiceSettings createRandom() {
427424
randomAlphaOfLength(10),
428425
randomAlphaOfLength(10),
429426
randomFrom(AmazonBedrockProvider.values()),
430-
randomIntegerOrNull(),
427+
randomNonNegativeIntOrNull(),
431428
randomBoolean(),
432-
randomIntegerOrNull(),
429+
randomNonNegativeIntOrNull(),
433430
randomSimilarityOrNull(),
434431
RateLimitSettingsTests.createRandom()
435432
);
436433
}
437434

438-
private static Integer randomIntegerOrNull() {
439-
return randomFrom(new Integer[] { null, randomNonNegativeInt() });
440-
}
441-
442435
private static SimilarityMeasure randomSimilarityOrNull() {
443436
return randomFrom(new SimilarityMeasure[] { null, randomSimilarityMeasure() });
444437
}

‎x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/azureaistudio/completion/AzureAiStudioChatCompletionTaskSettingsTests.java‎

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.elasticsearch.common.ValidationException;
1313
import org.elasticsearch.common.io.stream.Writeable;
1414
import org.elasticsearch.core.Nullable;
15+
import org.elasticsearch.test.ESTestCase;
1516
import org.elasticsearch.xcontent.XContentBuilder;
1617
import org.elasticsearch.xcontent.XContentFactory;
1718
import org.elasticsearch.xcontent.XContentType;
@@ -235,10 +236,10 @@ protected AzureAiStudioChatCompletionTaskSettings mutateInstance(AzureAiStudioCh
235236
var doSample = instance.doSample();
236237
var maxNewTokens = instance.maxNewTokens();
237238
switch (randomInt(3)) {
238-
case 0 -> temperature = randomValueOtherThan(temperature, AzureAiStudioChatCompletionTaskSettingsTests::randomDoubleOrNull);
239-
case 1 -> topP = randomValueOtherThan(topP, AzureAiStudioChatCompletionTaskSettingsTests::randomDoubleOrNull);
240-
case 2 -> doSample = randomValueOtherThan(doSample, AzureAiStudioChatCompletionTaskSettingsTests::randomBooleanOrNull);
241-
case 3 -> maxNewTokens = randomValueOtherThan(maxNewTokens, AzureAiStudioChatCompletionTaskSettingsTests::randomIntegerOrNull);
239+
case 0 -> temperature = randomValueOtherThan(temperature, ESTestCase::randomOptionalDouble);
240+
case 1 -> topP = randomValueOtherThan(topP, ESTestCase::randomOptionalDouble);
241+
case 2 -> doSample = doSample == null ? randomBoolean() : doSample == false;
242+
case 3 -> maxNewTokens = randomValueOtherThan(maxNewTokens, ESTestCase::randomNonNegativeIntOrNull);
242243
default -> throw new AssertionError("Illegal randomisation branch");
243244
}
244245
return new AzureAiStudioChatCompletionTaskSettings(temperature, topP, doSample, maxNewTokens);
@@ -254,22 +255,10 @@ protected AzureAiStudioChatCompletionTaskSettings mutateInstanceForVersion(
254255

255256
private static AzureAiStudioChatCompletionTaskSettings createRandom() {
256257
return new AzureAiStudioChatCompletionTaskSettings(
257-
randomDoubleOrNull(),
258-
randomDoubleOrNull(),
259-
randomBooleanOrNull(),
260-
randomIntegerOrNull()
258+
randomOptionalDouble(),
259+
randomOptionalDouble(),
260+
randomOptionalBoolean(),
261+
randomNonNegativeIntOrNull()
261262
);
262263
}
263-
264-
private static Double randomDoubleOrNull() {
265-
return randomFrom(new Double[] { null, randomDouble() });
266-
}
267-
268-
private static Boolean randomBooleanOrNull() {
269-
return randomFrom(new Boolean[] { null, randomBoolean() });
270-
}
271-
272-
private static Integer randomIntegerOrNull() {
273-
return randomFrom(new Integer[] { null, randomNonNegativeInt() });
274-
}
275264
}

‎x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/azureaistudio/embeddings/AzureAiStudioEmbeddingsServiceSettingsTests.java‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -453,15 +453,12 @@ protected AzureAiStudioEmbeddingsServiceSettings mutateInstance(AzureAiStudioEmb
453453
case 0 -> target = randomValueOtherThan(target, () -> randomAlphaOfLength(10));
454454
case 1 -> provider = randomValueOtherThan(provider, () -> randomFrom(AzureAiStudioProvider.values()));
455455
case 2 -> endpointType = randomValueOtherThan(endpointType, () -> randomFrom(AzureAiStudioEndpointType.values()));
456-
case 3 -> dimensions = randomValueOtherThan(dimensions, AzureAiStudioEmbeddingsServiceSettingsTests::randomIntegerOrNull);
456+
case 3 -> dimensions = randomValueOtherThan(dimensions, ESTestCase::randomNonNegativeIntOrNull);
457457
case 4 -> dimensionsSetByUser = randomValueOtherThan(dimensionsSetByUser, ESTestCase::randomBoolean);
458-
case 5 -> maxInputTokens = randomValueOtherThan(
459-
maxInputTokens,
460-
AzureAiStudioEmbeddingsServiceSettingsTests::randomIntegerOrNull
461-
);
458+
case 5 -> maxInputTokens = randomValueOtherThan(maxInputTokens, ESTestCase::randomNonNegativeIntOrNull);
462459
case 6 -> similarity = randomValueOtherThan(
463460
similarity,
464-
() -> randomFrom(new SimilarityMeasure[] { null, randomSimilarityMeasure() })
461+
AzureAiStudioEmbeddingsServiceSettingsTests::randomSimilarityMeasureOrNull
465462
);
466463
case 7 -> rateLimitSettings = randomValueOtherThan(rateLimitSettings, RateLimitSettingsTests::createRandom);
467464
default -> throw new AssertionError("Illegal randomisation branch");
@@ -492,15 +489,15 @@ private static AzureAiStudioEmbeddingsServiceSettings createRandom() {
492489
randomAlphaOfLength(10),
493490
randomFrom(AzureAiStudioProvider.values()),
494491
randomFrom(AzureAiStudioEndpointType.values()),
495-
randomIntegerOrNull(),
492+
randomNonNegativeIntOrNull(),
496493
randomBoolean(),
497-
randomIntegerOrNull(),
498-
randomFrom(new SimilarityMeasure[] { null, randomSimilarityMeasure() }),
494+
randomNonNegativeIntOrNull(),
495+
randomSimilarityMeasureOrNull(),
499496
RateLimitSettingsTests.createRandom()
500497
);
501498
}
502499

503-
private static Integer randomIntegerOrNull() {
504-
return randomFrom(new Integer[] { null, randomNonNegativeInt() });
500+
private static SimilarityMeasure randomSimilarityMeasureOrNull() {
501+
return randomFrom(new SimilarityMeasure[] { null, randomSimilarityMeasure() });
505502
}
506503
}

‎x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/azureaistudio/embeddings/AzureAiStudioEmbeddingsTaskSettingsTests.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ protected AzureAiStudioEmbeddingsTaskSettings createTestInstance() {
136136

137137
@Override
138138
protected AzureAiStudioEmbeddingsTaskSettings mutateInstance(AzureAiStudioEmbeddingsTaskSettings instance) throws IOException {
139-
String newUser = randomValueOtherThan(instance.user(), () -> randomFrom(new String[] { null, randomAlphaOfLength(15) }));
139+
String newUser = randomValueOtherThan(instance.user(), () -> randomAlphaOfLengthOrNull(15));
140140
return new AzureAiStudioEmbeddingsTaskSettings(newUser);
141141
}
142142

@@ -149,6 +149,6 @@ protected AzureAiStudioEmbeddingsTaskSettings mutateInstanceForVersion(
149149
}
150150

151151
private static AzureAiStudioEmbeddingsTaskSettings createRandom() {
152-
return new AzureAiStudioEmbeddingsTaskSettings(randomFrom(new String[] { null, randomAlphaOfLength(15) }));
152+
return new AzureAiStudioEmbeddingsTaskSettings(randomAlphaOfLengthOrNull(15));
153153
}
154154
}

‎x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/azureaistudio/rerank/AzureAiStudioRerankTaskSettingsTests.java‎

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -197,13 +197,10 @@ protected AzureAiStudioRerankTaskSettings createTestInstance() {
197197
@Override
198198
protected AzureAiStudioRerankTaskSettings mutateInstance(AzureAiStudioRerankTaskSettings instance) throws IOException {
199199
if (randomBoolean()) {
200-
Boolean returnDocuments = randomValueOtherThan(
201-
instance.returnDocuments(),
202-
AzureAiStudioRerankTaskSettingsTests::randomBooleanOrNull
203-
);
204-
return new AzureAiStudioRerankTaskSettings(returnDocuments, instance.topN());
200+
Boolean newReturnDocuments = instance.returnDocuments() == null ? randomBoolean() : instance.returnDocuments() == false;
201+
return new AzureAiStudioRerankTaskSettings(newReturnDocuments, instance.topN());
205202
} else {
206-
Integer topN = randomValueOtherThan(instance.topN(), AzureAiStudioRerankTaskSettingsTests::randomIntegerOrNull);
203+
Integer topN = randomValueOtherThan(instance.topN(), ESTestCase::randomNonNegativeIntOrNull);
207204
return new AzureAiStudioRerankTaskSettings(instance.returnDocuments(), topN);
208205
}
209206
}
@@ -214,22 +211,7 @@ protected AzureAiStudioRerankTaskSettings mutateInstanceForVersion(AzureAiStudio
214211
}
215212

216213
private static AzureAiStudioRerankTaskSettings createRandom() {
217-
return new AzureAiStudioRerankTaskSettings(randomBooleanOrNull(), randomIntegerOrNull());
218-
}
219-
220-
private static AzureAiStudioRerankTaskSettings createRandom(AzureAiStudioRerankTaskSettings settings) {
221-
return new AzureAiStudioRerankTaskSettings(
222-
randomValueOtherThan(settings.returnDocuments(), AzureAiStudioRerankTaskSettingsTests::randomBooleanOrNull),
223-
randomValueOtherThan(settings.topN(), AzureAiStudioRerankTaskSettingsTests::randomIntegerOrNull)
224-
);
225-
}
226-
227-
private static Boolean randomBooleanOrNull() {
228-
return randomFrom(new Boolean[] { null, randomBoolean() });
229-
}
230-
231-
private static Integer randomIntegerOrNull() {
232-
return randomFrom(new Integer[] { null, randomNonNegativeInt() });
214+
return new AzureAiStudioRerankTaskSettings(randomOptionalBoolean(), randomNonNegativeIntOrNull());
233215
}
234216

235217
private void assertThrowsValidationExceptionIfStringValueProvidedFor(String field) {

‎x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/azureopenai/completion/AzureOpenAiCompletionServiceSettingsTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ private static AzureOpenAiCompletionServiceSettings createRandom() {
3030
var deploymentId = randomAlphaOfLength(8);
3131
var apiVersion = randomAlphaOfLength(8);
3232

33-
return new AzureOpenAiCompletionServiceSettings(resourceName, deploymentId, apiVersion, null);
33+
return new AzureOpenAiCompletionServiceSettings(resourceName, deploymentId, apiVersion, RateLimitSettingsTests.createRandom());
3434
}
3535

3636
public void testFromMap_Request_CreatesSettingsCorrectly() {

‎x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/azureopenai/completion/AzureOpenAiCompletionTaskSettingsTests.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ public static AzureOpenAiCompletionTaskSettings createRandomWithUser() {
2828
}
2929

3030
public static AzureOpenAiCompletionTaskSettings createRandom() {
31-
var user = randomBoolean() ? randomAlphaOfLength(15) : null;
32-
return new AzureOpenAiCompletionTaskSettings(user);
31+
return new AzureOpenAiCompletionTaskSettings(randomAlphaOfLengthOrNull(15));
3332
}
3433

3534
public void testIsEmpty() {
@@ -110,7 +109,7 @@ protected AzureOpenAiCompletionTaskSettings createTestInstance() {
110109

111110
@Override
112111
protected AzureOpenAiCompletionTaskSettings mutateInstance(AzureOpenAiCompletionTaskSettings instance) throws IOException {
113-
String user = randomValueOtherThan(instance.user(), () -> randomBoolean() ? randomAlphaOfLength(15) : null);
112+
String user = randomValueOtherThan(instance.user(), () -> randomAlphaOfLengthOrNull(15));
114113
return new AzureOpenAiCompletionTaskSettings(user);
115114
}
116115
}

‎x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/azureopenai/embeddings/AzureOpenAiEmbeddingsServiceSettingsTests.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.elasticsearch.core.Nullable;
1414
import org.elasticsearch.inference.SimilarityMeasure;
1515
import org.elasticsearch.test.AbstractWireSerializingTestCase;
16+
import org.elasticsearch.test.ESTestCase;
1617
import org.elasticsearch.xcontent.XContentBuilder;
1718
import org.elasticsearch.xcontent.XContentFactory;
1819
import org.elasticsearch.xcontent.XContentType;
@@ -39,7 +40,7 @@ private static AzureOpenAiEmbeddingsServiceSettings createRandom() {
3940
var resourceName = randomAlphaOfLength(8);
4041
var deploymentId = randomAlphaOfLength(8);
4142
var apiVersion = randomAlphaOfLength(8);
42-
Integer dims = randomBoolean() ? 1536 : null;
43+
Integer dims = randomNonNegativeIntOrNull();
4344
Integer maxInputTokens = randomBoolean() ? null : randomIntBetween(128, 256);
4445
return new AzureOpenAiEmbeddingsServiceSettings(
4546
resourceName,
@@ -522,7 +523,7 @@ protected AzureOpenAiEmbeddingsServiceSettings mutateInstance(AzureOpenAiEmbeddi
522523
case 0 -> resourceName = randomValueOtherThan(resourceName, () -> randomAlphaOfLength(8));
523524
case 1 -> deploymentId = randomValueOtherThan(deploymentId, () -> randomAlphaOfLength(8));
524525
case 2 -> apiVersion = randomValueOtherThan(apiVersion, () -> randomAlphaOfLength(8));
525-
case 3 -> dimensions = randomValueOtherThan(dimensions, () -> randomFrom(randomInt(), null));
526+
case 3 -> dimensions = randomValueOtherThan(dimensions, ESTestCase::randomNonNegativeIntOrNull);
526527
case 4 -> dimensionsSetByUser = dimensionsSetByUser == false;
527528
case 5 -> maxInputTokens = randomValueOtherThan(maxInputTokens, () -> randomFrom(randomIntBetween(128, 256), null));
528529
case 6 -> similarity = randomValueOtherThan(similarity, () -> randomFrom(randomSimilarityMeasure(), null));

0 commit comments

Comments
 (0)