Smoke Tests Codegen Working #3155

sbera87 · 2024-10-18T22:09:47Z

Issue #, if available:

Description of changes:

Check all that applies:

Did a review by yourself.
Added proper tests to cover this PR. (If tests are not applicable, explain.)
Checked if this PR is a breaking (APIs have been changed) change.
Checked if this PR will not introduce cross-platform inconsistent behavior.
Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

…ibeIntegrations and ModifyIntegration APIs to create and manage Amazon Redshift Zero-ETL Integrations. Amazon Q Business now supports embedding the Amazon Q Business web experience on third-party websites. On a channel that you own, you can now replace an ongoing stream with a new stream by streaming up with the priority parameter appended to the stream key. This release adds support for email maximum delivery seconds that allows senders to control the time within which their emails are attempted for delivery. Enable proxy for reserved capacity fleet. Added sourceUrlType field to StartDeployment request Documentation update for AWS CloudFormation API Reference. AWS Resilience Hub now integrates with the myApplications platform, enabling customers to easily assess the resilience of applications defined in myApplications. The new Resiliency widget provides visibility into application resilience and actionable recommendations for improvement. We are expanding support for 40 new locales in AWS Transcribe Streaming.

…eters in the ListBuckets API. For ListBuckets requests that express pagination, Amazon S3 will now return both the bucket names and associated AWS regions in the response.

Updates Amazon RDS documentation for TAZ IAM support Add StartDashboardSnapshotJobSchedule API. RestoreAnalysis now supports restoring analysis to folders. This release adds validation to require specifying a SecurityGroup and Subnets in the Vpc object under PipesSourceSelfManagedKafkaParameters. It also adds support for iso-e, iso-f, and other non-commercial partitions in ARN parameters. This release adds Data Grant support, through which customers can programmatically create data grants to share with other AWS accounts and accept data grants from other AWS accounts. Added the registrations status of REQUIRES_AUTHENTICATION Updated the DomainName pattern for Active Directory Removing support for topK property in PromptModelInferenceConfiguration object, Making PromptTemplateConfiguration property as required, Limiting the maximum PromptVariant to 1

…state

…n a bad state" This reverts commit a8f6c9c.

RequestSpotInstances and RequestSpotFleet feature release. Adding converse support to CMI API's Removing FEDERATED from Create/List/Delete/GetDataCatalog API Adding the following project member designations: PROJECT_CATALOG_VIEWER, PROJECT_CATALOG_CONSUMER and PROJECT_CATALOG_STEWARD in the CreateProjectMembership API and PROJECT_CATALOG_STEWARD designation in the AddPolicyGrant API.

…ibeIntegrations and ModifyIntegration APIs to create and manage Amazon Redshift Zero-ETL Integrations. Amazon Q Business now supports embedding the Amazon Q Business web experience on third-party websites. On a channel that you own, you can now replace an ongoing stream with a new stream by streaming up with the priority parameter appended to the stream key. This release adds support for email maximum delivery seconds that allows senders to control the time within which their emails are attempted for delivery. Enable proxy for reserved capacity fleet. Added sourceUrlType field to StartDeployment request Documentation update for AWS CloudFormation API Reference. AWS Resilience Hub now integrates with the myApplications platform, enabling customers to easily assess the resilience of applications defined in myApplications. The new Resiliency widget provides visibility into application resilience and actionable recommendations for improvement. We are expanding support for 40 new locales in AWS Transcribe Streaming.

sbiscigl · 2024-10-25T13:14:47Z

cmake/sdks.cmake

@@ -162,7 +162,6 @@ if(BUILD_ONLY)
        else()
            set(SDK_DIR "generated/src/aws-cpp-sdk-${SDK}")
        endif()
-


still has white space change

...src/main/java/com/amazonaws/util/awsclientsmithygenerator/generators/CppImportContainer.java

sbiscigl · 2024-10-25T13:40:57Z

...src/main/java/com/amazonaws/util/awsclientsmithygenerator/generators/CppImportContainer.java

+        "algorithm"
+        );
+
+        this.unitTestHeaders = new HashSet<>();


dont create a new hash set and add to it, use ImmtuableList.of

In this case, it's a set .So I will just use Set.of()

sbiscigl · 2024-10-25T13:46:11Z

...rc/main/java/com/amazonaws/util/awsclientsmithygenerator/generators/ClientConfiguration.java

+import software.amazon.smithy.aws.smoketests.model.S3VendorParams;
+
+public final class ClientConfiguration {
+    private final AwsVendorParams awsParams;


AwsVendorParams and S3VendorParams both extend BaseAwsVendorParams, this is a anti pattern to access through a boolean "has type" pattern. the member variable should be of BaseAwsVendorParams, and when you build it assign it a concrete type then just refer to the configuration through the abstract type

ack. changed to base class and use instance of.

sbiscigl · 2024-10-25T13:50:45Z

.../main/java/com/amazonaws/util/awsclientsmithygenerator/generators/GenericCodegenAdapter.java

+    {
+        if(shape == null)
+        {
+            throw new Exception("Invalid shape found");


dont throw a Exception, it makes everyone up chain check it, either throw a generic RuntimeException or create a class that subclasses RuntimeException

sbiscigl · 2024-10-25T13:53:42Z

...n/src/main/java/com/amazonaws/util/awsclientsmithygenerator/generators/SmokeTestsParser.java

+            throw new Exception(String.format("No service trait detected in service shape with name=%s",serviceShape.getId().getName()));
+        }
+
+        //this.model.getShape(serviceShapeId)


still commented out

sbiscigl · 2024-10-25T13:55:13Z

...main/java/com/amazonaws/util/awsclientsmithygenerator/generators/SmokeTestsSourceWriter.java

+    {
+        write("${L|}",
+
+        "#include <aws/testing/AwsCppSdkGTestSuite.h>\n" + //


still here in latest commit, at least the function at least,if its unused, remove it

sbiscigl · 2024-10-25T13:56:38Z

...n/src/main/java/com/amazonaws/util/awsclientsmithygenerator/generators/CppSymbolVisitor.java

+     @Override
+     public Symbol enumShape(EnumShape shape) {
+
+        // return ShapeVisitor.super.enumShape(shape);


commented out code, inconsistent spacing

ack. oversight

sbiscigl · 2024-10-25T13:59:59Z

...n/src/main/java/com/amazonaws/util/awsclientsmithygenerator/generators/SmokeTestsParser.java

+    {   
+        if(!serviceShape.getTrait(ServiceTrait.class).isPresent())
+        {
+            throw new Exception(String.format("No service trait detected in service shape with name=%s",serviceShape.getId().getName()));


dont throw exception, everyon up chain has to check it, never throw or check Exception, subclass or throw a RuntimeException

What is your recommendation how I should handle this? I understand that stack unwinding will happen on exception but what do i return when service trait is missing. I can throw a Runtimeexception

they have a concept of "checked and unchecked exceptions" in java.
additionally, simple regular Exception is too broad and the exception handling error code must catch it and do it's logic accordingly to the exception string message (vs simply type of the exception).

ack. I will switch to runtime exception as it is one of those cases where I have to throw if service trait not found

...n/src/main/java/com/amazonaws/util/awsclientsmithygenerator/generators/SmokeTestsParser.java

sbera87 · 2024-10-25T18:55:09Z

Left some comments, have some concerns around exception handling and the structure of some of the operations. but overall the execution pattern looks good, just need to refactor some things.

also in the future make sure to remove commented code, system outs, and white space changes before marking the PR as ready to review

Addressed all the comments that were brought up. If there are some things missed, I will address

...n/src/main/java/com/amazonaws/util/awsclientsmithygenerator/generators/SmokeTestsParser.java

sbiscigl · 2024-10-28T13:15:29Z

...egen/src/main/java/com/amazonaws/util/awsclientsmithygenerator/generators/SmokeTestData.java

+    public boolean expectSuccess;
+    public Optional<String> errorShapeId;
+    //capture auth scheme as that decides the client constructor
+    String auth;


nit: why is this member the only one with a comment and the only one without access modifier?

ack. missed the specifier, shall add . The comment for this field is because this is a derived field, not parsed directly from the smoke test trait in model since its available in the service trait

sbiscigl · 2024-10-28T13:17:53Z

...rc/main/java/com/amazonaws/util/awsclientsmithygenerator/generators/SmithyCodegenPlugin.java

+        } catch (Exception e) {
+            // Handle the exception
+            System.err.println("An error occurred while running the smoke tests: " + e.getMessage());
+            e.printStackTrace();


the generation should fail. whatever invokes the generation in CI, should not fail. making it the callers responsibility to decide.

sbera87 added 14 commits October 15, 2024 21:25

v1 codegen

feeaccd

updates

76ef3f9

updates

f877902

codegen fixes

ae0b5b2

add service filter in configuration

1bba920

add deps

4dec4b3

deps

fbfb5c6

projections test

18f0386

working codegen from script

4b5a7f0

codegen script updates

0233376

working codegen with script

346c1f0

ignore temp files for smithy codegen

dc6da8d

cleanup + codegen fixes

fb8414b

tweaks

872c19b

sbera87 requested review from sbiscigl and SergeyRyabinin October 18, 2024 22:09

sbera87 added 2 commits October 18, 2024 18:11

remove temp files

049a36d

remove temp files

320e03d

sbera87 closed this Oct 18, 2024

sbera87 reopened this Oct 18, 2024

aws-sdk-cpp-automation and others added 10 commits October 20, 2024 00:32

Add support for the new optional bucket-region and prefix query param…

6b97b8c

…eters in the ListBuckets API. For ListBuckets requests that express pagination, Amazon S3 will now return both the bucket names and associated AWS regions in the response.

Update API model

785f364

Don't continue polling input stream in case it is closed or in a bad …

97e4f61

…state

Revert "Don't continue polling input stream in case it is closed or i…

cbad9dc

…n a bad state" This reverts commit a8f6c9c.

Update API model

b49ca3a

codegen script updates

9cb4e54

sbera87 added 5 commits October 23, 2024 00:46

cleanup

b27da0e

utilize importer for dependencies

31c8317

handle num

0c96460

cleanup + resolve PR comments

3f9f29b

restore

c75a2d7

sbera87 requested review from sbiscigl and SergeyRyabinin October 24, 2024 05:21

sbera87 added 3 commits October 24, 2024 08:37

remove helper for case conversion

efa5bb4

cleanup unused functions

1be4d61

remove commented lines

b27e018

sbiscigl reviewed Oct 25, 2024

View reviewed changes

sbera87 added 6 commits October 25, 2024 13:40

PR comments fix

e1c9608

cleanup

febfcaf

cleanup

c1a3350

cleanup

aec431a

cleanup

d336778

formatting fixes

d2c23a6

sbera87 requested a review from sbiscigl October 25, 2024 18:53

sbiscigl reviewed Oct 25, 2024

View reviewed changes

...n/src/main/java/com/amazonaws/util/awsclientsmithygenerator/generators/SmokeTestsParser.java Outdated Show resolved Hide resolved

sbiscigl reviewed Oct 25, 2024

View reviewed changes

...n/src/main/java/com/amazonaws/util/awsclientsmithygenerator/generators/SmokeTestsParser.java Outdated Show resolved Hide resolved

remove exceptions

b599f31

sbera87 requested a review from sbiscigl October 25, 2024 20:14

sbiscigl reviewed Oct 28, 2024

View reviewed changes

PR feedback changes

90f1772

sbera87 requested a review from sbiscigl October 28, 2024 18:05

remove catch exceptions

69600c5

sbiscigl approved these changes Oct 29, 2024

View reviewed changes

sbera87 merged commit 153e258 into main Oct 30, 2024
4 checks passed

sbera87 deleted the smoke-test-gen2 branch October 30, 2024 13:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Smoke Tests Codegen Working #3155

Smoke Tests Codegen Working #3155

sbera87 commented Oct 18, 2024 •

edited

Loading

sbiscigl Oct 25, 2024

sbiscigl Oct 25, 2024

sbera87 Oct 25, 2024

sbiscigl Oct 25, 2024

sbera87 Oct 25, 2024

sbiscigl Oct 25, 2024

sbera87 Oct 25, 2024

sbiscigl Oct 25, 2024

sbiscigl Oct 25, 2024

sbiscigl Oct 25, 2024

sbera87 Oct 25, 2024

sbiscigl Oct 25, 2024

sbera87 Oct 25, 2024 •

edited

Loading

SergeyRyabinin Oct 25, 2024

sbera87 Oct 25, 2024

sbera87 commented Oct 25, 2024

sbiscigl Oct 28, 2024

sbera87 Oct 28, 2024

sbiscigl Oct 28, 2024

Smoke Tests Codegen Working #3155

Smoke Tests Codegen Working #3155

Conversation

sbera87 commented Oct 18, 2024 • edited Loading

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

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

sbera87 Oct 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbera87 commented Oct 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbera87 commented Oct 18, 2024 •

edited

Loading

sbera87 Oct 25, 2024 •

edited

Loading