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

resource/github_organization_webhook: Avoid spurious diff #134

Merged
merged 1 commit into from
Aug 16, 2018

Conversation

radeksimko
Copy link
Contributor

Similar to #133 the same bug appears in org webhook, which has pretty much the same schema, so I refactored some of the functionality to reuse it.

Here's a test failing prior to the patch:

TF_ACC=1 go test ./github -v -run=TestAccGithubOrganizationWebhook_ -timeout 120m
=== RUN   TestAccGithubOrganizationWebhook_basic
--- PASS: TestAccGithubOrganizationWebhook_basic (3.22s)
=== RUN   TestAccGithubOrganizationWebhook_secret
--- FAIL: TestAccGithubOrganizationWebhook_secret (1.38s)
	testing.go:518: Step 0 error: After applying this step, the plan was not empty:

		DIFF:

		UPDATE: github_organization_webhook.foo
		  configuration.secret: "********" => "VerySecret"

		STATE:

		github_organization_webhook.foo:
		  ID = 44243925
		  provider = provider.github
		  active = true
		  configuration.% = 4
		  configuration.content_type = json
		  configuration.insecure_ssl = 0
		  configuration.secret = ********
		  configuration.url = https://www.terraform.io/webhook
		  events.# = 1
		  events.2342231791 = pull_request
		  name = web
		  url = https://api.github.com/orgs/terraformtesting/hooks/44243925
FAIL
FAIL	github.com/terraform-providers/terraform-provider-github/github	4.633s

After patch

TF_ACC=1 go test ./github -v -run=TestAccGithubOrganizationWebhook_ -timeout 120m
=== RUN   TestAccGithubOrganizationWebhook_basic
--- PASS: TestAccGithubOrganizationWebhook_basic (2.23s)
=== RUN   TestAccGithubOrganizationWebhook_secret
--- PASS: TestAccGithubOrganizationWebhook_secret (1.21s)
PASS

@radeksimko radeksimko added the Type: Bug Something isn't working as documented label Aug 15, 2018
Copy link
Contributor

@appilon appilon left a comment

Choose a reason for hiding this comment

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

I can be persuaded to let it slide but it doesn't feel like a great place for the config func. Other than that looks good.

github/util.go Outdated
@@ -12,6 +12,44 @@ const (
maxPerPage = 100
)

func webhookConfigurationSchema() *schema.Schema {
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't love where this lives, maybe create a file named something like webhook_helpers.go

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yeah, I had mixed feelings about that too, let me clean it up 🙂

@radeksimko
Copy link
Contributor Author

@appilon Moved the schema into its own file, PTAL.

@radeksimko radeksimko merged commit df8d5d8 into master Aug 16, 2018
@radeksimko radeksimko deleted the b-org-webhook-secret branch August 16, 2018 13:29
kfcampbell pushed a commit to kfcampbell/terraform-provider-github that referenced this pull request Jul 26, 2022
…bhook-secret

resource/github_organization_webhook: Avoid spurious diff
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as documented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants