From 522110668d477a6eaf71102b03d03a18d456f6a9 Mon Sep 17 00:00:00 2001 From: JordonPhillips Date: Mon, 21 Jun 2021 17:45:18 +0200 Subject: [PATCH 1/5] Add ability to filter tests This adds the ability to filter certain test cases. Reasons for doing this might include wanting to skip tests for a feature not yet implemented or wanting to skip a test whose asserted http request is only one of several valid options. --- .../codegen/HttpProtocolTestGenerator.java | 70 ++++++++++++++++--- 1 file changed, 62 insertions(+), 8 deletions(-) diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/HttpProtocolTestGenerator.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/HttpProtocolTestGenerator.java index cd6751cdafb..556e51213b7 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/HttpProtocolTestGenerator.java +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/HttpProtocolTestGenerator.java @@ -94,6 +94,7 @@ final class HttpProtocolTestGenerator implements Runnable { private final Symbol serviceSymbol; private final Set additionalStubs = new TreeSet<>(); private final ProtocolGenerator protocolGenerator; + private final TestFilter testFilter; /** Vends a TypeScript IFF it's needed. */ private final TypeScriptDelegator delegator; @@ -107,7 +108,8 @@ final class HttpProtocolTestGenerator implements Runnable { ShapeId protocol, SymbolProvider symbolProvider, TypeScriptDelegator delegator, - ProtocolGenerator protocolGenerator + ProtocolGenerator protocolGenerator, + TestFilter testFilter ) { this.settings = settings; this.model = model; @@ -117,6 +119,19 @@ final class HttpProtocolTestGenerator implements Runnable { this.delegator = delegator; this.protocolGenerator = protocolGenerator; serviceSymbol = symbolProvider.toSymbol(service); + this.testFilter = testFilter; + } + + HttpProtocolTestGenerator( + TypeScriptSettings settings, + Model model, + ShapeId protocol, + SymbolProvider symbolProvider, + TypeScriptDelegator delegator, + ProtocolGenerator protocolGenerator + ) { + this(settings, model, protocol, symbolProvider, delegator, protocolGenerator, + (service, operation, testCase, typeScriptSettings) -> false); } @Override @@ -227,7 +242,7 @@ private void generateClientRequestTest(OperationShape operation, HttpRequestTest String testName = testCase.getId() + ":Request"; testCase.getDocumentation().ifPresent(writer::writeDocs); - writer.openBlock("it($S, async () => {", "});\n", testName, () -> { + openTestBlock(operation, testCase, testName, () -> { // Create a client with a custom request handler that intercepts requests. writer.openBlock("const client = new $T({", "});\n", serviceSymbol, () -> { writer.write("...clientParams,"); @@ -277,7 +292,7 @@ private void generateServerRequestTest(OperationShape operation, HttpRequestTest String testName = testCase.getId() + ":ServerRequest"; testCase.getDocumentation().ifPresent(writer::writeDocs); - writer.openBlock("it($S, async () => {", "});\n", testName, () -> { + openTestBlock(operation, testCase, testName, () -> { Symbol serviceSymbol = symbolProvider.toSymbol(service); Symbol handlerSymbol = serviceSymbol.expectProperty("handler", Symbol.class); @@ -485,7 +500,7 @@ public void generateServerResponseTest(OperationShape operation, HttpResponseTes Symbol operationSymbol = symbolProvider.toSymbol(operation); testCase.getDocumentation().ifPresent(writer::writeDocs); String testName = testCase.getId() + ":ServerResponse"; - writer.openBlock("it($S, async () => {", "});\n", testName, () -> { + openTestBlock(operation, testCase, testName, () -> { Symbol outputType = operationSymbol.expectProperty("outputType", Symbol.class); writer.openBlock("class TestService implements Partial<$T> {", "}", serviceSymbol, () -> { writer.openBlock("$L(input: any, request: HttpRequest): Promise<$T> {", "}", @@ -508,7 +523,7 @@ public void generateServerResponseTest(OperationShape operation, HttpResponseTes private void generateResponseTest(OperationShape operation, HttpResponseTestCase testCase) { testCase.getDocumentation().ifPresent(writer::writeDocs); String testName = testCase.getId() + ":Response"; - writer.openBlock("it($S, async () => {", "});\n", testName, () -> { + openTestBlock(operation, testCase, testName, () -> { writeResponseTestSetup(operation, testCase, true); // Invoke the handler and look for the expected response to then perform assertions. @@ -536,8 +551,7 @@ private void generateServerErrorResponseTest( testCase.getDocumentation().ifPresent(writer::writeDocs); String testName = testCase.getId() + ":ServerErrorResponse"; - writer.openBlock("it($S, async () => {", "});\n", testName, () -> { - + openTestBlock(operation, testCase, testName, () -> { // Generates a Partial implementation of the service type that only includes // the specific operation under test. Later we'll have to "cast" this with an "as", // but using the partial in the meantime will give us proper type checking on the @@ -624,7 +638,7 @@ private void generateErrorResponseTest( // we can test for any operation specific values properly. String testName = testCase.getId() + ":Error:" + operation.getId().getName(); testCase.getDocumentation().ifPresent(writer::writeDocs); - writer.openBlock("it($S, async () => {", "});\n", testName, () -> { + openTestBlock(operation, testCase, testName, () -> { writeResponseTestSetup(operation, testCase, false); // Invoke the handler and look for the expected exception to then perform assertions. @@ -788,6 +802,20 @@ private void writeParamAssertions( }); } + private void openTestBlock( + OperationShape operation, + HttpMessageTestCase testCase, + String testName, + Runnable f + ) { + // Skipped tests are still generated, just not run. + if (testFilter.skip(service, operation, testCase, settings)) { + writer.openBlock("it.skip($S, async() => {", "});", testName, f); + } else { + writer.openBlock("it($S, async () => {", "});", testName, f); + } + } + /** * Supports writing out TS specific input types in the generated code * through visiting the target shape at the same time as the node. If @@ -945,6 +973,32 @@ public Void stringNode(StringNode node) { } } + /** + * Functional interface for skipping tests. + */ + @FunctionalInterface + public interface TestFilter { + /** + * A function that determines whether or not to skip a test. + * + *

A test might be temporarily skipped if it's a known failure that + * will be addressed later, or if the test in question asserts a + * serialized message that can have multiple valid forms. + * + * @param service The service for which tests are being generated. + * @param operation The operation for which tests are being generated. + * @param testCase The test case in question. + * @param settings The settings being used to generate the test service. + * @return True if the test should be skipped, false otherwise. + */ + boolean skip( + ServiceShape service, + OperationShape operation, + HttpMessageTestCase testCase, + TypeScriptSettings settings + ); + } + /** * Supports writing out TS specific output types in the generated code * through visiting the target shape at the same time as the node. If From 401c16adfd92ca4ec32d9938a3fb397ccf6ef317 Mon Sep 17 00:00:00 2001 From: JordonPhillips Date: Mon, 21 Jun 2021 18:37:04 +0200 Subject: [PATCH 2/5] Make protocol test generation a part of protocols This updates the way protocol tests are generated. Instead of always generating http protocol tests, it now relies on protocols to initiate test generation. This is important for non-http protocols. It's also important for being able to customize the generation for http protocols. There is an unfortunate implication - a test file will always be generated now. This shouldn't *really* matter since that won't be part of the uploaded artifacts anyway. --- .../typescript/codegen/CodegenVisitor.java | 17 +++++++- .../codegen/HttpProtocolTestGenerator.java | 41 +++++++------------ .../integration/ProtocolGenerator.java | 7 ++++ 3 files changed, 37 insertions(+), 28 deletions(-) diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/CodegenVisitor.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/CodegenVisitor.java index 0d8cafa1cfd..710e609a551 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/CodegenVisitor.java +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/CodegenVisitor.java @@ -20,6 +20,7 @@ import java.util.Comparator; import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.ServiceLoader; @@ -214,8 +215,20 @@ void execute() { // Generate protocol tests IFF found in the model. if (protocolGenerator != null) { ShapeId protocol = protocolGenerator.getProtocol(); - new HttpProtocolTestGenerator( - settings, model, protocol, symbolProvider, writers, protocolGenerator).run(); + ProtocolGenerator.GenerationContext context = new ProtocolGenerator.GenerationContext(); + context.setProtocolName(protocolGenerator.getName()); + context.setIntegrations(integrations); + context.setModel(model); + context.setService(service); + context.setSettings(settings); + context.setSymbolProvider(symbolProvider); + String baseName = protocol.getName().toLowerCase(Locale.US) + .replace("-", "_") + .replace(".", "_"); + writers.useFileWriter(String.format("tests/functional/%s.spec.ts", baseName), writer -> { + context.setWriter(writer); + protocolGenerator.generateProtocolTests(context); + }); } // Write each pending writer. diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/HttpProtocolTestGenerator.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/HttpProtocolTestGenerator.java index 556e51213b7..8fac6547fde 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/HttpProtocolTestGenerator.java +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/HttpProtocolTestGenerator.java @@ -63,6 +63,7 @@ import software.amazon.smithy.protocoltests.traits.HttpResponseTestCase; import software.amazon.smithy.protocoltests.traits.HttpResponseTestsTrait; import software.amazon.smithy.typescript.codegen.integration.ProtocolGenerator; +import software.amazon.smithy.typescript.codegen.integration.ProtocolGenerator.GenerationContext; import software.amazon.smithy.utils.IoUtils; import software.amazon.smithy.utils.MapUtils; import software.amazon.smithy.utils.Pair; @@ -81,7 +82,7 @@ * TODO: try/catch and if/else are still cumbersome with TypeScriptWriter. */ @SmithyInternalApi -final class HttpProtocolTestGenerator implements Runnable { +public final class HttpProtocolTestGenerator implements Runnable { private static final Logger LOGGER = Logger.getLogger(HttpProtocolTestGenerator.class.getName()); private static final String TEST_CASE_FILE_TEMPLATE = "tests/functional/%s.spec.ts"; @@ -96,42 +97,30 @@ final class HttpProtocolTestGenerator implements Runnable { private final ProtocolGenerator protocolGenerator; private final TestFilter testFilter; - /** Vends a TypeScript IFF it's needed. */ - private final TypeScriptDelegator delegator; - - /** The TypeScript writer that's only allocated once if needed. */ private TypeScriptWriter writer; + private boolean writerInitialized = false; HttpProtocolTestGenerator( - TypeScriptSettings settings, - Model model, - ShapeId protocol, - SymbolProvider symbolProvider, - TypeScriptDelegator delegator, + GenerationContext context, ProtocolGenerator protocolGenerator, TestFilter testFilter ) { - this.settings = settings; - this.model = model; - this.protocol = protocol; + this.settings = context.getSettings(); + this.model = context.getModel(); + this.protocol = context.getSettings().getProtocol(); this.service = settings.getService(model); - this.symbolProvider = symbolProvider; - this.delegator = delegator; + this.symbolProvider = context.getSymbolProvider(); + this.writer = context.getWriter(); this.protocolGenerator = protocolGenerator; serviceSymbol = symbolProvider.toSymbol(service); this.testFilter = testFilter; } HttpProtocolTestGenerator( - TypeScriptSettings settings, - Model model, - ShapeId protocol, - SymbolProvider symbolProvider, - TypeScriptDelegator delegator, + GenerationContext context, ProtocolGenerator protocolGenerator ) { - this(settings, model, protocol, symbolProvider, delegator, protocolGenerator, - (service, operation, testCase, typeScriptSettings) -> false); + this(context, protocolGenerator, (service, operation, testCase, typeScriptSettings) -> false); } @Override @@ -215,18 +204,18 @@ private void generateServerOperationTests(OperationShape operation, OperationInd private void onlyIfProtocolMatches(T testCase, Runnable runnable) { if (testCase.getProtocol().equals(protocol)) { LOGGER.fine(() -> format("Generating protocol test case for %s.%s", service.getId(), testCase.getId())); - allocateWriterIfNeeded(); + initializeWriterIfNeeded(); runnable.run(); } } - private void allocateWriterIfNeeded() { - if (writer == null) { - delegator.useFileWriter(createTestCaseFilename(), writer -> this.writer = writer); + private void initializeWriterIfNeeded() { + if (!writerInitialized) { writer.addDependency(TypeScriptDependency.AWS_SDK_TYPES); writer.addDependency(TypeScriptDependency.AWS_SDK_PROTOCOL_HTTP); // Add the template to each generated test. writer.write(IoUtils.readUtf8Resource(getClass(), "protocol-test-stub.ts")); + writerInitialized = true; } } diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/ProtocolGenerator.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/ProtocolGenerator.java index 05f42fd697b..50ff36aa1e1 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/ProtocolGenerator.java +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/ProtocolGenerator.java @@ -183,6 +183,13 @@ default void generateSharedComponents(GenerationContext context) { */ void generateResponseDeserializers(GenerationContext context); + /** + * Generates protocol tests to assert the protocol works properly. + * + * @param context Generation context. + */ + void generateProtocolTests(GenerationContext context); + /** * Generates the name of a serializer function for shapes of a service. * From 0fdcca8223e803212ea79293dc7755f94d9a21ab Mon Sep 17 00:00:00 2001 From: JordonPhillips Date: Mon, 21 Jun 2021 19:06:31 +0200 Subject: [PATCH 3/5] Defer file writer creation for protocol tests --- .../typescript/codegen/CodegenVisitor.java | 7 +++---- .../codegen/HttpProtocolTestGenerator.java | 8 ++++---- .../typescript/codegen/TypeScriptDelegator.java | 10 ++++++++++ .../codegen/integration/ProtocolGenerator.java | 16 ++++++++++++++++ 4 files changed, 33 insertions(+), 8 deletions(-) diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/CodegenVisitor.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/CodegenVisitor.java index 710e609a551..ed1b1bbeaa2 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/CodegenVisitor.java +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/CodegenVisitor.java @@ -225,10 +225,9 @@ void execute() { String baseName = protocol.getName().toLowerCase(Locale.US) .replace("-", "_") .replace(".", "_"); - writers.useFileWriter(String.format("tests/functional/%s.spec.ts", baseName), writer -> { - context.setWriter(writer); - protocolGenerator.generateProtocolTests(context); - }); + String protocolTestFileName = String.format("tests/functional/%s.spec.ts", baseName); + context.setDeferredWriter(() -> writers.checkoutFileWriter(protocolTestFileName)); + protocolGenerator.generateProtocolTests(context); } // Write each pending writer. diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/HttpProtocolTestGenerator.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/HttpProtocolTestGenerator.java index 8fac6547fde..bc71e10588c 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/HttpProtocolTestGenerator.java +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/HttpProtocolTestGenerator.java @@ -96,9 +96,9 @@ public final class HttpProtocolTestGenerator implements Runnable { private final Set additionalStubs = new TreeSet<>(); private final ProtocolGenerator protocolGenerator; private final TestFilter testFilter; + private final GenerationContext context; private TypeScriptWriter writer; - private boolean writerInitialized = false; HttpProtocolTestGenerator( GenerationContext context, @@ -110,10 +110,10 @@ public final class HttpProtocolTestGenerator implements Runnable { this.protocol = context.getSettings().getProtocol(); this.service = settings.getService(model); this.symbolProvider = context.getSymbolProvider(); - this.writer = context.getWriter(); this.protocolGenerator = protocolGenerator; serviceSymbol = symbolProvider.toSymbol(service); this.testFilter = testFilter; + this.context = context; } HttpProtocolTestGenerator( @@ -210,12 +210,12 @@ private void onlyIfProtocolMatches(T testCase, R } private void initializeWriterIfNeeded() { - if (!writerInitialized) { + if (writer == null) { + writer = context.getWriter(); writer.addDependency(TypeScriptDependency.AWS_SDK_TYPES); writer.addDependency(TypeScriptDependency.AWS_SDK_PROTOCOL_HTTP); // Add the template to each generated test. writer.write(IoUtils.readUtf8Resource(getClass(), "protocol-test-stub.ts")); - writerInitialized = true; } } diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/TypeScriptDelegator.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/TypeScriptDelegator.java index 2c485461eb3..1877ef4aded 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/TypeScriptDelegator.java +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/TypeScriptDelegator.java @@ -131,6 +131,16 @@ void useFileWriter(String filename, Consumer writerConsumer) { writerConsumer.accept(checkoutWriter(filename)); } + /** + * Gets a previously created writer or creates a new one if needed + * and adds a new line if the writer already exists. + * + * @param filename Name of the file to create. + */ + TypeScriptWriter checkoutFileWriter(String filename) { + return checkoutWriter(filename); + } + private TypeScriptWriter checkoutWriter(String filename) { String formattedFilename = Paths.get(filename).normalize().toString(); boolean needsNewline = writers.containsKey(formattedFilename); diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/ProtocolGenerator.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/ProtocolGenerator.java index 50ff36aa1e1..21eb6d57b40 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/ProtocolGenerator.java +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/ProtocolGenerator.java @@ -17,6 +17,7 @@ import java.util.Collection; import java.util.List; +import java.util.function.Supplier; import java.util.stream.Collectors; import software.amazon.smithy.codegen.core.CodegenException; import software.amazon.smithy.codegen.core.Symbol; @@ -270,6 +271,7 @@ class GenerationContext { private ServiceShape service; private SymbolProvider symbolProvider; private TypeScriptWriter writer; + private Supplier writerSupplier; private List integrations; private String protocolName; @@ -306,11 +308,24 @@ public void setSymbolProvider(SymbolProvider symbolProvider) { } public TypeScriptWriter getWriter() { + if (writerSupplier != null && writer == null) { + writer = writerSupplier.get(); + } return writer; } public void setWriter(TypeScriptWriter writer) { this.writer = writer; + if (writer != null) { + this.writerSupplier = null; + } + } + + public void setDeferredWriter(Supplier writerSupplier) { + this.writerSupplier = writerSupplier; + if (writerSupplier != null) { + this.writer = null; + } } public List getIntegrations() { @@ -336,6 +351,7 @@ public GenerationContext copy() { copy.setService(service); copy.setSymbolProvider(symbolProvider); copy.setWriter(writer); + copy.setDeferredWriter(writerSupplier); copy.setIntegrations(integrations); copy.setProtocolName(protocolName); return copy; From 481633d6fd0e9427fab8da3a60f182f63a76c653 Mon Sep 17 00:00:00 2001 From: JordonPhillips Date: Mon, 21 Jun 2021 19:37:02 +0200 Subject: [PATCH 4/5] Make http protocol test generator contructors public --- .../smithy/typescript/codegen/HttpProtocolTestGenerator.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/HttpProtocolTestGenerator.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/HttpProtocolTestGenerator.java index bc71e10588c..28c62d47c00 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/HttpProtocolTestGenerator.java +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/HttpProtocolTestGenerator.java @@ -100,7 +100,7 @@ public final class HttpProtocolTestGenerator implements Runnable { private TypeScriptWriter writer; - HttpProtocolTestGenerator( + public HttpProtocolTestGenerator( GenerationContext context, ProtocolGenerator protocolGenerator, TestFilter testFilter @@ -116,7 +116,7 @@ public final class HttpProtocolTestGenerator implements Runnable { this.context = context; } - HttpProtocolTestGenerator( + public HttpProtocolTestGenerator( GenerationContext context, ProtocolGenerator protocolGenerator ) { From 68302ab3a73e71e47bb5a2d86a4b8daeeeb53929 Mon Sep 17 00:00:00 2001 From: JordonPhillips Date: Mon, 21 Jun 2021 20:40:01 +0200 Subject: [PATCH 5/5] Pull protocol from protocol generator --- .../smithy/typescript/codegen/HttpProtocolTestGenerator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/HttpProtocolTestGenerator.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/HttpProtocolTestGenerator.java index 28c62d47c00..61067c7936d 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/HttpProtocolTestGenerator.java +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/HttpProtocolTestGenerator.java @@ -107,7 +107,7 @@ public HttpProtocolTestGenerator( ) { this.settings = context.getSettings(); this.model = context.getModel(); - this.protocol = context.getSettings().getProtocol(); + this.protocol = protocolGenerator.getProtocol(); this.service = settings.getService(model); this.symbolProvider = context.getSymbolProvider(); this.protocolGenerator = protocolGenerator;