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

[testing] Acceptance tests for rhoas_kafka #24

Merged
merged 19 commits into from
Sep 28, 2022

Conversation

juandspy
Copy link
Collaborator

@juandspy juandspy commented Sep 23, 2022

Summary

Added some acceptance tests for rhoas_kafka:

  • TestAccRHOASKafka_Basic: checks that we are able to spin up a Kafka cluster and delete it.
  • TestAccRHOASKafka_Update: checks that we are able to spin up a Kafka cluster, change its name and delete it.
    • This one is failing because updating the name makes the resource to be replaced.
    • This is because the update function is not yet implemented.
    • I left this one commented waiting for the update feature to come.
  • TestAccRHOASKafka_Error: checks that an error is returned if the configuration is wrong. For example, if the cloud provider isn't supported.

Testing

Run make testacc. You have to export your OFFLINE_TOKEN variable first.

@juandspy juandspy marked this pull request as ready for review September 23, 2022 14:00
Comment on lines +146 to +148
if !strings.Contains(err.Error(), "not found") {
return diag.FromErr(errors.Wrapf(err, "Error waiting for example instance (%s) to be deleted", d.Id()))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I am missing something here but why do we not care about the error if it contains "not found"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I found that sometimes the API returns an error "not found" when the resource has been deleted and the terraform provider fails. So I thought that a "not found" is OK if we are deleting the resource

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay this is kind of weird but maybe there is no way around it.

Comment on lines +126 to +128
if err1.Error() == "404 Not Found" {
return data, "404", nil
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we checking this just not sure? Also we will do more error handling in the future anyway so nothing needs to be perfect right now.

Copy link
Collaborator Author

@juandspy juandspy Sep 28, 2022

Choose a reason for hiding this comment

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

Similarly to this comment, I discovered that sometimes the API returns a "404 Not Found" and the delete function errors. I think that if we get a 404, there shouldn't be any error, as not finding the resource means it has been deleted.

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, bit weird but as I said not sure how to get around it right now.

@juandspy juandspy marked this pull request as ready for review September 28, 2022 08:37
Copy link
Contributor

@jackdelahunt jackdelahunt left a comment

Choose a reason for hiding this comment

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

👍

@jackdelahunt jackdelahunt merged commit 29dbeb2 into redhat-developer:main Sep 28, 2022
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.

None yet

2 participants