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

Support mark_utilized in available_prefix resource #111

Merged
merged 1 commit into from
Jan 20, 2022

Conversation

cova-fe
Copy link
Contributor

@cova-fe cova-fe commented Jan 14, 2022

This small patch simply add the mark_utilized parameter for available_prefix resource. Changes in data source are only to make my linter happy :)

@fbreckle
Copy link
Collaborator

Which linter do you use?

Also, please add tests!

@cova-fe
Copy link
Contributor Author

cova-fe commented Jan 14, 2022

I added some tests, please let me know if it is what you had in mind.
I'm using a gopls under neovim, and the message I get is like this one for data_source_netbox_tag.go:
16 "name": &schema.Schema{ redundant type from array, slice, or map composite literal

@fbreckle
Copy link
Collaborator

Can this attribute be unset, or does this run into the same issue as #109
Maybe you can unset the attribute in a test and then see if it is false

@cova-fe
Copy link
Contributor Author

cova-fe commented Jan 17, 2022

Can this attribute be unset, or does this run into the same issue as #109 Maybe you can unset the attribute in a test and then see if it is false

Just tested (by hand, it was faster than adding a test and see it failing without hope :) ). The parameter is not shown in the API call JSON when it should be changed to "false". The change is planned and apply reports it, but no changes are made by the PUT call. So it seems that the issue #109 hits also in this case. I guess that for the time being we can skip the true->false transition test. I'll be happy to add the test at a later date, including my other change previously merged. (tenant_id in vrf)

@fbreckle
Copy link
Collaborator

fbreckle commented Jan 19, 2022

You could add a test that unsets / sets to false the attribute and then verifies that it is false. This will probably make the test fail.

You can then use the ExpectError attribute of https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/helper/resource#TestStep to make a test that is expected to fail.

This would acknowledge the bug in the codebase. After fixing the bug in go-netbox, the test will "fail" (since it expects an error but doesnt get one) and the ExpectError can be removed.

Let me know if you want to do that, else I just merge and eventually maybe an issue pops up.

@cova-fe
Copy link
Contributor Author

cova-fe commented Jan 19, 2022

You mean something like this?

@fbreckle
Copy link
Collaborator

Absolutely! 👍

@fbreckle fbreckle merged commit a2d4162 into e-breuninger:master Jan 20, 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.

2 participants