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

test(ecs): add unit tests for drain hook lambda #27803

Closed
wants to merge 2 commits into from
Closed

test(ecs): add unit tests for drain hook lambda #27803

wants to merge 2 commits into from

Conversation

msambol
Copy link
Contributor

@msambol msambol commented Nov 1, 2023

Closes #27725.

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

@github-actions github-actions bot added feature-request A feature should be added or improved. p2 admired-contributor [Pilot] contributed between 13-24 PRs to the CDK labels Nov 1, 2023
@aws-cdk-automation aws-cdk-automation requested a review from a team November 1, 2023 23:51
@msambol
Copy link
Contributor Author

msambol commented Nov 1, 2023

I need some help on this one as I don't normally test Python functions this way.

  1. I'm not using MagicMock. Should I be?
  2. Is there something I need to do in the build to make sure this gets run?

@scanlonp
Copy link
Contributor

scanlonp commented Nov 6, 2023

@msambol it looks like the integ tests are failing since the handler code changed. That will cause the snapshots to be different. Simply running those integ tests should be good to get the build passing.

I am not as sure about needing MagicMock, I can look into it.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 38778be
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@msambol msambol mentioned this pull request Nov 8, 2023
Merged
@aws-cdk-automation
Copy link
Collaborator

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

17 similar comments
@aws-cdk-automation
Copy link
Collaborator

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@aws-cdk-automation
Copy link
Collaborator

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@aws-cdk-automation
Copy link
Collaborator

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@aws-cdk-automation
Copy link
Collaborator

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@aws-cdk-automation
Copy link
Collaborator

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@aws-cdk-automation
Copy link
Collaborator

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@aws-cdk-automation
Copy link
Collaborator

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@aws-cdk-automation
Copy link
Collaborator

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@aws-cdk-automation
Copy link
Collaborator

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@aws-cdk-automation
Copy link
Collaborator

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@aws-cdk-automation
Copy link
Collaborator

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@aws-cdk-automation
Copy link
Collaborator

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@aws-cdk-automation
Copy link
Collaborator

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@aws-cdk-automation
Copy link
Collaborator

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@aws-cdk-automation
Copy link
Collaborator

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@aws-cdk-automation
Copy link
Collaborator

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@aws-cdk-automation
Copy link
Collaborator

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@aws-cdk-automation
Copy link
Collaborator

This PR has been in the BUILD FAILING 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.

@github-actions github-actions bot mentioned this pull request Dec 1, 2023
Closed
@aws-cdk-automation
Copy link
Collaborator

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

@aws-cdk-automation aws-cdk-automation added the closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. label Dec 5, 2023
mergify bot pushed a commit that referenced this pull request Feb 12, 2024
@msambol
fix(stepfunctions-tasks): missing permissions for running tasks on ecs (
683d595 
#27891)

While working on [#27803](#27803), I noticed the integration tests for `aws-stepfunctions-tasks/ecs` were not fully working (they deployed but the state machines did not run successfully). This PR addresses two issues:

1. Missing permissions for `ecs:RunTask` on the task definition version.
<img width="1587" alt="sfn-role" src="https://github.com/aws/aws-cdk/assets/3310356/13a0d402-8cbb-4852-9708-290f3a3b6711">

2. The sample container was from a Lambda image. This resulted in the following error: `entrypoint requires the handler name to be the first argument`. I changed the image to `docker/library/python:3.12`.

These changes result in the successful execution of all four state machines in `aws-stepfunctions-tasks/ecs`.


----
*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
GavinZZ pushed a commit that referenced this pull request Feb 22, 2024
@msambol @GavinZZ
fix(stepfunctions-tasks): missing permissions for running tasks on ecs (
828c57f 
#27891)

While working on [#27803](#27803), I noticed the integration tests for `aws-stepfunctions-tasks/ecs` were not fully working (they deployed but the state machines did not run successfully). This PR addresses two issues:

1. Missing permissions for `ecs:RunTask` on the task definition version.
<img width="1587" alt="sfn-role" src="https://github.com/aws/aws-cdk/assets/3310356/13a0d402-8cbb-4852-9708-290f3a3b6711">

2. The sample container was from a Lambda image. This resulted in the following error: `entrypoint requires the handler name to be the first argument`. I changed the image to `docker/library/python:3.12`.

These changes result in the successful execution of all four state machines in `aws-stepfunctions-tasks/ecs`.


----
*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
admired-contributor [Pilot] contributed between 13-24 PRs to the CDK closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. feature-request A feature should be added or improved. p2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

(ecs): create unit tests for drain hook lambda handler
3 participants
@msambol @scanlonp @aws-cdk-automation