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

Handle the case when issue labels already exist #13182

Merged
merged 2 commits into from
Mar 31, 2017

Conversation

sethvargo
Copy link
Contributor

This fixes GH-13163

I'm not sure how to write an acceptance test for this, since I'd have to create the issue outside of the acceptance test first. Any suggestions or examples of where we do that already?

@radeksimko
Copy link
Member

Existing acceptance tests are passing, it would be nice to add another one covering this use case.
The code otherwise looks good, assuming conflicts are resolved.

I'd have to create the issue outside of the acceptance test first

How about we just create github_repository as part of the acceptance test which will have the mentioned default labels and then try modified one of the default labels we expect to exist there?

@radeksimko radeksimko added the waiting-response An issue/pull request is waiting for a response from the community label Mar 31, 2017
@radeksimko radeksimko requested review from radeksimko and removed request for stack72 March 31, 2017 08:52
@sethvargo sethvargo requested review from radeksimko and removed request for radeksimko March 31, 2017 16:02
var testAccGitHubIssueLabelExistsConfig string = fmt.Sprintf(`
// Create a repository which has the default labels
resource "github_repository" "test" {
name = "tf-acc-repo-label-abc1234"
Copy link
Member

Choose a reason for hiding this comment

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

I think we'll need to randomize this, but apparently the other tests here aren't randomized yet either, so I'll do it in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, I tried that. I used random_id, but random_id doesn't get compiled 😦

Copy link
Member

Choose a reason for hiding this comment

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

So we usually generate the randomness within the test via helpers (https://github.com/hashicorp/terraform/tree/master/helper/acctest), intentionally to not couple providers like random with other ones.

@@ -6,16 +6,24 @@ description: |-
Provides a GitHub issue label resource.
---

# github\_issue_label
# github_issue_label
Copy link
Member

Choose a reason for hiding this comment

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

I never really understood the point of the \ escaping here in this context 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does nothing... Trust me lol. This is identical.

@radeksimko radeksimko removed the waiting-response An issue/pull request is waiting for a response from the community label Mar 31, 2017
@sethvargo sethvargo merged commit e1be539 into master Mar 31, 2017
@sethvargo sethvargo deleted the sethvargo/create_or_update branch March 31, 2017 20:38
@ghost
Copy link

ghost commented Apr 14, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

github_issue_label is not idempotent
2 participants