Skip to content

Commit 2e1cac3

Browse files
dagnirdagnir
authored
Testing fixes (#2185)
- Fix ParseArn function name in test - Add ability to skip specific client endpoint tests. Note that this does not affect the auto generated tests that test the generated provider directly. That test will contine to test all of the test cases in endpoint-tests.json - Update S3 endpoint-tests.json - Skip some S3 client endpoint tests because they don't contain the necessary information (operationInputs) in order to generate the right request to use.
1 parent 645f8a6 commit 2e1cac3

File tree

14 files changed

+5788
-5189
lines changed

14 files changed

+5788
-5189
lines changed

‎codegen/src/main/java/software/amazon/awssdk/codegen/model/config/customization/CustomizationConfig.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,11 @@ public class CustomizationConfig {
206206
*/
207207
private boolean skipEndpointTestGeneration;
208208

209+
/**
210+
* A mapping from the skipped test's description to the reason why it's being skipped.
211+
*/
212+
private Map<String, String> skipEndpointTests;
213+
209214
private boolean useGlobalEndpoint;
210215

211216
private CustomizationConfig() {
@@ -536,4 +541,13 @@ public boolean useGlobalEndpoint() {
536541
public void setUseGlobalEndpoint(boolean useGlobalEndpoint) {
537542
this.useGlobalEndpoint = useGlobalEndpoint;
538543
}
544+
545+
public Map<String, String> getSkipEndpointTests() {
546+
return skipEndpointTests;
547+
}
548+
549+
public void setSkipEndpointTests(Map<String, String> skipEndpointTests) {
550+
this.skipEndpointTests = skipEndpointTests;
551+
}
552+
539553
}

‎codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/EndpointRulesClientTestSpec.java

Lines changed: 87 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.squareup.javapoet.AnnotationSpec;
2020
import com.squareup.javapoet.ClassName;
2121
import com.squareup.javapoet.CodeBlock;
22+
import com.squareup.javapoet.FieldSpec;
2223
import com.squareup.javapoet.MethodSpec;
2324
import com.squareup.javapoet.ParameterizedTypeName;
2425
import com.squareup.javapoet.TypeSpec;
@@ -32,6 +33,8 @@
3233
import java.util.Optional;
3334
import java.util.stream.Stream;
3435
import javax.lang.model.element.Modifier;
36+
import org.junit.jupiter.api.AfterAll;
37+
import org.junit.jupiter.api.BeforeEach;
3538
import org.junit.jupiter.params.ParameterizedTest;
3639
import org.junit.jupiter.params.provider.MethodSource;
3740
import org.mockito.Mockito;
@@ -48,13 +51,15 @@
4851
import software.amazon.awssdk.codegen.poet.ClassSpec;
4952
import software.amazon.awssdk.codegen.poet.PoetExtension;
5053
import software.amazon.awssdk.codegen.poet.PoetUtils;
54+
import software.amazon.awssdk.core.SdkSystemSetting;
5155
import software.amazon.awssdk.core.async.AsyncRequestBody;
5256
import software.amazon.awssdk.core.rules.testing.AsyncTestCase;
5357
import software.amazon.awssdk.core.rules.testing.BaseRuleSetClientTest;
5458
import software.amazon.awssdk.core.rules.testing.SyncTestCase;
5559
import software.amazon.awssdk.core.rules.testing.util.EmptyPublisher;
5660
import software.amazon.awssdk.core.sync.RequestBody;
5761
import software.amazon.awssdk.regions.Region;
62+
import software.amazon.awssdk.utils.Validate;
5863

5964
public class EndpointRulesClientTestSpec implements ClassSpec {
6065
private final IntermediateModel model;
@@ -73,6 +78,13 @@ public TypeSpec poetSpec() {
7378
.addModifiers(Modifier.PUBLIC)
7479
.superclass(BaseRuleSetClientTest.class);
7580

81+
if (isS3()) {
82+
b.addField(s3RegionEndpointSystemPropertySaveValueField());
83+
}
84+
85+
b.addMethod(methodSetupMethod());
86+
b.addMethod(teardownMethod());
87+
7688
if (hasSyncClient()) {
7789
b.addMethod(syncTest());
7890
}
@@ -174,18 +186,18 @@ private MethodSpec syncTestsSourceMethod() {
174186
test.getOperationInputs().forEach(opInput -> {
175187
OperationModel opModel = model.getOperation(opInput.getOperationName());
176188

177-
b.addCode("new $T($S, $L, $L)",
189+
b.addCode("new $T($S, $L, $L$L)",
178190
SyncTestCase.class,
179191
test.getDocumentation(),
180192
syncOperationCallLambda(opModel, test.getParams(), opInput.getOperationParams()),
181-
TestGeneratorUtils.createExpect(test.getExpect()));
193+
TestGeneratorUtils.createExpect(test.getExpect()), getSkipReasonBlock(test.getDocumentation()));
182194
});
183195
} else {
184-
b.addCode("new $T($S, $L, $L)",
196+
b.addCode("new $T($S, $L, $L$L)",
185197
SyncTestCase.class,
186198
test.getDocumentation(),
187199
syncOperationCallLambda(defaultOpModel, test.getParams(), Collections.emptyMap()),
188-
TestGeneratorUtils.createExpect(test.getExpect()));
200+
TestGeneratorUtils.createExpect(test.getExpect()), getSkipReasonBlock(test.getDocumentation()));
189201
}
190202

191203
if (testIter.hasNext()) {
@@ -302,18 +314,18 @@ private MethodSpec asyncTestsSourceMethod() {
302314
test.getOperationInputs().forEach(opInput -> {
303315
OperationModel opModel = model.getOperation(opInput.getOperationName());
304316

305-
b.addCode("new $T($S, $L, $L)",
317+
b.addCode("new $T($S, $L, $L$L)",
306318
AsyncTestCase.class,
307319
test.getDocumentation(),
308320
asyncOperationCallLambda(opModel, test.getParams(), opInput.getOperationParams()),
309-
TestGeneratorUtils.createExpect(test.getExpect()));
321+
TestGeneratorUtils.createExpect(test.getExpect()), getSkipReasonBlock(test.getDocumentation()));
310322
});
311323
} else {
312-
b.addCode("new $T($S, $L, $L)",
324+
b.addCode("new $T($S, $L, $L$L)",
313325
AsyncTestCase.class,
314326
test.getDocumentation(),
315327
asyncOperationCallLambda(defaultOpModel, test.getParams(), Collections.emptyMap()),
316-
TestGeneratorUtils.createExpect(test.getExpect()));
328+
TestGeneratorUtils.createExpect(test.getExpect()), getSkipReasonBlock(test.getDocumentation()));
317329
}
318330

319331
if (testIter.hasNext()) {
@@ -485,6 +497,10 @@ private CodeBlock setClientParams(String builderName, Map<String, TreeNode> para
485497
case SDK_ENDPOINT:
486498
b.addStatement("$N.endpointOverride($T.create($L))", builderName, URI.class, valueLiteral);
487499
break;
500+
case AWS_S3_USE_GLOBAL_ENDPOINT:
501+
b.addStatement("$T.setProperty($L, $L ? \"global\" : \"regional\")", System.class,
502+
s3RegionalEndpointSystemPropertyCode(), valueLiteral);
503+
break;
488504
default:
489505
break;
490506
}
@@ -546,4 +562,67 @@ private boolean isS3() {
546562
private ClassName configClass() {
547563
return poetExtension.getClientClass(model.getCustomizationConfig().getServiceConfig().getClassName());
548564
}
565+
566+
private Map<String, String> getSkippedTests() {
567+
Map<String, String> skippedTests = model.getCustomizationConfig().getSkipEndpointTests();
568+
if (skippedTests == null) {
569+
return Collections.emptyMap();
570+
}
571+
return skippedTests;
572+
}
573+
574+
private CodeBlock getSkipReasonBlock(String testName) {
575+
if (getSkippedTests().containsKey(testName)) {
576+
Validate.notNull(getSkippedTests().get(testName), "Test %s must have a reason for skipping", testName);
577+
return CodeBlock.builder().add(", $S", getSkippedTests().get(testName)).build();
578+
}
579+
return CodeBlock.builder().build();
580+
}
581+
582+
private MethodSpec methodSetupMethod() {
583+
MethodSpec.Builder b = MethodSpec.methodBuilder("methodSetup")
584+
.addModifiers(Modifier.PUBLIC)
585+
.addAnnotation(BeforeEach.class)
586+
.returns(void.class);
587+
588+
b.addStatement("super.methodSetup()");
589+
590+
// S3 rules assume UseGlobalEndpoint == false by default
591+
if (isS3()) {
592+
b.addStatement("$T.setProperty($L, $S)", System.class, s3RegionalEndpointSystemPropertyCode(), "regional");
593+
}
594+
595+
return b.build();
596+
}
597+
598+
private CodeBlock s3RegionalEndpointSystemPropertyCode() {
599+
return CodeBlock.builder()
600+
.add("$T.AWS_S3_US_EAST_1_REGIONAL_ENDPOINT.property()", SdkSystemSetting.class)
601+
.build();
602+
}
603+
604+
private FieldSpec s3RegionEndpointSystemPropertySaveValueField() {
605+
return FieldSpec.builder(String.class, "regionalEndpointPropertySaveValue")
606+
.addModifiers(Modifier.PRIVATE, Modifier.STATIC)
607+
.initializer("$T.getProperty($L)", System.class, s3RegionalEndpointSystemPropertyCode())
608+
.build();
609+
}
610+
611+
private MethodSpec teardownMethod() {
612+
MethodSpec.Builder b = MethodSpec.methodBuilder("teardown")
613+
.addModifiers(Modifier.PUBLIC, Modifier.STATIC)
614+
.addAnnotation(AfterAll.class)
615+
.returns(void.class);
616+
617+
if (isS3()) {
618+
b.beginControlFlow("if (regionalEndpointPropertySaveValue != null)")
619+
.addStatement("$T.setProperty($L, regionalEndpointPropertySaveValue)", System.class,
620+
s3RegionalEndpointSystemPropertyCode())
621+
.endControlFlow()
622+
.beginControlFlow("else")
623+
.addStatement("$T.clearProperty($L)", System.class, s3RegionalEndpointSystemPropertyCode())
624+
.endControlFlow();
625+
}
626+
return b.build();
627+
}
549628
}

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/builder/test-query-client-builder-class.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,15 @@ private QueryEndpointProvider defaultEndpointProvider() {
7272
return QueryEndpointProvider.defaultProvider();
7373
}
7474

75+
@Override
7576
public B booleanContextParam(Boolean booleanContextParam) {
7677
clientContextParams.put(QueryClientContextParams.BOOLEAN_CONTEXT_PARAM, booleanContextParam);
7778
return thisBuilder();
7879
}
7980

81+
@Override
8082
public B stringContextParam(String stringContextParam) {
8183
clientContextParams.put(QueryClientContextParams.STRING_CONTEXT_PARAM, stringContextParam);
8284
return thisBuilder();
8385
}
84-
}
86+
}

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/query/customization.config

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
{
22
"authPolicyActions" : {
33
"skip" : true
4-
}
4+
},
5+
"skipEndpointTests": {
6+
"test case 4": "Does not work"
7+
}
58
}

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/query/endpoint-tests.json

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,25 @@
4242
"url": "https://myservice.aws"
4343
}
4444
}
45+
},
46+
{
47+
"documentation": "test case 4",
48+
"params": {
49+
"region": "us-east-6"
50+
},
51+
"operationInputs": [
52+
{
53+
"operationName": "OperationWithContextParam",
54+
"operationParams": {
55+
"StringMember": "this is a test"
56+
}
57+
}
58+
],
59+
"expect": {
60+
"endpoint": {
61+
"url": "https://myservice.aws"
62+
}
63+
}
4564
}
4665
],
4766
"version": "1.0"

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules/endpoint-rules-test-class.java

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import java.net.URI;
44
import java.util.Arrays;
55
import java.util.List;
6+
import org.junit.jupiter.api.AfterAll;
7+
import org.junit.jupiter.api.BeforeEach;
68
import org.junit.jupiter.params.ParameterizedTest;
79
import org.junit.jupiter.params.provider.MethodSource;
810
import software.amazon.awssdk.annotations.Generated;
@@ -21,6 +23,15 @@
2123

2224
@Generated("software.amazon.awssdk:codegen")
2325
public class QueryClientEndpointTests extends BaseRuleSetClientTest {
26+
@BeforeEach
27+
public void methodSetup() {
28+
super.methodSetup();
29+
}
30+
31+
@AfterAll
32+
public static void teardown() {
33+
}
34+
2435
@MethodSource("syncTestCases")
2536
@ParameterizedTest
2637
public void syncClient_usesCorrectEndpoint(SyncTestCase tc) {
@@ -61,7 +72,17 @@ private static List<SyncTestCase> syncTestCases() {
6172
OperationWithContextParamRequest request = OperationWithContextParamRequest.builder()
6273
.stringMember("this is a test").build();
6374
builder.build().operationWithContextParam(request);
64-
}, Expect.builder().endpoint(Endpoint.builder().url(URI.create("https://myservice.aws")).build()).build()));
75+
}, Expect.builder().endpoint(Endpoint.builder().url(URI.create("https://myservice.aws")).build()).build()),
76+
new SyncTestCase("test case 4", () -> {
77+
QueryClientBuilder builder = QueryClient.builder();
78+
builder.credentialsProvider(BaseRuleSetClientTest.CREDENTIALS_PROVIDER);
79+
builder.httpClient(getSyncHttpClient());
80+
builder.region(Region.of("us-east-6"));
81+
OperationWithContextParamRequest request = OperationWithContextParamRequest.builder()
82+
.stringMember("this is a test").build();
83+
builder.build().operationWithContextParam(request);
84+
}, Expect.builder().endpoint(Endpoint.builder().url(URI.create("https://myservice.aws")).build()).build(),
85+
"Does not work"));
6586
}
6687

6788
private static List<AsyncTestCase> asyncTestCases() {
@@ -92,6 +113,16 @@ private static List<AsyncTestCase> asyncTestCases() {
92113
OperationWithContextParamRequest request = OperationWithContextParamRequest.builder()
93114
.stringMember("this is a test").build();
94115
return builder.build().operationWithContextParam(request);
95-
}, Expect.builder().endpoint(Endpoint.builder().url(URI.create("https://myservice.aws")).build()).build()));
116+
}, Expect.builder().endpoint(Endpoint.builder().url(URI.create("https://myservice.aws")).build()).build()),
117+
new AsyncTestCase("test case 4", () -> {
118+
QueryAsyncClientBuilder builder = QueryAsyncClient.builder();
119+
builder.credentialsProvider(BaseRuleSetClientTest.CREDENTIALS_PROVIDER);
120+
builder.httpClient(getAsyncHttpClient());
121+
builder.region(Region.of("us-east-6"));
122+
OperationWithContextParamRequest request = OperationWithContextParamRequest.builder()
123+
.stringMember("this is a test").build();
124+
return builder.build().operationWithContextParam(request);
125+
}, Expect.builder().endpoint(Endpoint.builder().url(URI.create("https://myservice.aws")).build()).build(),
126+
"Does not work"));
96127
}
97128
}

‎core/aws-core/src/main/java/software/amazon/awssdk/awscore/rules/AwsEndpointProviderUtils.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public static String endpointBuiltIn(ExecutionAttributes executionAttributes) {
6666
return invokeSafely(() -> {
6767
URI endpointOverride = executionAttributes.getAttribute(SdkExecutionAttribute.CLIENT_ENDPOINT);
6868
return new URI(endpointOverride.getScheme(), null, endpointOverride.getHost(), endpointOverride.getPort(),
69-
endpointOverride.getPath(), null, null).toString();
69+
endpointOverride.getPath(), null, endpointOverride.getFragment()).toString();
7070
});
7171
}
7272
return null;

‎core/sdk-core/src/test/resources/software/amazon/awssdk/core/rules/valid-rules/parse-arn.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
"documentation": "bucket is set and is an arn",
4949
"conditions": [
5050
{
51-
"fn": "parseArn",
51+
"fn": "aws.parseArn",
5252
"argv": [
5353
{
5454
"ref": "Bucket"

‎services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/endpoints/UseGlobalEndpointResolver.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public UseGlobalEndpointResolver(SdkClientConfiguration config) {
4444
String defaultS3UsEast1RegionalEndpointFromSmartDefaults =
4545
config.option(ServiceMetadataAdvancedOption.DEFAULT_S3_US_EAST_1_REGIONAL_ENDPOINT);
4646
this.useUsEast1RegionalEndpoint =
47-
new Lazy<>(() -> useUsEast1RegionalEndpoint(() ->config.option(SdkClientOption.PROFILE_FILE),
47+
new Lazy<>(() -> useUsEast1RegionalEndpoint(() -> config.option(SdkClientOption.PROFILE_FILE),
4848
() -> config.option(SdkClientOption.PROFILE_NAME),
4949
defaultS3UsEast1RegionalEndpointFromSmartDefaults));
5050
}

0 commit comments

Comments
 (0)