-
Notifications
You must be signed in to change notification settings - Fork 77
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
upgrade heroku-go #169
upgrade heroku-go #169
Conversation
NOTE: while working on this, I noticed an issue with the way we reference buildpacks, this may break
Nice @shawncatz 🙌 Yes indeed, thanks for bringing up buildpack URLs 🆚 names. I created #170 to track that. When you say that you "fixed this in my other PR", what do you mean? |
sorry, I misspoke, didn't mean fixed, just meant that I made the change to make the tests pass... but we still need to work out how we want to handle buildpack URLs vs names |
The fact that “All checks have passed” above does not really mean the tests pass. Many are skipped for PR’s, only run by Hashicorp’s CI to gate release. I’ve started a local run of full acceptance tests to confirm this PR is okay. |
Yes, of course, I understand |
Okay cool, we can merge this in then look at #167. |
The run finished in about 1h 50m, and failed because some tests errored or skipped due to misconfiguration on my part. (Result output pasted below.) I'm going to try to fix my mis-configs and get this to pass today. Not sure what the
|
Awesome thanks for that @mars |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed-up the test params and re-ran the tests that errored or skipped in the last run. Also, opened PR #171 to clarify the test params including the slug requirements. So, with that…
✅ The full suite passes
$ HEROKU_API_KEY=xxxxx \
[email protected] \
HEROKU_SPACES_ORGANIZATION=xxxxx \
HEROKU_ORGANIZATION=xxxxx \
HEROKU_SLUG_ID=xxxxx \
[email protected] \
[email protected] \
make testacc \
TEST="./heroku/" \
TESTARGS='"-run=TestAccHerokuAppRelease_importBasic|TestAccHerokuFormation_importBasic|TestAccHerokuSpaceAppAccess_importBasic|TestAccHerokuTeamCollaborator_importBasic|TestAccHerokuTeamMember_importBasic|TestAccHerokuAppRelease_Basic|TestAccHerokuAppRelease_OrgBasic|TestAccHerokuFormationSingleUpdate_WithOrg|TestAccHerokuFormationUpdateFreeDyno|TestAccHerokuSpaceAppAccess_Basic|TestAccHerokuTeamCollaborator_Org|TestAccHerokuTeamCollaboratorPermsOutOfOrder_Org|TestAccHerokuTeamMember_Org"'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./heroku/ -v "-run=TestAccHerokuAppRelease_importBasic|TestAccHerokuFormation_importBasic|TestAccHerokuSpaceAppAccess_importBasic|TestAccHerokuTeamCollaborator_importBasic|TestAccHerokuTeamMember_importBasic|TestAccHerokuAppRelease_Basic|TestAccHerokuAppRelease_OrgBasic|TestAccHerokuFormationSingleUpdate_WithOrg|TestAccHerokuFormationUpdateFreeDyno|TestAccHerokuSpaceAppAccess_Basic|TestAccHerokuTeamCollaborator_Org|TestAccHerokuTeamCollaboratorPermsOutOfOrder_Org|TestAccHerokuTeamMember_Org" -timeout 120m -ldflags="-X=github.com/terraform-providers/terraform-provider-heroku/version.ProviderVersion=test"
=== RUN TestAccHerokuAppRelease_importBasic
--- PASS: TestAccHerokuAppRelease_importBasic (9.28s)
=== RUN TestAccHerokuFormation_importBasic
--- PASS: TestAccHerokuFormation_importBasic (11.79s)
=== RUN TestAccHerokuSpaceAppAccess_importBasic
--- PASS: TestAccHerokuSpaceAppAccess_importBasic (451.47s)
=== RUN TestAccHerokuTeamCollaborator_importBasic
--- PASS: TestAccHerokuTeamCollaborator_importBasic (13.26s)
=== RUN TestAccHerokuTeamMember_importBasic
--- PASS: TestAccHerokuTeamMember_importBasic (1.25s)
=== RUN TestAccHerokuAppRelease_Basic
--- PASS: TestAccHerokuAppRelease_Basic (8.68s)
=== RUN TestAccHerokuAppRelease_OrgBasic
--- PASS: TestAccHerokuAppRelease_OrgBasic (11.83s)
=== RUN TestAccHerokuFormationSingleUpdate_WithOrg
--- PASS: TestAccHerokuFormationSingleUpdate_WithOrg (11.68s)
=== RUN TestAccHerokuFormationUpdateFreeDyno
--- PASS: TestAccHerokuFormationUpdateFreeDyno (8.71s)
=== RUN TestAccHerokuSpaceAppAccess_Basic
--- PASS: TestAccHerokuSpaceAppAccess_Basic (438.80s)
=== RUN TestAccHerokuTeamCollaborator_Org
--- PASS: TestAccHerokuTeamCollaborator_Org (12.18s)
=== RUN TestAccHerokuTeamCollaboratorPermsOutOfOrder_Org
--- PASS: TestAccHerokuTeamCollaboratorPermsOutOfOrder_Org (12.29s)
=== RUN TestAccHerokuTeamMember_Org
--- PASS: TestAccHerokuTeamMember_Org (1.14s)
PASS
ok github.com/terraform-providers/terraform-provider-heroku/heroku 992.417s
Any other concerns before we merge? @davidji99 @talbright @joestump |
No concern...you guys are on top of things, lgtm! |
NOTE: while working on this, I noticed an issue with the way we reference buildpacks, this may break. I'd like reviewers thoughts on how to change this (a buildpack sub resource, checking if the value is a URL)
(I made a change this in my other PR, but was ruminating as I was pulling those changes here and I see issues potentially with people mixing Buildpack Registry names and URLs for custom buildpacks, so the resource may need to change to support them separately)
Break out the
heroku-go
vendor update from #167