aws-cdk icon indicating copy to clipboard operation
aws-cdk copied to clipboard

feat(apigatewayv2-integrations): `WebSocketMockIntegration` props

Open nmussy opened this issue 1 year ago • 5 comments

Issue # (if applicable)

None

Reason for this change

Unlike the other integrations, WebSocketMockIntegration did not have a props interface, and was missing a few properties. This PR does not include integration responses, they are already being implemented in #29661

Description of changes

  • Added requestTemplates and templateSelectionExpression to the newly created WebSocketMockIntegrationProps

Description of how you validated changes

Unit and integration tests were updated

Checklist


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

nmussy avatar Jun 22 '24 08:06 nmussy

Exemption Request: WebSocketMockIntegration is currently not mentioned in any README file

nmussy avatar Jun 22 '24 08:06 nmussy

Exemption Request: WebSocketMockIntegration is currently not mentioned in any README file

I don't think this is a good enough reasoning. Some documentation should have been added when this construct is initially added. Would you please add somethiing to the README to explain the usage and what this is for? Appreciate it.

GavinZZ avatar Aug 28 '24 16:08 GavinZZ

Sorry for the late reply, catching up on my notifications. I've added a section in the README for mock integrations.

nit comment: Can you update the integ test file name to be more descriptive? something like websocket-mock-integration or anything align the line.

I understand the possible source for confusion here, but the existing integration files for the response types follow the same naming pattern:

$ tree packages/@aws-cdk-testing/framework-integ/test/aws-apigatewayv2-integrations/test/websocket/ -L 1 -I '*/' --gitignore
packages/@aws-cdk-testing/framework-integ/test/aws-apigatewayv2-integrations/test/websocket/
├── integ.aws.ts
├── integ.lambda-connect-disconnect-trigger.ts
├── integ.lambda.ts
├── integ.mock.ts
├── integ.route-response.ts
└── integ.sqs.ts

$ tree packages/@aws-cdk-testing/framework-integ/test/aws-apigatewayv2-integrations/test/http/ -L 1 -I '*/' --gitignore
packages/@aws-cdk-testing/framework-integ/test/aws-apigatewayv2-integrations/test/http/
├── integ.add-subroute-integration.ts
├── integ.alb.ts
├── integ.http-proxy.ts
├── integ.lambda-proxy.ts
├── integ.nlb.ts
├── integ.service-discovery.ts
└── integ.stepfunctions.ts

Let me know how you want to handle this, but only renaming integ.mock.ts would make it look out of place. I also think there is enough context in the relative file path, adding more would be a bit repetitive (aws-apigatewayv2-integrations/test/websocket/integ.mock.ts)

nmussy avatar Dec 09 '24 15:12 nmussy

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 81.40%. Comparing base (3fbc785) to head (e77f094). Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #30622   +/-   ##
=======================================
  Coverage   81.40%   81.40%           
=======================================
  Files         223      223           
  Lines       13727    13727           
  Branches     2411     2411           
=======================================
  Hits        11175    11175           
  Misses       2274     2274           
  Partials      278      278           
Flag Coverage Δ
suite.unit 81.40% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
packages/aws-cdk 80.74% <ø> (ø)
packages/aws-cdk-lib/core 82.10% <ø> (ø)

codecov[bot] avatar Dec 09 '24 15:12 codecov[bot]

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

aws-cdk-automation avatar Jan 08 '25 00:01 aws-cdk-automation

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify[bot] avatar Jan 14 '25 21:01 mergify[bot]

@mergifyio refresh

kaizencc avatar Jan 15 '25 16:01 kaizencc

refresh

✅ Pull request refreshed

mergify[bot] avatar Jan 15 '25 16:01 mergify[bot]

@mergifyio update

kaizencc avatar Jan 15 '25 17:01 kaizencc

update

✅ Branch has been successfully updated

mergify[bot] avatar Jan 15 '25 17:01 mergify[bot]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify[bot] avatar Jan 15 '25 17:01 mergify[bot]

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: e77f094b5e7aaf7aa63faf82e43151824efabdce
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation avatar Jan 15 '25 18:01 aws-cdk-automation

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify[bot] avatar Jan 15 '25 18:01 mergify[bot]

Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.

github-actions[bot] avatar Jan 15 '25 18:01 github-actions[bot]