From 01514c6dc2834830e2c7079df9c7a8228d03f304 Mon Sep 17 00:00:00 2001
From: Joe Selman <jsselman@google.com>
Date: Mon, 14 Aug 2017 13:52:48 -0700
Subject: [PATCH] Cleanup testing-related code by using ConflictsWith

Also adds better comments around how update works
---
 google/resource_runtimeconfig_config.go       |  4 ++-
 google/resource_runtimeconfig_config_test.go  | 36 +++++++++++++++++++
 google/resource_runtimeconfig_variable.go     | 16 ++++++---
 .../resource_runtimeconfig_variable_test.go   |  4 +--
 4 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/google/resource_runtimeconfig_config.go b/google/resource_runtimeconfig_config.go
index 628f46776ba..4b4fb10017a 100644
--- a/google/resource_runtimeconfig_config.go
+++ b/google/resource_runtimeconfig_config.go
@@ -98,11 +98,13 @@ func resourceRuntimeconfigConfigRead(d *schema.ResourceData, meta interface{}) e
 func resourceRuntimeconfigConfigUpdate(d *schema.ResourceData, meta interface{}) error {
 	config := meta.(*Config)
 
+	// Update works more like an 'overwrite' method - we build a new runtimeconfig.RuntimeConfig struct and it becomes
+	// the new config. This means our Update logic looks an awful lot like Create (and hence, doesn't use
+	// schema.ResourceData.hasChange()).
 	fullName := d.Id()
 	runtimeConfig := runtimeconfig.RuntimeConfig{
 		Name: fullName,
 	}
-	// Update the description (currently its the only thing that could have changed)
 	if v, ok := d.GetOk("description"); ok {
 		runtimeConfig.Description = v.(string)
 	}
diff --git a/google/resource_runtimeconfig_config_test.go b/google/resource_runtimeconfig_config_test.go
index 0bccc2a3a93..e039df2751b 100644
--- a/google/resource_runtimeconfig_config_test.go
+++ b/google/resource_runtimeconfig_config_test.go
@@ -62,6 +62,35 @@ func TestAccRuntimeconfig_update(t *testing.T) {
 	})
 }
 
+func TestAccRuntimeconfig_updateEmptyDescription(t *testing.T) {
+	var runtimeConfig runtimeconfig.RuntimeConfig
+	configName := fmt.Sprintf("runtimeconfig-test-%s", acctest.RandString(10))
+	description := "my test description"
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckRuntimeconfigConfigDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccRuntimeconfigConfig_basicDescription(configName, description),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRuntimeConfigExists(
+						"google_runtimeconfig_config.foobar", &runtimeConfig),
+					testAccCheckRuntimeConfigDescription(&runtimeConfig, description),
+				),
+			}, {
+				Config: testAccRuntimeconfigConfig_emptyDescription(configName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRuntimeConfigExists(
+						"google_runtimeconfig_config.foobar", &runtimeConfig),
+					testAccCheckRuntimeConfigDescription(&runtimeConfig, ""),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckRuntimeConfigDescription(runtimeConfig *runtimeconfig.RuntimeConfig, description string) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		if runtimeConfig.Description != description {
@@ -121,3 +150,10 @@ resource "google_runtimeconfig_config" "foobar" {
  	description = "%s"
 }`, name, description)
 }
+
+func testAccRuntimeconfigConfig_emptyDescription(name string) string {
+	return fmt.Sprintf(`
+resource "google_runtimeconfig_config" "foobar" {
+ 	name = "%s"
+}`, name)
+}
diff --git a/google/resource_runtimeconfig_variable.go b/google/resource_runtimeconfig_variable.go
index 8a92b28cad3..97a16317fca 100644
--- a/google/resource_runtimeconfig_variable.go
+++ b/google/resource_runtimeconfig_variable.go
@@ -34,13 +34,15 @@ func resourceRuntimeconfigVariable() *schema.Resource {
 			},
 
 			"value": {
-				Type:     schema.TypeString,
-				Optional: true,
+				Type:          schema.TypeString,
+				Optional:      true,
+				ConflictsWith: []string{"text"},
 			},
 
 			"text": {
-				Type:     schema.TypeString,
-				Optional: true,
+				Type:          schema.TypeString,
+				Optional:      true,
+				ConflictsWith: []string{"value"},
 			},
 
 			"update_time": {
@@ -95,6 +97,10 @@ func resourceRuntimeconfigVariableUpdate(d *schema.ResourceData, meta interface{
 		return err
 	}
 
+	// Update works more like an 'overwrite' method - we build a new runtimeconfig.Variable struct and it becomes the
+	// new config. This means our Update logic looks an awful lot like Create (and hence, doesn't use
+	// schema.ResourceData.hasChange()).
+
 	variable, _, err := newRuntimeconfigVariableFromResourceData(d, project)
 	if err != nil {
 		return err
@@ -146,7 +152,7 @@ func newRuntimeconfigVariableFromResourceData(d *schema.ResourceData, project st
 	text, textSet := d.GetOk("text")
 	value, valueSet := d.GetOk("value")
 
-	if (textSet && valueSet) || (!textSet && !valueSet) {
+	if !textSet && !valueSet {
 		return nil, "", fmt.Errorf("You must specify one of value or text.")
 	}
 
diff --git a/google/resource_runtimeconfig_variable_test.go b/google/resource_runtimeconfig_variable_test.go
index 5874bcdf14e..975d62c2f95 100644
--- a/google/resource_runtimeconfig_variable_test.go
+++ b/google/resource_runtimeconfig_variable_test.go
@@ -99,7 +99,7 @@ func TestAccRuntimeconfigVariable_errorsOnBothValueAndText(t *testing.T) {
 		Steps: []resource.TestStep{
 			{
 				Config:      testAccRuntimeconfigVariable_invalidBothTextValue(),
-				ExpectError: regexp.MustCompile("You must specify one of value or text"),
+				ExpectError: regexp.MustCompile("conflicts with"),
 			},
 		},
 	})
@@ -265,6 +265,6 @@ resource "google_runtimeconfig_config" "foobar" {
 
 resource "google_runtimeconfig_variable" "foobar" {
 	parent = "${google_runtimeconfig_config.foobar.name}"
-	name = "%s"
+	name = "my-variable-namespace/%s"
 }`, acctest.RandString(10), acctest.RandString(10))
 }