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

Add tests for remaining aws integrations #48

Closed
wants to merge 9 commits into from
Closed

Add tests for remaining aws integrations #48

wants to merge 9 commits into from

Conversation

nmuesch
Copy link
Contributor

@nmuesch nmuesch commented Dec 17, 2019 •
edited
Loading

Adds small tests for the generate new external id and list namespaces and two logs async endpoints that are now generated.

Also addresses missed review from - #32

Also uses the updated schema that uses the more standard Request/Response instead of Input/Output

@nmuesch nmuesch requested a review from a team as a code owner December 17, 2019 21:36
@gzussa
Copy link
Contributor

gzussa commented Dec 19, 2019

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jirikuncar
Copy link
Contributor

@nmuesch can you have a look at these errors?

# github.com/DataDog/datadog-api-client-go/api/v1/datadog_test [github.com/DataDog/datadog-api-client-go/api/v1/datadog.test]
./api_aws_integration_test.go:149:84: cannot use testAwsAccount (type datadog.AwsAccount) as type datadog.AwsAccountCreateResponse in argument to TESTAPICLIENT.AWSIntegrationApi.GenerateNewAWSExternalID
./api_aws_integration_test.go:154:26: apiResp.GetExternalId undefined (type datadog.Error400 has no field or method GetExternalId)
./api_aws_logs_integration_test.go:12:61: undefined: datadog.AwsAccountAndLambdaRequest
./api_aws_logs_integration_test.go:12:97: undefined: datadog.AwsLogsServicesRequest
./api_aws_logs_integration_test.go:22:22: undefined: datadog.AwsAccountAndLambdaRequest
./api_aws_logs_integration_test.go:27:22: undefined: datadog.AwsLogsServicesRequest
./api_aws_logs_integration_test.go:117:23: undefined: datadog.AwsLogsListResponseLambdas
./api_aws_logs_integration_test.go:152:28: status.GetErrors undefined (type datadog.AwsLogsAsyncResponse has no field or method GetErrors)
./api_aws_logs_integration_test.go:163:25: status.GetErrors undefined (type datadog.AwsLogsAsyncResponse has no field or method GetErrors)
./api_aws_logs_integration_test.go:164:25: status.GetErrors undefined (type datadog.AwsLogsAsyncResponse has no field or method GetErrors)
./api_aws_logs_integration_test.go:164:25: too many errors
FAIL	github.com/DataDog/datadog-api-client-go/api/v1/datadog [build failed]

@nmuesch
Copy link
Contributor Author

nmuesch commented Dec 19, 2019

Yupp, those errors are expected until the related spec changes are merged :)

@nmuesch nmuesch changed the title Add tests for two remaining aws integrations Add tests for remaining aws integrations Dec 20, 2019
jirikuncar
jirikuncar reviewed Dec 20, 2019
View reviewed changes
api/v1/datadog/api_aws_logs_integration_test.go
if err != nil || httpresp.StatusCode != 200 {
t.Errorf("Error deleting Lambda %v: Response %s: %v", deleteOutput, err.(datadog.GenericOpenAPIError).Body(), err)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think Hippo has just changed it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I went through PR #51 and cleaned up this + one other example I found in the GCP test. Should be all good now.

jirikuncar
jirikuncar suggested changes Dec 20, 2019
View reviewed changes
Copy link
Contributor

@jirikuncar jirikuncar left a comment

Choose a reason for hiding this comment

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

Please check the merge conflict resolution so we are not reverting past changes. Thanks!

zippolyte
zippolyte requested changes Dec 20, 2019
View reviewed changes
api/v1/datadog/api_aws_integration_test.go Outdated Show resolved Hide resolved
api/v1/datadog/api_aws_integration_test.go Outdated Show resolved Hide resolved
api/v1/datadog/api_aws_logs_integration_test.go Outdated Show resolved Hide resolved
api/v1/datadog/api_aws_logs_integration_test.go Outdated Show resolved Hide resolved
api/v1/datadog/api_aws_logs_integration_test.go Outdated Show resolved Hide resolved
api/v1/datadog/api_aws_logs_integration_test.go Outdated Show resolved Hide resolved
@nmuesch
Copy link
Contributor Author

nmuesch commented Dec 20, 2019

@zippolyte @jirikuncar Thanks for the reviews, I went though and changed most Errorf to Fatalf since it looks like we're moving towards there (and makes sense to fail quick there)

I also addressed the feedback for making sure we print the response body everywhere and assert we have 200 response code.

Can you take another pass?

gzussa
gzussa approved these changes Dec 23, 2019
View reviewed changes
Copy link
Contributor

@gzussa gzussa left a comment

Choose a reason for hiding this comment

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

:shipit:

@nmuesch nmuesch mentioned this pull request Dec 27, 2019
Merged
@nmuesch
Copy link
Contributor Author

nmuesch commented Dec 27, 2019

Will be included in #53

@gzussa
Copy link
Contributor

gzussa commented Dec 28, 2019

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@nmuesch
Copy link
Contributor Author

nmuesch commented Dec 28, 2019

Thanks for the reviews. This PR is inside #53 now, can I get an approval there, then I'll close this one out. Thanks :)

@nmuesch
Copy link
Contributor Author

nmuesch commented Dec 30, 2019

Closing, this was merged as part of #53

@nmuesch nmuesch closed this Dec 30, 2019
@dd-devflow dd-devflow bot deleted the datadog-api-spec/pr/162 branch August 21, 2024 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jirikuncar jirikuncar jirikuncar requested changes

@zippolyte zippolyte zippolyte requested changes

@gzussa gzussa gzussa approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nmuesch @gzussa @jirikuncar @zippolyte