Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore(scheduler-targets): lambda invoke target integration test #27088

Merged
merged 8 commits into from
Sep 29, 2023
Merged

chore(scheduler-targets): lambda invoke target integration test #27088

merged 8 commits into from
Sep 29, 2023

Conversation

filletofish
Copy link
Contributor

@filletofish filletofish commented Sep 10, 2023 •
edited
Loading

Continuing @Jacco work from #26268

Currently we were second-guessing if the implementations of ScheduleExpression and ScheduleTargetInput generate valid and correct CloudFormation.

This integration test verifies that lambda invoke target is invoked:

  • A schedule is deployed that triggers a Lambda every minute after deployment.
  • The function adds a Tag to itself Key=OutputValue Value=base64 of the JSON string made of the input event
  • An assertion is made that checks for 10 minutes if the expected Tag appears

Advances #23394

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

Jacco and others added 2 commits September 9, 2023 15:34
@Jacco @filletofish
Integration Test
1e91d1d
@filletofish
Update integration test
6ab012d 
- Set schedule to recurring every minute
- Move test to targets package
- Minor updates of how Scheduler API is used
@github-actions github-actions bot added effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p1 labels Sep 10, 2023
@aws-cdk-automation aws-cdk-automation requested a review from a team September 10, 2023 19:02
@github-actions github-actions bot added the repeat-contributor [Pilot] contributed between 3-5 PRs to the CDK label Sep 10, 2023
@aws-cdk-automation aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Sep 10, 2023
@kaizencc kaizencc changed the title chore(scheduler): Integration Test chore(scheduler): integration test Sep 12, 2023
kaizencc
kaizencc previously requested changes Sep 13, 2023
View reviewed changes
Copy link
Contributor

@kaizencc kaizencc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@filletofish a few comments but otherwise this should be good. thanks for writing this!

packages/@aws-cdk/aws-scheduler-targets-alpha/test/integ.schedule.ts Outdated
runtime: lambda.Runtime.NODEJS_18_X,
functionName: functionName,
});
func.addEnvironment('FUNC_ARN', stack.resolve(functionArn));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we have to add this and why can't we use func.functionArn

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise - it creates a cyclic dependency

packages/@aws-cdk/aws-scheduler-targets-alpha/test/integ.schedule.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-scheduler-targets-alpha/test/integ.schedule.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-scheduler-targets-alpha/test/integ.schedule.ts Outdated Show resolved Hide resolved
@kaizencc kaizencc changed the title chore(scheduler): integration test chore(scheduler-targets): integration test Sep 13, 2023
@aws-cdk-automation aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Sep 13, 2023
@mergify mergify bot dismissed kaizencc’s stale review September 23, 2023 15:24

Pull request has been modified.

@filletofish
Update lambda invoke integration tests
f0d2f24 
- Rename the schedule test to lambda invoke test
- Do not create IAM role
- Update documentation / comments
- Move target lambda function code from inline into the asset
 (separate file)
@filletofish filletofish changed the title chore(scheduler-targets): integration test chore(scheduler-targets): lambda invoke target integration test Sep 23, 2023
@filletofish
Copy link
Contributor Author

@kaizencc could you please restart the build? it failed with seemingly transient error when attempting to download packages from NPM

@aws-cdk-automation aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Sep 25, 2023
rix0rrr
rix0rrr previously approved these changes Sep 27, 2023
View reviewed changes
@mergify
Copy link
Contributor

mergify bot commented Sep 27, 2023

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).

@aws-cdk-automation aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Sep 27, 2023
@mergify
Copy link
Contributor

mergify bot commented Sep 27, 2023

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 mergify bot dismissed rix0rrr’s stale review September 29, 2023 11:53

Pull request has been modified.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

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

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

@mergify
Copy link
Contributor

mergify bot commented Sep 29, 2023

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 mergify bot merged commit a1f2ec2 into aws:main Sep 29, 2023
9 checks passed
@github-actions github-actions bot mentioned this pull request Oct 1, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kaizencc kaizencc kaizencc left review comments

@mrgrain mrgrain mrgrain approved these changes

@rix0rrr rix0rrr rix0rrr left review comments

Assignees
No one assigned
Labels
effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p1 repeat-contributor [Pilot] contributed between 3-5 PRs to the CDK
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@filletofish @aws-cdk-automation @mrgrain @rix0rrr @kaizencc @Jacco