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.

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

Add tests for UpdateTaskProtection API to high-level TMDS tests #3740

Merged
merged 2 commits into from
Jun 12, 2023

Conversation

amogh09
Copy link
Contributor

@amogh09 amogh09 commented Jun 8, 2023

Summary

Add comprehensive unit tests for UpdateTaskProtection API to Task Metadata Server (TMDS) tests. These test cases are already covered by handler-level unit tests for UpdateTaskProtection handler, however, we will be moving the handler to ecs-agent module in upcoming PRs and having the API tested by TMDS tests will help gain confidence in the move.

This change is a part of a series of similar changes we have been making for many TMDS endpoints. See #3739, #3729, #3722, and #3708 that did the same for GetTaskProtection, taskWithTags, task metadata, and container metadata endpoints, respectively.

Implementation details

  • Update testTMDSRequest function and TMDSTestCase struct to support request body and arbitrary HTTP method. This is because UpdateTaskProtection API expects a request body and PUT HTTP method.
  • Add TestUpdateTaskProtection function to TMDS test file. This function contains tests for UpdateTaskProtection API.
  • Remove TestAgentAPIV1UpdateTaskProtectionHandler as it is superseded by TestUpdateTaskProtection.

Testing

New tests cover the changes: NA

Description for the changelog

Add tests for UpdateTaskProtection API to high-level TMDS tests

Licensing

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

@amogh09 amogh09 force-pushed the update-task-protection-tests branch from 5d76305 to 9beb80a Compare June 8, 2023 18:21
@amogh09 amogh09 marked this pull request as ready for review June 8, 2023 18:36
@amogh09 amogh09 requested a review from a team as a code owner June 8, 2023 18:36
}

// Test cases start here
t.Run("task ARN not found", func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: Could we perhaps use/invoke a helper function here instead of having an anonymous function call for each test case? It seems like they're doing the same thing, only with the setStateExpectations, expectedStatusCode, and expectedResponseBody fields being different across test cases.

Copy link
Contributor Author

@amogh09 amogh09 Jun 8, 2023

Choose a reason for hiding this comment

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

It is a standard practice to pass an anonymous function to t.Run call. That being said, I have added a commit that adds a helper function for running a test case and populating path and method fields that are common across all test cases. All other fields have variations across test cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants