heroku_addon_attachment #19
Conversation
…xisting addons to be attached to additional apps
…ed Name to the heroku_addon schema (so it matches the documentation)
There was a problem hiding this comment.
Hey @JamesBelchamber
Thanks for this PR, I've taken a look through and this mostly LGTM - if we can add an extra acceptance test covering the optional name field then this should be good to merge :)
Thanks!
| app_id = "%s" | ||
| addon_id = "%s" | ||
| }`, appName, addonName) | ||
| } |
There was a problem hiding this comment.
could we also add an additional test making use of the name field?
|
The tests were a bit dumb anyway, I've refactored them and added an extra one :) |
| func testAccCheckHerokuAddonAttachmentConfig_named(appName string, addonName string, name string) string { | ||
| return fmt.Sprintf(` | ||
| resource "heroku_addon_attachment" "foobar" { | ||
| app_id = "%s" |
There was a problem hiding this comment.
Thanks for adding the new test - I've just tried running the tests and they currently fail on an empty subscription due to the missing Heroku App:
$ acctests heroku TestAccHerokuAddonAttachment_
=== RUN TestAccHerokuAddonAttachment_Basic
--- FAIL: TestAccHerokuAddonAttachment_Basic (1.40s)
testing.go:434: Step 0 error: Error applying: 1 error(s) occurred:
* heroku_addon_attachment.foobar: 1 error(s) occurred:
* heroku_addon_attachment.foobar: Post https://api.heroku.com/addon-attachments: No such app.
=== RUN TestAccHerokuAddonAttachment_Named
--- FAIL: TestAccHerokuAddonAttachment_Named (0.19s)
testing.go:434: Step 0 error: Error applying: 1 error(s) occurred:
* heroku_addon_attachment.foobar: 1 error(s) occurred:
* heroku_addon_attachment.foobar: Post https://api.heroku.com/addon-attachments: No such app.
FAIL
exit status 1
FAIL github.com/terraform-providers/terraform-provider-heroku/heroku 1.617s
.. could we update this to create the Heroku App as part of the tests so the tests are standalone? However this otherwise LGTM 👍
There was a problem hiding this comment.
Hmm. I haven't been running acctest so I didn't realise that. I'll get it working, will probably need to create an addon too.
There was a problem hiding this comment.
@JamesBelchamber no worries, for context from the snippet above I'm using acctests as a bash wrapper for TF_ACC=1 go test ./heroku ... as it's less typing :)
There was a problem hiding this comment.
Is there a way to focus the test to a single resource? I'd rather avoid creating chargeable resources that I don't need to.
There was a problem hiding this comment.
sure - this command will run just a single test (the test name is a prefix match):
TF_ACC=1 go test ./heroku -v -timeout 120m -run=TestAccHerokuAddonAttachment_Basic
hope that helps - let me know if you want me to run any of the tests to confirm too :)
|
That should work better, we're creating the app and the addon now :) |
Passes tests :) |
There was a problem hiding this comment.
LGTM - thanks for this! Tests pass:
$ acctests heroku TestAccHerokuAddonAttachment_
=== RUN TestAccHerokuAddonAttachment_Basic
--- PASS: TestAccHerokuAddonAttachment_Basic (7.75s)
=== RUN TestAccHerokuAddonAttachment_Named
--- PASS: TestAccHerokuAddonAttachment_Named (6.89s)
PASS
ok github.com/terraform-providers/terraform-provider-heroku/heroku 14.664s
…hment heroku_addon_attachment
This creates the heroku_addon_attachment resource, which allows pre-existing addons to be attached to additional apps. Fixes #3